Repository navigation
fix(plans): 404 missing plan and reconcile stale scheduled-purchase Edit (#1403) - #1425
Merged
Merged
Conversation
…dit (#1403) Editing a scheduled purchase whose plan no longer exists surfaced a generic "Failed to load plan details" and left the orphaned row on screen. The PR #780 fix (Edit passes plan_id, not the execution id) is intact and verified; the residual cause of the identical error is a scheduled-purchase row that outlived its plan: the plan was deleted (ON DELETE SET NULL detaches the execution) or the caller's account scope changed. Clicking Edit then loads GET /plans/{id}, which failed in two ways: - Backend: getPlan returned GetPurchasePlan's wrapped config.ErrNotFound raw. The router's IsNotFoundError only matches the api-package sentinel, so a missing plan surfaced as 500 instead of 404 (unlike updatePlan / patchPlan). Route it through mapCreatePlanStorageError so it maps to 404. - Frontend: editPlan swallowed the failure with a generic toast and never reconciled the list. It now returns a success boolean, shows an actionable message on a 404, and the scheduled-purchase Edit handler refetches the planned-purchases list so the orphaned row is dropped. Regression tests at both layers fail on the pre-fix code and pass after.
cristim
added
triaged
Member
Author
|
@coderabbitai review |
Member
Author
|
Merged to main (all CI green after the tflint-503 outage cleared) closes #1403. Edit scheduled purchase - getPlan now returns 404 (not 500) for a missing plan and the frontend reconciles the stale list; adversarial-reviewed SHIP. |
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.
What
Editing a scheduled (planned) purchase whose backing plan no longer exists showed a generic
Failed to load plan detailstoast and left the orphaned row on screen (closes #1403).Root cause
The PR #780 fix (the Edit button passes
plan_id, not the execution's own id) is intact and verified —getPlanis called with the correct plan FK. The residual cause of the identical error is a scheduled-purchase row that outlived its plan:purchase_executions.plan_idON DELETE SET NULL, detaching the execution), orClicking Edit then loads
GET /plans/{id}, which failed in two places:Backend —
getPlanreturnedGetPurchasePlan's wrappedconfig.ErrNotFoundraw. The router'sIsNotFoundErroronly matches the api-package*notFoundErrorsentinel, so a missing plan surfaced as a 500 instead of a 404 (unlike sibling handlersupdatePlan/patchPlan/getPlanForPurchaseCreation, which route throughmapCreatePlanStorageError). This change routesgetPlanthrough the same helper.Frontend —
editPlanswallowed the failure with a generic toast and never reconciled the list, leaving a dead Edit button. It now returns a success boolean, shows an actionable message on a 404 (This plan is no longer available. It may have been deleted.), and the scheduled-purchase Edit handler refetches the planned-purchases list so the orphaned row is dropped. Other errors (network blip, transient 5xx) keep the generic message since the plan may still exist.Tests (fail before, pass after — verified by stashing each fix)
internal/api:TestHandler_getPlan_NotFound_MapsTo404— a missing plan maps to a 404 ClientError, not a raw 500.frontend:edit action reconciles the list when the plan is gone (#1403)— asserts the correctplan_idis still used, the actionable 404 message fires, andgetPlannedPurchasesis refetched to drop the stale row.Gates
go build ./.../go vet ./...: passgo test ./internal/api/...: pass (1724)gocyclo -over 10: clean on changed codegolangci-lint run ./...(pinned CI v2.10.1): 0 issuesnpm test: 2568 passed;npm run build: pass;npm run lint: 0 errors