Skip to content

fix(ui-react): stop editing an existing model input from failing at save (#91) - #93

Merged
mosoriob merged 1 commit into
developfrom
fix/91-input-junction
Aug 9, 2026
Merged

fix(ui-react): stop editing an existing model input from failing at save (#91)#93
mosoriob merged 1 commit into
developfrom
fix/91-input-junction

Conversation

@mosoriob

@mosoriob mosoriob commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #91. Found on #76, walking the signed-in form against TACC's live catalog.

Fault 1 — the save hard-fails (blocker)

Editing any existing input failed:

expected one of the values ['_PLACEHOLDER'] for type
'modelcatalog_configuration_input_update_column', but found 'is_optional'

Role user holds insert/select/delete on modelcatalog_configuration_input and no
update permission (graphql_engine/metadata/tables.yaml:3088). Under that role Hasura
emits the update-column enum with _PLACEHOLDER alone, so
on_conflict.update_columns: [is_optional] is rejected before it runs.

InsertConfigurationInputJunction is replaced by SetConfigurationInputOptional
delete_by_pk + insert_one in one document, so both root fields run in a single
Hasura transaction and the row is never left missing. This is the same delete-then-insert
the API layer already uses for junction updates.

The mutation is now called only when is_optional actually changed. The old code fired
it for every row in toUpdate, so editing a label, a format, a description — or the
standard variable and unit, which is the SOW workflow — was enough to fail.

Not TACC-specific: it came from this repo's metadata, so it reproduced everywhere.
No metadata change is needed, so nothing has to be applied to TACC's Hasura.

Why codegen never caught it. The committed SDL snapshot is the admin schema, where
is_optional is an updatable column. Codegen and the offline tests validate against that,
never against the user role the app actually runs as.

Fault 2 — a successful save rendered stale (minor)

ConfigurationDetail reads the same GetConfiguration cache entry the form does, and no
mutation wrote to the cache, so a successful save looked as though nothing happened until a
manual reload. The form now refetches before onSaved. A failed refresh is swallowed — it
must not report a successful save as failed.

Verification

  • 760 tests pass; lint 0 errors; npm run build clean; Prettier clean.
  • Both new assertions were checked red against the pre-fix code: making the junction
    write unconditional fails the label-edit test, and removing the refetch fails the
    refetch test.
  • Types regenerated with codegen against the committed SDL snapshot. The graphql.ts diff
    is +21/-15, confined to the renamed mutation.

Still unverified: the new document has not been run under a real Tapis token. Both root
fields are individually proven under the user role (#76 wrote and deleted junction rows),
but the composite mutation needs one live save at localhost:3000 signed in as
mosorio@portals to close #91.

Every edit to an existing input failed with:

  expected one of the values ['_PLACEHOLDER'] for type
  'modelcatalog_configuration_input_update_column', but found 'is_optional'

Role `user` has insert/select/delete on modelcatalog_configuration_input and no
update permission, so under that role Hasura emits the update-column enum with
`_PLACEHOLDER` alone and the upsert's `update_columns: [is_optional]` is rejected
before it runs. The committed SDL snapshot is the admin schema, where the column
does exist, which is why codegen accepted a document no signed-in user can send.

Replace the upsert with SetConfigurationInputOptional: delete_by_pk + insert_one
in one document, so both root fields run in a single Hasura transaction and the
row is never left missing. Call it only when is_optional actually changed — the
old code fired it for every row in toUpdate, so editing a label, a format, a
description, or the standard variable and unit was enough to fail.

Also refetch GetConfiguration before onSaved. ConfigurationDetail reads the same
cache entry and nothing wrote to it, so a successful save rendered stale until a
manual reload.

Fixes #91
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
monorepo Ready Ready Preview Aug 8, 2026 11:56pm

@mosoriob

mosoriob commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Verified live against TACC, signed in as mosorio@portals. Full detail on #91.

The core claim was settled with a zero-write probe — Hasura validates before it executes, so
sending each document with a bogus id separates the two outcomes cleanly:

document result under the user role
old, update_columns: [is_optional] validation-failed — the exact _PLACEHOLDER error
new, delete_by_pk + insert_one constraint-violation (foreign key, from the fake id)

So the new document is accepted where the old one never could be. Nothing was written.

End-to-end through the form on simpleModelAnnotated, using a throwaway input rather than the
client's existing row:

  • Add → saved, appeared with no manual reload (Fault 2 fixed).
  • Edit the label → saved clean. Operations sent: UpdateConfiguration,
    UpdateDatasetSpecification, GetConfigurationno junction mutation, since is_optional
    did not change. This is the save that used to hard-fail.
  • Toggle OptionalSetConfigurationInputOptional ran with no errors and is_optional: true
    was confirmed in the database.
  • Cleanup → both rows deleted; the catalog is back to its exact prior state.

CI note: the first Tests run failed on ConfigurationForm.test.tsx:236 (Configuration name is required), a pre-existing flake in the async validation assertion — not a test from this PR, and
green in the other workflow's Tests job. Rerun.

@mosoriob
mosoriob merged commit d9f12f9 into develop Aug 9, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant