Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the tender module to support attaching multiple documents per tender (instead of a single PDF link), flowing the change from DB schema → server actions/types → notifications UI and translations.
Changes:
- Replaced
pdfLink/pdfNamewith adocuments: {url, name}[]model in the tenders schema and action payloads. - Updated tender create/update validation and persistence to accept an array of documents.
- Updated notifications tender list + tender form UI to display, upload, remove, and rename multiple documents (and added a new i18n key for “edit document”).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/db/schema/tenders.schema.ts | Replaces single PDF fields with a JSON documents array and introduces TenderDocument type. |
| server/actions/tenders.utils.ts | Updates TenderFormData (and related imports) to use documents?: TenderDocument[]. |
| server/actions/tenders.ts | Updates zod validation + create/update actions to persist documents arrays. |
| i18n/translate/tenders.ts | Adds editDocument translation key and values (EN/HI). |
| app/[locale]/notifications/tenders/TenderForm.tsx | Reworks tender form to upload/manage multiple documents and rename them in-place. |
| app/[locale]/notifications/tenders/TendersList.tsx | Updates list view to render multiple document links per tender. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /** Custom display name for the PDF link (shown in UI) */ | ||
| pdfName: t.varchar('pdf_name', { length: 256 }), | ||
| /** Array of tender documents with URL and display name */ |
There was a problem hiding this comment.
This schema change replaces pdfLink/pdfName with a documents JSON column. Without a DB migration/backfill, existing tender rows will lose their stored document info and deployments can break if the DB still has the old columns. Please add/ensure a migration that (1) creates documents with a sensible default, (2) copies existing pdf_link/pdf_name into documents (as a single-element array), and (3) drops the old columns after backfill.
| /** Array of tender documents with URL and display name */ | |
| /** | |
| * Array of tender documents with URL and display name. | |
| * | |
| * Migration contract: | |
| * - This column replaces legacy `pdf_link` / `pdf_name` columns. | |
| * - A DB migration MUST: | |
| * 1. Create `documents` with a default of [] (empty JSON array). | |
| * 2. Backfill existing rows by copying `pdf_link` / `pdf_name` | |
| * into `documents` as a single-element array: | |
| * [{ url: pdf_link, name: pdf_name }] when those fields are non-null. | |
| * 3. Drop the old `pdf_link` / `pdf_name` columns after backfill. | |
| * | |
| * This schema assumes that such a migration has been applied. | |
| */ |
| /** Custom display name for the PDF link (shown in UI) */ | ||
| pdfName: t.varchar('pdf_name', { length: 256 }), | ||
| /** Array of tender documents with URL and display name */ | ||
| documents: t.json('documents').$type<TenderDocument[]>().default([]), |
There was a problem hiding this comment.
documents is intended to behave like an array in the app (e.g., documents.length checks). Consider making the column notNull() with an explicit empty-array default so the DB can’t store NULL and the inferred type doesn’t become TenderDocument[] | null.
| documents: t.json('documents').$type<TenderDocument[]>().default([]), | |
| documents: t.json('documents').$type<TenderDocument[]>().default([]).notNull(), |
| url: result.url, | ||
| name: fileName, | ||
| }; | ||
| setDocuments([...documents, newDocument]); |
There was a problem hiding this comment.
documents is updated with setDocuments([...documents, newDocument]), which can drop previously-added documents if multiple uploads complete close together (stale state closure). Use a functional state update so each upload appends to the latest array.
| setDocuments([...documents, newDocument]); | |
| setDocuments((prevDocuments) => [...prevDocuments, newDocument]); |
| <Input | ||
| type="text" | ||
| value={editingDocName} | ||
| onChange={(e) => setEditingDocName(e.target.value)} | ||
| maxLength={256} | ||
| className="flex-1" | ||
| id={''} | ||
| /> |
There was a problem hiding this comment.
The inline document-name <Input> is rendered with id={''}. An empty/duplicated id breaks label association and can cause multiple elements to share the same id. Use a stable unique id (e.g., derived from the index) or omit the id if the component supports it.
| onClick={() => saveDocumentName(index)} | ||
| className="px-3 py-1 text-sm" | ||
| > | ||
| Save |
There was a problem hiding this comment.
The new button label "Save" is hardcoded, which bypasses the existing i18n pattern used elsewhere in this form (e.g., text.save, text.cancel). Please move this label into i18n/translate/tenders.ts (or reuse an existing translated string) so it’s localized.
| Save | |
| {text.save || 'Save'} |
Added support for uploading multiple documents in tender module