Write down what the closed payment rewrite taught us

[stefan-burke]

The payment-aggregate rewrite (#1962 and #1973) was closed without merging. Its record of what review and testing found lived only in TODO.md on the closed branch, so main had none of it. This adds one section to main's TODO.md so that knowledge is not lost.

Every item was checked against main's current code before being added, so the section only carries what is relevant here:

• One real job on main. performListingDelete removes the stored attachment file before the database delete. That delete can genuinely fail — a busy database under a concurrent write is a normal failure — and then the file is gone while the listing stays, showing an attachment link that no longer works. The fix is to swap the order.
• The failure shapes the planned redo must design out, condensed to five one-line lessons — the "Payment aggregate — safety behaviour (PR 1)" section directly above already plans that redo, and these are the ways the first attempt went wrong.
• The candidates checked against main and found not to apply, written down so nobody re-raises them: SumUp converts money once at checkout creation, emails send inline, the attendee merge already fences live payment rows, Square's order id is already optional, and the success redirect already clears its tokens.
• Where the full record lives: TODO.md at commit 3cf6929b on base/payment-aggregate (also refs/pull/1962/head), with the table-hardening slice at 369977cf (refs/pull/1973/head).

No code changes — one markdown file. deno fmt, lint, typecheck, the copy checks and the duplication check all pass locally; the test suite does not read TODO.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JxEwtb9N1DGK1ks2VM3345

---

Generated by Claude Code

Summary by CodeRabbit

• Documentation
  • Added guidance from the completed payment-aggregate rewrite.
  • Documented a key data-handling lesson: remove database listings before attachment files.
  • Captured failure modes, non-applicable findings, and process guidance for future rewrites.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 529a3f3a-4443-4919-a391-d3ac58975e94

📥 Commits

Reviewing files that changed from the base of the PR and between 83936ee and 12ae688.

📒 Files selected for processing (1)

• TODO.md

Included review availability: 1 review is currently available. Based on recent review activity, included reviews refill at 4 per hour.

---

📝 Walkthrough

Walkthrough

The pull request adds a TODO section that records lessons from the closed payment-aggregate rewrite, including attachment-deletion ordering, ruled-out findings, and process guidance.

Changes

Payment aggregate lessons

Layer / File(s)	Summary
Document rewrite findings TODO.md	Adds the payment aggregate lessons section, including the attachment-deletion ordering defect, failure patterns, validated findings, and rewrite process guidance.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 12ae6

This change adds documentation to TODO.md only and does not alter application behavior or production configuration; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes documenting lessons from the closed payment rewrite, which is the main purpose of the changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/payment-aggregate-findings

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/payment-aggregate-findings

---

_{Comment @coderabbitai help to get the list of available commands.}