Accept track_position in POST/PATCH flowsheet controllers (BS#943)#945
Merged
Conversation
BS#835 / PR #932 shipped the schema, the read projection, and the V2 wire shape for `flowsheet.track_position`, but the V1 `POST /flowsheet` and `PATCH /flowsheet/entries/:id` controllers in apps/backend/controllers/flowsheet.controller.ts didn't accept the field on the request body. Without this, the dj-site flowsheet picker (E6-6, dj-site#501) can send `track_position` and BS silently drops it. Three edits: - `FSEntryRequestBody.track_position?: string | null` and `UpdateRequestBody.track_position?: string | null` declared. - `addEntry` album_id branch forwards `body.track_position ?? null` into `NewFSEntry` explicitly (the explicit assembly didn't spread `body`). - The free-form branch already propagates via the existing `...body` spread, so the type widening alone is the entire wiring there. The message-only branch (talkset / breakpoint / PSA) deliberately does NOT carry `track_position` — that column is `entry_type='track'`-only by design. A regression test pins this. `updateEntry` service does a straight `db.update(flowsheet).set(data)`, so widening `UpdateRequestBody` is the only wiring needed. Tests: - tests/unit/controllers/flowsheet.controller.test.ts: 3 new addEntry cases + 1 updateEntry case pinning the forwarding contract; the message-branch negative case prevents future drift. - tests/integration/flowsheet.spec.js: round-trip POST then read for both the album_id branch ("A1") and the free-form branch ("B2"), exercising real PG + transformToV2's projection. Local CI: typecheck, lint (0 errors), format:check, test:unit (1937 passing). Closes #943.
Review feedback on PR #945: 1. apps/backend/app.yaml: the V1 wire contract changes — POST /flowsheet accepts `track_position` and PATCH /flowsheet updates it — but the swagger schema didn't reflect that. Add `track_position` (string, nullable) to the `FlowsheetEntry` component (used by POST) and to the inline PATCH request schema. 2. tests/integration/flowsheet.spec.js: the unit test for `updateEntry` mocks the service. Add a round-trip integration test that POSTs a row with `track_position: 'A1'`, PATCHes it to 'B2', then clears it to `null`, asserting the PATCH response each time. This pins the actual UPDATE wire path through Postgres (Drizzle `.update(...).set(data) .returning()`) and prevents a future regression that drops the field on the write side.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires the V1
POST /flowsheetandPATCH /flowsheet/entries/:idcontrollers to accepttrack_positionon the request body and forward it through to the DB. BS#835 / PR #932 shipped the column, the read projection, and the V2 wire shape — but the controllers were silently dropping the field. Without this wiring the dj-site flowsheet picker (E6-6, WXYC/dj-site#501) would writetrack_positionover the wire and the column would stay NULL forever.Changes
FSEntryRequestBodytrack_position?: string | nullUpdateRequestBodytrack_position?: string | nulladdEntryalbum_id branchtrack_position: body.track_position ?? nullintoNewFSEntryaddEntryfree-form branch...bodyspread propagates once the type allows itaddEntrymessage-only branchentry_type='track'-only by design; regression test pins this)updateEntrydb.update(flowsheet).set(data)already passes any field on the type-wideneddatathroughTest plan
addEntry:track_positionlands in theaddTrackcalltrack_positiondoes NOT land (negative case)updateEntry— forwardstrack_positionvia the servicePOST /flowsheet:track_position: 'A1'written and read backtrack_position: 'B2'written and read backnpm run typecheckcleannpm run lint— 0 errors (396 pre-existing warnings unchanged)npm run format:checkcleannpm run test:unit— 1937 passing (151 suites)Why this is split from BS#835
PR #932 was scoped to the schema + read projection. Splitting the controller wiring into its own PR keeps the storage change reviewable on its own, lets the controller wiring land closer to dj-site#501 (the actual writer), and means a hypothetical revert of #932 wouldn't snag this on the way out.
Project-32 compatibility
Same posture as BS#835: this PR doesn't touch
apps/backend/services/metadata/,apps/backend/services/lml/, or any of the project-#32 surfaces. Controller-layer-only.Closes #943.