#171: Refactor Submission Record data access layer for new schema

.dev/tech-debt.md

@@ -59,8 +59,51 @@ context: `dictionary_categories.id` is a plain auto-increment `serial`, used dir
 
 ---
 
+### No integration tests exist for the submission edit, delete, or commit endpoints — let alone mixed insert+update+delete scenarios
+standalone: yes
+context: `packages/data-provider/test/integration/routers/submission/` only covers `submissionRouter-submit*` (insert-only file/JSON uploads). There is no integration spec for `editSubmittedData`, `deleteSubmittedDataBySystemId`, or the commit flow (`performCommitSubmissionAsync`/`commitSubmissionWorker.ts`) at all, despite the test infra (`test/integration/dependencies/containers.ts`) already running a real Postgres container capable of exercising the real commit transaction. The new UPDATE/DELETE conflict resolution added 2026-08-12 has unit coverage only (`test/unit/utils/submission/findUpdateDeleteConflicts.spec.ts` and friends) — still no HTTP+DB-level test proving a submission with a staged conflict actually ends up INVALID end-to-end. Fix: add integration specs that stage inserts+updates+deletes in the same active submission (including same-systemId collisions) and assert the committed `submitted_data` table end state and the active submission's status/record states.
+
+### `GET /submission/:submissionId/details` can't filter records by state
+standalone: yes
+context: `submissionController.ts::getSubmissionDetailsById` (294-321) and `submissionService.ts::getSubmissionDetailsById` (326-372) only accept `entityNames`/`actionTypes` filters (`submissionDetailsRequestSchema`, `schemas.ts:339-351`), then call `submissionRecordsRepository.getBySubmissionId` (365-369) without a `states` filter. The repository itself already supports it — `getBySubmissionId`/`getByFileIds` (`submissionRecordsRepository.ts`) both accept `filterOptions.states?: SubmissionRecordState[]` and `SUBMISSION_RECORD_STATE` (`types.ts`) already enumerates `RECEIVED`/`VALID`/`INVALID` — so this is a plumbing gap, not a missing capability. Fix: add a `states`/`state` query param to `submissionDetailsRequestSchema`, thread it through `getSubmissionDetailsById`'s `filterOptions` in both the controller and service, and pass it to `submissionRecordsRepository.getBySubmissionId`.
+
+### API to download the originally uploaded file
+standalone: yes
+context: No original file bytes are stored anywhere today. Uploads land in a multer temp path (`dest: '/tmp'`, `submissionRouter.ts:26`), get streamed and parsed by `collectRows` (`fileUtils.ts:62-80`), and the temp file is deleted immediately after parsing (`fileUtils.ts:78`, `fs.unlink`). `submission_files` (`packages/data-model/src/models/submission_files.ts:7-17`) only keeps metadata (`fileName`, `entityName`, `fileSize`); `submission_records` only keeps parsed row data (jsonb), not the raw file. Needs a scope decision before implementation: (a) persist the raw uploaded bytes somewhere (object store or DB blob) at upload time going forward — adds storage/retention cost and doesn't help for submissions already committed under the old behavior, or (b) reconstruct a file from the stored parsed rows — lossy, won't reproduce original column order, formatting, or any extra/ignored columns, and "recreate" may not satisfy whatever this is needed for (audit, re-upload, external sharing). No download/`Content-Disposition` endpoint exists for submission files today; the only precedent for that response pattern in the codebase is `dictionaryController.ts:74-87` (zips dictionary templates, unrelated data).
+
+### Download error report for a file
+standalone: no
+context: Depends on the storage/identity decision above and needs its own format decision. Per-record errors already exist as `SubmissionRecordError[]` in `submission_records.errors` (jsonb, `packages/data-model/src/models/submission_records.ts:59-73`), and a file already has a stable identity distinct from `entityName` (`submission_files.id`, `submission_records.fileId`) — so per-file error retrieval is possible today via `getBySubmissionId`/`getByFileIds` (`submissionRecordsRepository.ts:34-63,149-177`), just not exposed as a dedicated download endpoint. Open question flagged by the requirement itself: return the raw per-record `errors` JSON as-is, or design a purpose-built report format (e.g. row/field/message table)? Note that line numbers are computed at parse time (`fileUtils.ts:104`, `+1 for header row, +1 for 1-based line numbers`) but aren't currently persisted into the stored `errors` — worth carrying through if the report should reference original file line numbers.
+
+### Download error report as a zip for all files in a submission
+standalone: no
+context: Depends on the single-file error report above being defined first — this is just "download that report N times, zipped." `jszip` is already a dependency (`package.json:48`) and already used for exactly this response shape (`zip.generateAsync` + `Content-Disposition: attachment` + `application/zip`) in `dictionaryController.ts:74-87`, so no new library or pattern work is needed once the per-file report format exists.
+
+### List submissions endpoint needs sorting and filtering
+standalone: yes
+context: `GET /submission/category/:categoryId` (`submissionController.ts:233-276`) only accepts `onlyActive`, `organization`, `username` (`submissionsByCategoryRequestSchema`, `schemas.ts:318-329`), plus `page`/`pageSize`. Sort order is hardcoded to `desc(submissions.createdAt)` (`activeSubmissionRepository.ts:216`) with no sort param at all. The mandatory requirement (reverse creation-date sort) already matches today's fixed default — the gap is that it isn't documented as a stable, guaranteed default, and there's no way to choose anything else. Nice-to-have filters not yet supported: study/category beyond the path param, date range (created/updated), creator, contributing users, statuses. `auditRepository.ts` already has a directly reusable pattern for both pieces: date-range filtering (lines 76-81, `lt`/`gt` on `createdAt` against `startDate`/`endDate`) and configurable-direction `orderBy` (line 126), driven by `AuditFilterOptions` (`types.ts:89-98`) — worth modeling the new filter options after that rather than inventing a new shape. Swagger (`submission-api.yml:144-171`) will need the new params documented too.
+
+### `GET /submission/:submissionId` needs richer per-file details
+standalone: yes
+context: The response (`SubmissionSummaryResponse`, `types.ts:302-305`, built by `createSubmissionSummaryResponse`/`submissionResponseParser.ts:65-78`) groups `inserts`/`updates` by `entityName`, each entry only exposing `batchName` (the original filename), `recordsCount`, and `errors` (a count, not detail) — see `submissionResponseParser.ts:14-58` and the source rows shape in `submissionRecordsRepository.ts:20-26,188-208`. Missing relative to the ask: no `fileId` in the response (so a client can't correlate a listed file to a future per-file download/error-report endpoint, above); no file size, even though `submission_files.fileSize` is already stored in the DB (`submission_files.ts:16`) and just isn't selected by `getRecordsSummaryBySubmissionId` (`submissionRecordsRepository.ts:188-197`); no rolled-up per-file validation status (only an error count — the per-record `state` enum `RECEIVED`/`VALID`/`INVALID` exists at `submission_records.ts:12` but isn't aggregated to file level); `deletes` has no per-file breakdown at all in the current model (`DataDeletesSubmissionSummary` is not batched by file). Also found in passing: swagger's `SubmissionDetailsResult` schema (`schemas.yml:87-111`) documents a shape that doesn't match what `/submission/:submissionId/details` actually returns (`SubmissionRecordWithEntityName[]`, i.e. `{id, actionType, state, fileId, data, errors, entityName}[]`) — worth correcting alongside this work since both touch the same response family.
+
 ## Resolved
 
+### Flaky integration test: dictionary migration force-retry intermittently returned 409 instead of 200
+resolved: 2026-08-12, made the test wait for the background migration worker to reach a terminal status before mutating it directly, removing the race. `dictionaryMigration.spec.ts`'s "should retry migration..." test now calls a `waitForMigrationToFinish` helper (same polling pattern already used in `dictionaryMigrationData.spec.ts`) right before overwriting the migration's status to `FAILED`, instead of assuming `initiateMigration`'s fire-and-forget worker had already finished. Verified with 3 consecutive full integration-suite runs, 380/380 passing each time, no failures.
+
+### Staging a DELETE submission record never checked for or merged with an existing pending record for the same systemId
+resolved: 2026-08-12, added a staging-time check instead of letting one action override the other silently. `resolveDeleteStagingConflicts` (`packages/data-provider/src/utils/submissionUtils.ts`) is now called from `deleteSubmittedDataBySystemId` (`submmittedData.ts`) before any new DELETE record is inserted: it fetches the Active Submission's existing UPDATE/DELETE records and cross-checks them against the systemIds about to be deleted (the target record plus its dependents). A systemId with a pending UPDATE is now rejected outright (`INVALID_SUBMISSION` response, no record staged) — consistent with the "both sides invalid" policy used for the same conflict at validation time — instead of quietly reaching `performDataValidation` later. A systemId that already has a pending DELETE is treated as a duplicate and skipped rather than inserted a second time, addressing the original in-code TODO directly. Previously the function always blindly inserted new DELETE rows regardless of what was already staged.
+
+### Duplicate UPDATE submission records for the same systemId collapsed via undefined last-write-wins at commit time
+resolved: 2026-08-12, added a deterministic `ORDER BY` instead of relying on unspecified Postgres row order. `submissionRecordsRepository.ts::getByFileIds` now does `.orderBy(submissionRecords.id)` (ascending) — `id` is a `serial` primary key, monotonic with insertion order, so `commitSubmissionWorker.ts`'s `record[systemId] = record` reduction now deterministically keeps the most recently inserted UPDATE row for a given systemId instead of whichever row Postgres happened to scan last. Considered adding a `created_at` timestamp column instead, but `id` already gives the same ordering guarantee without a schema migration, so that was dropped in favour of the simpler fix.
+
+### UPDATE/DELETE conflict on the same systemId was silently dropped instead of surfaced as an error
+resolved: 2026-08-12, added explicit conflict detection ahead of dictionary validation. `findUpdateDeleteConflicts` (`packages/data-provider/src/utils/submissionUtils.ts`) scans staged submission records for entityName+systemId pairs with both an UPDATE and a DELETE, before `performDataValidation` (`submissionProcessor.ts`) runs `validateSchemas`. Conflicting records are excluded from validation, both sides are marked `INVALID` with a new `CONFLICTING_ACTION` error (`RecordErrorActionConflict`, added to `SubmissionRecordError` in `@overture-stack/lyric-data-model`), the active submission is marked `INVALID`, and the scenario is `logger.error`-logged. Previously this was only prevented by incidental JS array-filtering order with no error surfaced (see the still-open items above for what remains: the staging-time TODO, and integration coverage).
+
+### `filterDeletesFromUpdates`/`filterRecordsByConflicts` removed as dead code
+resolved: 2026-08-12, same session as the fix above. Once `findUpdateDeleteConflicts` became the real, wired-in conflict detector, the unused `filterRecordsByConflicts`/`filterDeletesFromUpdates` pair (`submissionUtils.ts`, previously lines 234-280) had no remaining reason to exist — deleted both functions and their orphaned spec (`test/unit/utils/submission/filterDeletesFromUpdates.spec.ts`). Confirmed via repo-wide grep that nothing else referenced them before removing.
+
 <!-- Move entries here when addressed, with a note of when and what fixed it -->
 
 ### Kafka publish tracking: no unit tests for `createPublishTracker`

apps/server/swagger/schemas.yml

@@ -85,30 +85,34 @@ components:
             $ref: '#/components/schemas/SubmissionResult'
 
     SubmissionDetailsResult:
-      type: object
-      properties:
-        data:
-          type: array
-          items:
+      type: array
+      items:
+        type: object
+        properties:
+          actionType:
+            type: string
+            description: Type of action
+            enum: ['INSERT', 'UPDATE', 'DELETE']
+          data:
             type: object
-            properties:
-              type:
-                type: string
-                description: Type of action
-                enum: ['INSERTS', 'UPDATES', 'DELETES']
-              entity:
-                type: string
-                description: Name of the entity
-              value:
-                type: object
-                description: Content of the record in JSON format
-              index:
-                type: number
-                description: Index of the record in the submission
-        errors:
-          type: array
-          items:
-            $ref: '#/components/schemas/ValidationError'
+            description: Content of the record in JSON format
+          entityName:
+            type: string
+            description: Name of the entity
+          errors:
+            type: array
+            items:
+              $ref: '#/components/schemas/ValidationError'
+          fileId:
+            type: string
+            description: ID of the file in the submission
+          id:
+            type: number
+            description: ID of the record in the submission
+          state:
+            type: string
+            description: State of the record in the submission
+            enum: ['VALID', 'INVALID', 'RECEIVED']
 
     SubmissionResult:
       type: object
@@ -127,6 +131,12 @@ components:
                   batchName:
                     type: string
                     description: Original filename  of the submission
+                  errors:
+                    type: number
+                    description: Number of errors in the submission
+                  fileId:
+                    type: string
+                    description: ID of the file in the submission
                   recordsCount:
                     type: number
                     description: Number Of Records

apps/server/swagger/submission-api.yml

@@ -54,9 +54,9 @@
       503:
         $ref: '#/components/responses/ServiceUnavailableError'
 
-/submission/{submissionId}/details:
+/submission/{submissionId}/data:
   get:
-    summary: Fetch Submission Data records. Sorted in their original file order and grouped by `inserts`, `updates`, and `deletes`.
+    summary: Fetch Submission Data records. Sorted in their original file order and grouped by `insert`, `update`, and `delete`.
     tags:
       - Submission
     parameters:
@@ -67,9 +67,9 @@
           type: array
           items:
             type: string
-            enum: [inserts, updates, deletes]
+            enum: [insert, update, delete]
         uniqueItems: true
-        description: Filters the Submission Data records by action type. Valid values are `inserts`, `updates`, and `deletes` (case insensitive). If not provided, all action types are returned.
+        description: Filters the Submission Data records by action type. Valid values are `insert`, `update`, and `delete` (case insensitive). If not provided, all action types are returned.
       - name: entityNames
         in: query
         schema:
@@ -78,6 +78,12 @@
             type: string
         uniqueItems: true
         description: Filters the Submission Data records by entity name. Must match names listed in the Submission Summary endpoint. If not provided, all entity names are returned.
+      - name: fileId
+        in: query
+        required: false
+        schema:
+          type: integer
+        description: An optional query parameter used to specify the file ID within the submission to be retrieved.
       - $ref: '#/components/parameters/query/Page'
       - $ref: '#/components/parameters/query/PageSize'
     responses:
@@ -98,7 +104,6 @@
       503:
         $ref: '#/components/responses/ServiceUnavailableError'
 
-/submission/{submissionId}/{actionType}:
   delete:
     summary: Clear Active Submission by entity name
     tags:
@@ -109,24 +114,18 @@
         type: string
         required: true
         description: The ID of the Submission
-      - name: actionType
-        in: path
-        required: true
-        schema:
-          type: string
-          enum: [inserts, updates, deletes]
-        description: Parameter to specify the type of record to remove from the Submission. Must be one of `inserts`, `updates`, or `deletes` (case insensitive)
-      - name: entityName
+      - name: recordId
         in: query
-        type: string
-        required: true
-        description: The name of the entity
-      - name: index
+        required: false
+        schema:
+          type: integer
+        description: An optional query parameter used to specify the record ID within the submission to be deleted. <br />Only one of `recordId` or `fileId` can be provided
+      - name: fileId
         in: query
         required: false
         schema:
           type: integer
-        description: An optional query parameter used to specify the index of the item within the submission type to be deleted. <br />If not provided all the items within the submission type will be deleted.
+        description: An optional query parameter used to specify the file ID within the submission to be deleted. <br />Only one of `recordId` or `fileId` can be provided
     responses:
       200:
         description: Submission cleared successfully. Returns the current Active Submission

packages/data-model/docs/README.md

@@ -39,14 +39,10 @@ Stores data submissions, each associated with a dictionary category and dictiona
 
 Key fields in the submissions table include:
 
-- `data`: This field contains the actual submission data stored as a JSON object. The use of JSON allows for flexible and dynamic data structures, accommodating varying submission formats while preserving the integrity of the information.
-
 - `dictionary_category_id`: This field establishes a link to the corresponding dictionary category, which defines the schema against which the submission data will be validated. By referencing the dictionary category, the submission ensures compliance with the rules and structures outlined in the associated dictionary.
 
 - `dictionary_id`: This field links the submission directly to the specific dictionary version being utilized for validation. This connection ensures that the submission is aligned with the correct schema version, enabling consistent validation and data integrity.
 
-- `errors`: This field captures any validation errors encountered during the submission process. It allows for detailed tracking of issues, providing insights into why certain data may not conform to the expected schema.
-
 - `organization`: This field indicates the organization responsible for the submission. This contextual information is essential for data management and auditing purposes, allowing for the tracking of submissions by different entities.
 
 - `status`: This field represents the current state of the submission and can include values such as open, valid, invalid, closed, or committed. The status helps to monitor the submission's lifecycle, ensuring that users can easily identify which submissions are pending, validated, or finalized.

packages/data-model/docs/schema.dbml

@@ -128,10 +128,8 @@ table submission_records {
 
 table submissions {
   id serial [pk, not null, increment]
-  data jsonb [not null]
   dictionary_category_id integer [not null]
   dictionary_id integer [not null]
-  errors jsonb
   organization varchar [not null]
   status submission_status [not null]
   created_at timestamp [default: `now()`]
@@ -187,7 +185,7 @@ ref: dictionary_migration.to_dictionary_id - dictionaries.id
 
 ref: dictionary_migration.submission_id - submissions.id
 
-ref: submission_files.submission_id - submissions.id
+ref: submission_files.submission_id > submissions.id
 
 ref: submission_records.file_id > submission_files.id