fix(formdesigner): bind form_uid as UUID, matching the column 004 creates - #1103
Merged
Conversation
…ates
`004_form_uid_uuid_type.sql` retypes `form_schemas.form_uid` and
`form_data.form_uid` to native `UUID`. The storage layer never followed:
`services/storage.py` and `services/submissions.py` still called `str()` on
every form_uid before binding it, each marked `# TASK-2008: form_uid column
is VARCHAR(36) until migrated`.
So the package shipped a migration that breaks its own data access. Applying
004 — the migration parrot itself publishes — makes asyncpg reject every
bind with `invalid input for query argument $1: '...' ('str' object has no
attribute 'bytes')`, and `FormRegistry.load_from_storage()` silently returns
ZERO forms: registry.py:1070 catches the error per-row and logs a warning, so
the app boots looking healthy with an empty registry.
Observed on a real deployment (FieldSync staging, 2026-08-02): 64 forms
before applying 004, `Loaded 0 forms from storage` after.
Changes — the column is UUID, so bind a UUID:
* storage.py — `_create_table_sql` declares `form_uid UUID NOT NULL` (was
VARCHAR(36)); save/load/delete bind `form.form_uid` directly. Module
docstring updated.
* submissions.py — same for the `form_data` DDL, its `ADD COLUMN`, and the
insert.
* api/handlers.py — the str->UUID normalisation in the descriptor merge is
KEPT (it is inert against a UUID column and still covers a deployment that
has not applied 004); only its now-false comment is corrected.
Tests updated rather than removed — they were pinning the old contract and
were right to fail:
* `form_uid UUID NOT NULL` in the DDL assertion.
* save/load/load-with-version now assert `isinstance(arg, uuid.UUID)` as well
as equality. Equality alone is not enough: it would still pass if the code
found another way to bind a string.
* `test_delete_by_form_uid` passed the literal `"test-uid-004"` — not even a
UUID — to a method annotated `form_uid: uuid.UUID`, and only passed because
`delete()` stringified it and the fake connection type-checks nothing. It
now uses a real UUID.
Verified:
* Package suite: 18 failed / 1790 passed / 80 errors — byte-identical to the
same suite on clean `dev`. Delta is empty; the pre-existing failures are
untouched.
* End to end against a real 004-migrated Postgres: `list_forms()` returns 59,
and `load()` resolves 59 of 59. Before this change: 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
The bug
packages/parrot-formdesigner/migrations/004_form_uid_uuid_type.sqlretypesform_schemas.form_uidandform_data.form_uidto nativeUUID. The storage layer never followed: it still calledstr()on every form_uid before binding, each site marked# TASK-2008: form_uid column is VARCHAR(36) until migrated.The package ships a migration that breaks its own data access. Apply 004 — the migration parrot itself publishes — and asyncpg rejects every bind:
The failure mode is the bad kind.
FormRegistry.load_from_storage()catches per row and logs a warning (registry.py:1070), so the app boots looking healthy with an empty registry.Observed on a real deployment (FieldSync staging, 2026-08-02): 64 forms loaded before applying 004,
Loaded 0 forms from storageafter.The fix
The column is UUID, so bind a UUID.
services/storage.py—_create_table_sqldeclaresform_uid UUID NOT NULL; save / load / delete bindform.form_uiddirectly. Module docstring updated.services/submissions.py— same for theform_dataDDL, itsADD COLUMN IF NOT EXISTS, and the insert.api/handlers.py— thestr -> UUIDnormalisation in the descriptor merge is kept. It is inert against a UUID column and still covers a deployment that has not applied 004. Only its now-false comment is corrected.Tests: updated, not deleted
Four tests failed. They were pinning the old contract and were right to fail — so they now pin the new one:
form_uid UUID NOT NULL.isinstance(arg, uuid.UUID)as well as equality. Equality alone would still pass if the code found another way to bind a string.test_delete_by_form_uidpassed the literal"test-uid-004"— not even a UUID — to a method annotatedform_uid: uuid.UUID. It only passed becausedelete()stringified it and the fake connection type-checks nothing. It now uses a real UUID.Verification
devlist_forms()-> 59,load()resolves 59 of 59. Before: 0Note on compatibility
This is a clean cut: after this change the code requires 004 to have been applied. That matches the direction FEAT-393 already took in Python (
FormSchema.form_uidisuuid.UUID), and 004 is already published. A deployment on an unmigrated column needs to run 004 — which was always the intent, sinceinitialize()does not retype.Worth flagging separately:
migrations/README.mddocuments only steps 001-003. 004, 005 and 006 exist in the directory but appear in no execution order, so anyone following the README lands exactly in the state this PR fixes.🤖 Generated with Claude Code