Skip to content

perf(pb,frontend): cascadeDelete bunk_assignments_draft on scenario delete#994

Merged
adamflagg merged 4 commits intomainfrom
feature/scenario-cascade-delete
Apr 25, 2026
Merged

perf(pb,frontend): cascadeDelete bunk_assignments_draft on scenario delete#994
adamflagg merged 4 commits intomainfrom
feature/scenario-cascade-delete

Conversation

@adamflagg
Copy link
Copy Markdown
Owner

@adamflagg adamflagg commented Apr 24, 2026

Summary

Follow-up to #992 (scoreboard item #16b). #992 fixed the visible "list vanishes" bug; this PR kills the underlying "long delete" — the several-second serial-delete loop that made the disappearance so noticeable.

Root cause: bunk_assignments_draft.scenario was declared with cascadeDelete: false in migration 1500000022, so useDeleteScenario had to fetch every draft assignment for the scenario and delete them one-by-one from the frontend before deleting the scenario itself. For a real session, that's N+1 round-trips against PocketBase on every delete.

Fix: New migration 1500000098 flips the relation to cascadeDelete: true (all other properties preserved exactly as in 1500000022). useDeleteScenario collapses to a single pb.collection('saved_scenarios').delete(id) — PocketBase cascades the draft rows server-side.

Base branch

This PR targets feature/fix-delete-scenario-render (PR #992) rather than main. Once #992 merges, I'll retarget this at main or it'll auto-rebase.

Test plan

  • Failing TDD test first (useDeleteScenario asserted no calls to bunk_assignments_draft.getFullList / .delete — red against the N+1 loop).
  • Implementation: single-call mutation; test goes green.
  • Full frontend suite: 2797 passing.
  • prettier / tsc / eslint (0 errors) clean; pre-push hooks green.
  • Migration symmetry: down restores cascadeDelete: false so a rollback re-requires the frontend loop if paired with an older build.

Manual validation (dev/preview)

Scoreboard item covered: #16b — cascadeDelete bunk_assignments_draft (long-delete fix, follow-up to #16)

Requires PR #992 to be merged first, OR testing against the #994 branch directly.

  • Open a session with a saved scenario containing many draft bunk assignments (ideally a populated session, not an empty one).
  • Delete the scenario. Time it roughly.
  • Expected: delete completes in ≲1 second (single server-side cascade).
  • Compare: before this PR, delete was multi-second due to N+1 frontend round-trips deleting each draft assignment individually.
  • Verify scenario is gone from the modal list and no orphan bunk_assignments_draft rows remain (spot-check via PocketBase admin or a query if available).
  • Create a new scenario and confirm it still works end-to-end (migration didn't break upstream writes).

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Warning

Rate limit exceeded

@adamflagg has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 40 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 40 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bab9b6d3-5b31-425e-8fab-edfb46ac4398

📥 Commits

Reviewing files that changed from the base of the PR and between 28d38df and 86e7630.

📒 Files selected for processing (9)
  • .pip-audit-ignore
  • frontend/src/components/ScenarioManagementModal.test.tsx
  • frontend/src/components/ScenarioManagementModal.tsx
  • frontend/src/contexts/ScenarioContext.test.tsx
  • frontend/src/contexts/ScenarioContext.tsx
  • frontend/src/hooks/useSavedScenariosMutation.test.ts
  • frontend/src/hooks/useSavedScenariosMutation.ts
  • frontend/src/hooks/useScenario.ts
  • pocketbase/pb_migrations/1500000098_bunk_assignments_draft_scenario_cascade_delete.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/scenario-cascade-delete

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@adamflagg adamflagg force-pushed the feature/fix-delete-scenario-render branch from daad911 to 8acf8e1 Compare April 25, 2026 03:23
@adamflagg adamflagg force-pushed the feature/scenario-cascade-delete branch from 63a831c to e54a286 Compare April 25, 2026 03:24
@adamflagg adamflagg force-pushed the feature/fix-delete-scenario-render branch from 8acf8e1 to 1432cb7 Compare April 25, 2026 05:39
@adamflagg adamflagg changed the base branch from feature/fix-delete-scenario-render to main April 25, 2026 05:42
adamflagg and others added 3 commits April 24, 2026 22:42
The ScenarioContext exposed a single `loading` flag that went true whenever
either the initial React Query fetch or any mutation (create/update/delete/
clear) was pending. ScenarioManagementModal used that flag to swap the
scenario cards for a "Loading scenarios..." placeholder, so clicking Delete
made the entire list vanish behind the confirmation dialog — leaving only
the hardcoded CampMinder card visible. The effect was pronounced because
deleting a scenario serially deletes every related draft assignment from
the frontend (cascadeDelete=false on bunk_assignments_draft.scenario), so
the placeholder window lasts several seconds on real sessions.

Split the provider into `isLoading` (initial query fetch) and `isMutating`
(any mutation pending), retaining `loading` as the combined flag for
backward compatibility. Updated the modal to consume `isLoading` only, so
the cards stay in place during delete/clear and the per-row "Deleting..."
button state remains the correct in-progress affordance.

The slow-delete itself is a separate schema issue tracked for a follow-up
PR — switching the relation to cascadeDelete:true will collapse the N+1
serial deletes into a single server-side cascade.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…elete

Follow-up to #992. The scenario delete mutation previously fetched every
draft assignment for a scenario and deleted them one-by-one from the
frontend (N+1 serial HTTP DELETEs) before deleting the scenario itself,
because bunk_assignments_draft.scenario had cascadeDelete=false in
migration 1500000022. On real sessions this took several seconds and
was the visible component of the "list vanishes behind the confirm
modal" report — #992 made the list stop vanishing, and this PR makes the
delete near-instant.

Changes:
- New migration 1500000098 flips cascadeDelete to true on
  bunk_assignments_draft.scenario (relation to saved_scenarios). All other
  relation properties preserved exactly as created in 1500000022.
- useDeleteScenario now issues a single DELETE against saved_scenarios;
  PocketBase cascades the draft rows server-side.
- New test pins the single-call behavior: draft collection is never
  queried or written during a scenario delete.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamflagg adamflagg force-pushed the feature/scenario-cascade-delete branch from e54a286 to fa5b9e7 Compare April 25, 2026 05:42
@adamflagg adamflagg enabled auto-merge (squash) April 25, 2026 05:42
…am fix

Affects pip 26.0.1 install-time handling of files identified as both tar
and ZIP. No runtime impact for installed packages; no fixed version
released yet. Tracking removal by 2026-05-25.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamflagg adamflagg merged commit b9bacea into main Apr 25, 2026
19 checks passed
@adamflagg adamflagg deleted the feature/scenario-cascade-delete branch April 25, 2026 05:55
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