Skip to content

fix(ui-react): delete a problem statement, task or thread bottom-up (#99) - #115

Merged
mosoriob merged 1 commit into
developfrom
fix/99-delete-orphans-rows
Aug 10, 2026
Merged

fix(ui-react): delete a problem statement, task or thread bottom-up (#99)#115
mosoriob merged 1 commit into
developfrom
fix/99-delete-orphans-rows

Conversation

@mosoriob

Copy link
Copy Markdown
Contributor

Fixes #99. Needs mintproject/graphql_engine#14 — this PR alone changes nothing.

What was wrong

Deleting a problem statement removed it from the list and left it in the database, with its task and its thread. The owner cannot see the orphan, because the user select filter reads the same provenance the delete removed. anonymous still can.

The ticket named one failed root field. All eleven failed. The user role may delete one of these rows only while it still carries a CREATE provenance event from that user, and every child on the tree is gated the same way through the thread's provenance or permission rows. The mutation deleted provenance and permission first, so everything after it lost its authorisation and matched 0 rows. delete_*_by_pk answers null for a filtered-out row, with no errors key, so Apollo resolved and the app said "Problem statement deleted".

The ticket's first option cannot work. Those FKs were ON DELETE RESTRICT, so deleting the row before its provenance is refused by Postgres. Both orders were run against a live MINT database:

ORDER B (row before provenance): REFUSED -- update or delete on table "problem_statement"
  violates foreign key constraint "problem_statement_provenance_problem_statement_id_fkey"
ORDER A (provenance before row): deleted 0 row(s)
orphan_left: 1

No client-only fix exists, which is why this needs mintproject/graphql_engine#14: six FKs become ON DELETE CASCADE so provenance and permission go with the row they authorise instead of before it.

Not a regression from the Lit port. ui/src/queries/{problem-statement,task,thread}/delete.graphql are byte-identical in this respect. Delete is broken in the incumbent app too, and has been since these permissions were written.

What this PR changes

  1. All three mutations delete bottom-up and never touch provenance or permission. Six root fields removed from DeleteProblemStatement, four from DeleteTask, two from DeleteThread.

  2. Restores three root fields the port dropped. Lit deletes dataslice_resource, dataslice and thread_data; ui-react did not. That was invisible while the mutation was a no-op, but Thread wizard dead-ends at Parameters: threadExecutionData is never loaded #104 started writing those rows, so a fixed delete would have failed on thread_data_thread_id_fkey instead. Order is load-bearing and is asserted by a test: dataslice's permission filter walks through thread_data, so it must go first — legal because thread_data.dataslice_id is DEFERRABLE INITIALLY DEFERRED.

  3. assertDeleted turns a zero-row delete into an error. A silent no-op is what kept this invisible for as long as it was.

Verification

  • The migration was applied to the mint.local dev cluster, and the new mutation rehearsed there root field by root field, under the exact predicates Hasura compiles each permission into. Every field matched its row, all six dependants cascaded, and the deferred FK check passed at SET CONSTRAINTS ALL IMMEDIATE. Run in a rolled-back transaction; no residue.
  • The Submit one thread run end to end against TACC's Ensemble Manager #92 orphan named in the ticket (5jjeamy1msm8hzwb and its task and thread) is removed from the dev cluster. It needed the admin secret, because the permission it would have needed was the provenance that was deleted.
  • 910 tests green, tsc -b clean, lint clean. The 27 new assertions were checked to fail against the old code (12 of 22 in the mutation-order test).

Not done here

The round trip through Hasura under a real Tapis token has not been run — the proof is at the SQL layer, using the predicates Hasura generates, against the real database. Worth one click through the signed-in app once mintproject/graphql_engine#14 is applied.

The three delete mutations removed the provenance and permission rows
first, copied from the Lit app. Those rows are what authorise the
delete: the user role may remove one of these only while the row still
carries a CREATE provenance event from that user, and every child on the
tree is gated through the thread's provenance or permission. So the
whole cascade matched 0 rows, and Hasura reports a filtered-out row as
`null` with no errors key, which the app read as success.

Deleting the row first does not work either -- those FKs were
ON DELETE RESTRICT. graphql_engine now cascades them, so this deletes
bottom-up and never touches provenance or permission.

Two more faults fixed with it:

- The port dropped Lit's dataslice_resource, dataslice and thread_data
  deletes. Nothing noticed while the mutation was a no-op, but since
  #104 started writing those rows a fixed delete would have hit an FK
  error. Order is load-bearing: dataslice is filtered through
  thread_data, so it goes first, which the deferred FK allows.

- assertDeleted raises when _by_pk comes back null, so a delete the
  server declines can no longer render as a success toast.

Requires graphql_engine PR #14.

Fixes #99
@vercel

vercel Bot commented Aug 10, 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 10, 2026 1:20am

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