Conversation
TrickkyRicky
requested review from
iamsrijan724,
kierstinhicks,
mrysav and
whao37
| import { prisma } from '@/lib/prisma'; | ||
|
|
||
| export async function GET(request: Request) { | ||
| const UPLOAD_DIR = path.join(process.cwd(), 'uploads', 'nonprofit-documents'); |
There was a problem hiding this comment.
Can you make this configurable via environment variable? eg FILE_UPLOADS or something?
When this is deployed via our Coolify/Docker set up, I will create a Docker volume and mount it at some arbitrary point, and set the uploads directory to that.
edit: Okay, I do see you're effectively doing that with the new volume, but IMO it would be better to make it explicitly configurable rather than rely on process.cwd
|
|
||
| // Write new file to disk | ||
| await mkdir(UPLOAD_DIR, { recursive: true }); | ||
| const diskFileName = `${Date.now()}-${file.name}`; |
There was a problem hiding this comment.
non-blocker: rather than Date.now(), can you either
- use a custom date format so there are eg. no spaces or special characters, eg.
20260301230200, or - use a plain epoch timestamp?
|
|
||
| // Write new file to disk | ||
| await mkdir(UPLOAD_DIR, { recursive: true }); | ||
| const diskFileName = `${Date.now()}-${file.name}`; |
| expect(data).toEqual({ error: 'User not found' }); | ||
| }); | ||
|
|
||
| it('should allow a roleless user to set their own role during onboarding', async () => { |
There was a problem hiding this comment.
Not sure if there is a test for this already, but I don't think we should allow users to onboard themselves as Admin
| let filePath: string | undefined; | ||
|
|
||
| if (documentData?.fileData) { | ||
| const fileBuffer = Buffer.from( | ||
| Object.values(documentData.fileData) as number[] | ||
| ); | ||
| await mkdir(UPLOAD_DIR, { recursive: true }); | ||
| const diskFileName = `${documentId}-${documentData.fileName || 'document'}`; | ||
| filePath = path.join(UPLOAD_DIR, diskFileName); | ||
| await writeFile(filePath, fileBuffer); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can functions like these be separated out into their own helper class? I feel like I'm seeing some file-related features duplicated (eg. getting a file name) and we don't want a situation where refactoring means changing a bunch of places.
3 participants
What changed
NonprofitDocumentwas storing raw file bytes in afileData Bytescolumn. So we now store the reference to where the file is located with the newfilePathcolumn. Existing records are left the same and that flow is fine but new files going forward will have thefilePathcolumn populated instead offileData.Two storage paths
fileDatafilePathNULLNULLThe download endpoint handles both transparently
Key changes
Database
fileDatamade optional (Bytes?) existing rows left samefilePath String?addedNULLon all existing rowsAPI
GET /api/nonprofit-documentslist endpoint, returns metadata onlyGET /api/nonprofit-documents/download?id=new dedicated download endpoint; checksfilePathfirst, falls back tofileDatafor legacy recordsPOST /api/nonprofit-documentswrites to disk, storesfilePath, clearsfileDataon updatePATCH /api/nonprofit-documentssame as POST; also deletes the old file from disk when replacingOther
docker-compose.yml—uploads/directory mounted as a named volume so files survive container restarts