fix: create todo when empty by dudenamedjune · Pull Request #6 · aws-samples/amplify-vite-react-template

dudenamedjune · 2024-06-01T20:17:24Z

Description of changes:
Changed template so that when a user cancels or submits it does not create a todo when empty.

…ancel for real Top 3 from docs/business-review-2026-07-15.md, after verifying each against the code. (#2 on that list — internal notes on the customer PDF — shipped in 49f2c23.) 1. Recurring plans never started billing ($1,188/yr per customer) startSubscription had exactly one caller in the repo: a small button in CustomerDetail. Neither completion path called it, so the business's own locked rule — "$99 at booking, monthly starts after the first visit completes" — existed only as a comment. A website-booked customer paid $99, got serviced every 30 days forever, and was never charged again, while the Dashboard counted them healthy. Both completion paths (finalizeServiceReport and completeJob) now start billing. It is idempotent, so later visits are a no-op rather than a second subscription, and it never throws: the visit really happened and must stand even with no card on file. A plan that could not start stays ACTIVE with no subscription and is listed by a new Dashboard card, "Serviced but not billing" — the query the review noted did not exist anywhere. That card deliberately excludes plans whose first visit hasn't happened; those are *supposed* to be unbilled, and listing them would bury the real ones. 2. Every paid website booking looked unpaid, and the CRM offered to charge it again bookingFinalize created the Customer, Plan, Job and Agreement but never an Invoice — so the duplicate-charge guard queried an empty table and always passed, and the Charge button's `!invoice` test was permanently blind. Completing a paid $299 wasp job put a "Charge $299" button on the record with no confirmation. Job now carries paidAt/paidPaymentIntentId, written in the same create as the job, and that is what every charge path refuses on — a field that cannot go missing the way a ledger row can. chargeOneTimeJob refuses server-side, the button is hidden, and the row shows a "paid $299 online" badge. finalizeBooking also writes the PAID Invoice so funnel revenue reaches the ledger; it uses a booking-derived id so a retry is a no-op, and deliberately does not throw — the finalization claim is released on any error and none of the creates above it are idempotent, so throwing would have Stripe retry and duplicate the customer. 3. Cancelling did not stop the billing The public cancel path marked the plan CANCELED and never called Stripe. Visits stopped, charges continued, and the agreement says "cancel anytime" — that is the fact pattern ROSCA exists for. Plan cancellation now lives in one place (shared/subscription.ts) that both the office mutation and the funnel call. Stripe goes first: if it fails we throw and leave the plan ACTIVE, which is visibly wrong and retried, rather than marking it cancelled while the card keeps being charged. Also: a plan cancelled from the Stripe dashboard kept a dead subscription id, which read as healthy everywhere. The webhook now clears it, and startPlanBilling checks CANCELED before the id short-circuit. NOT DONE — review item #6 is a misread, do not action it It claims quarterly/bimonthly plans are "billed twelve times a year — systematic overcharging", assuming $45 is per-visit. rateCards.ts:7 says otherwise in its own header ("quarterly residential -> $45 + 3x$10 = $75/mo"), the field is monthlyCents, and bookingFinalize discloses "continues quarterly at $45.00/month". Plans are priced per month whatever the visit cadence; billing quarterly plans every three months would cut that revenue by two thirds. The interval is unchanged and now carries a comment and a test saying why. The review's underlying point — a CSR cannot tell how often the card is charged — is real and still open. Verified with a probe against a fake Stripe: 14 assertions covering the start, idempotency, the no-card path, cancel-calls-Stripe, cancel-fails-safe, and the monthly interval. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…cellation Commit D from docs/business-review-2026-07-15.md rev 3 — the four residuals the review opened against 372f30b. All four are cases where 372f30b made the right call and then stopped one step short of it mattering. 1. The fourteen assertions are now in the repository (44 of them) 372f30b's message claimed the monthly-billing rule "carries a comment and a test saying why." The comment was real; the test was a scratch file that was run and discarded. That claim was wrong and the review was right to call it. vitest is wired into apps/web, `npm test` runs it, and amplify.yml runs it in the backend phase *before* pipeline-deploy — so a broken money rule fails the build instead of reaching production. 44 tests across billing lifecycle, lead capture, the not-billing digest, and cancellation. Each was mutation-checked: reintroducing the exact bug it guards makes it fail. Reinstating the review's own retracted item #6 (bill quarterly plans every three months) now fails two tests by name. That check found a real hole in my first draft — asserting `interval === "month"` alone passes an every-3-months subscription, because interval_count is the half that moves the cadence. Both are asserted now. 2. Two silent failures, in the class 49f2c23 set out to eliminate Not throwing was correct in both — throwing would make Stripe retry a chain of non-idempotent creates and duplicate the customer. Telling nobody was a separate, wrong decision, and the review separated them correctly. `notifyOffice` moves to shared/email.ts and now pages a human when a paid booking's invoice cannot be written ("their money exists only in Stripe — record it by hand, do not charge the card again") and when a serviced plan fails to start billing. Both name the customer and say what to do. Not used for email failures: routing an alarm through the subsystem it reports on is how alarms go unheard. 3. The queue announces itself "Serviced but not billing" only existed for whoever opened the Dashboard. The daily digest now reports serviced plans with no subscription, totalled in annual dollars. It excludes plans whose first visit hasn't happened — those are supposed to be unbilled, and listing them is how a digest teaches people to ignore it. 4. A failed cancellation no longer costs the customer their refund 372f30b made this endpoint honest, and honesty needed a script. It now records the date the customer FIRST asked to cancel, and refundability is judged from that date, not from when the cancellation finally succeeded — an attempt that fails on day four because Stripe is down must not forfeit their refund when it retries on day three. The customer gets the truth and a phone number instead of "please try again", the office is paged to finish it by hand and honour the original date, and the refund call is idempotency-keyed so a retry cannot double-refund. Also fixed while here: booking-public's local notifyOffice returned silently when SES_NOTIFY_EMAIL was unset — a quiet failure in the alerting path itself. A test caught it. Verified: 44 tests, both apps typecheck and build, mutation checks confirm the tests fail when their bug returns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ller ones Verified 3d13b00 against the tree, not against its message. All four residuals the review opened against 372f30b are genuinely closed. - 44 tests, 4 files, all passing. npm test runs in amplify.yml:12 in the backend phase before pipeline-deploy, so a broken money rule fails the build. - I reintroduced retracted item #6 against the current tree (interval_count: 3 for quarterly plans) and the suite failed exactly two tests, by the names the commit message claims. Restored; suite green. The guard is real. - notifyOffice pages a human for the missing ledger row (bookingFinalize.ts:250) and the plan that failed to start billing (crm-docs:366). - The daily digest reports serviced-but-not-billing in annual dollars, correctly excluding plans whose first visit has not happened. - cancelRequestedOn is persisted before Stripe is called and refundability is judged from it, so an outage on day four cannot cost the customer their refund on a day-three retry. Refund is idempotency-keyed. Customer gets a 503 with a phone number instead of "please try again", office is paged with the date to honour. Credit: mutation-checking their own tests found a hole this review would have missed — asserting interval === "month" alone passes an every-3-months subscription, because interval_count is the half that moves the cadence. Both are asserted now. Second time this team has improved on a recommendation rather than merely executing it. Three new findings, all small, none undoing the above: 1. There are now three notifyOffice implementations, not one. The commit says it "moves to shared/email.ts"; it was added there, and the copies at booking-public/handler.ts:771 and lead-intake/handler.ts:152 remain, with different failure semantics. The shared one logs "nobody was told" when SES_NOTIFY_EMAIL is unset; lead-intake's silently falls back to info@. This collides with Commit M — routing lead mail to sales@ touches a path that does not go through the shared helper. Consolidate before doing M. 2. The write that records cancelRequestedOn sits outside the try. If that Dynamo update fails, the endpoint returns the generic "please try again", no date is recorded and no office alert fires — so the day-three retry loses the refund the commit just protected. Same failure class, one line above the fix. 3. apps/crm has no test harness and its CI stage runs only npm run build. Items 11 and 12 live there. Commit I will have nowhere to put a test that CI runs. The suite now guards what was reviewed, not what is reachable: none of the items still bleeding today has a test, and three of them are in the app with no harness. Stand up vitest in apps/crm before Commit I. Next: Commit E, "Refunds exist" — the live half of item 4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

INVENTORY item #6. The roster ladder had three branches — loading, empty, table — so a failed read fell through to `users.length === 0` and rendered "No users found.": an outage presented as an empty organisation, on the one screen where "there are no users" is never true of a signed-in admin's own team. The inventory undersold this. The error was not missing, it was in the wrong card. `team.error` was rendered in the *invite* form's action row beside SaveStatus, where a roster-read failure reads as "your invite failed" while the roster card below it claims the team is empty. So this is a move rather than an addition — adding the branch and leaving the old line would report one failure twice, in two cards, with two different meanings. The branch goes in the order PATTERNS specifies: loaded, then error, then empty, then content. The profile read at :65 still deliberately does not surface its error — the roster renders without names and signatures, and that call is already documented in place. Verified by test, not in a browser: the CRM is behind Cognito magic-link auth and cannot be driven without a real sign-in, so Team.test.tsx follows MarketingTasks.test.tsx as the template and asserts all four states. It exercises the errors-array path rather than a rejection, because client.queries.* reports failure by resolving — the unwrap at :53 is what turns that into a throw, and it is the failure mode this screen actually sees. Both new assertions were proven by mutation rather than trusted. Removing the error branch fails two tests with "No users found." on screen, which is the reported bug reproduced. Restoring the duplicate in the invite row fails the count assertion with "expected [ …(2) ] to have a length of 1 but got 2". Neither test can pass against the code it was written to reject. 378 tests, typecheck and typecheck:backend clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: create todo when empty

71ed1bf

Navigation Menu

Uh oh!

fix: create todo when empty - #6

fix: create todo when empty#6
dudenamedjune wants to merge 1 commit into
aws-samples:mainfrom
dudenamedjune:fix-create-todo

dudenamedjune commented Jun 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

dudenamedjune commented Jun 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant