Move bulk refund result messages into locale catalog - #1901
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughBulk attendee-refund result messages now use translated segments for success, failure, error, remaining, and completion states. Locale entries and refund tests assert exact copy, no-payment handling, and activity-log behavior. ChangesAttendee refund messaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
f70a1f9 to
4a3f194
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a3f194311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| t("admin.attendees.refund_all_result_failures", { | ||
| count: problemCount, |
There was a problem hiding this comment.
Separate errored refunds from failed count
When any refund errors, problemCount already includes those errors, and the next clause appends errorCount as its own sentence. A batch with one declined refund and one thrown or ledger error now tells the operator “There were 2 failures. There was 1 error”, which reads like three problematic payments even though only two refunds failed. Use failedCount for the failures clause, or make the first clause say total problems, so the numbers shown to the person stay accurate.
AGENTS.md reference: AGENTS.md:L196-L198
Useful? React with 👍 / 👎.
What changed
Bulk refund result messages used to be hard-coded English strings inside the route handler. They now live in the locale catalog as ICU plural messages, so every surface that shows them stays worded the same way and can be translated.
Why
A person running refunds sees a result like "2 refunds succeeded. There was 1 failure. Some payments may have already been refunded." on the admin page. That copy was built from inline string concatenation in the route, so it could not be translated and had no single source of truth. Moving it into the catalog means the wording is defined once and reused everywhere, the plural forms are correct for any count, and the copy-check and translation-coverage gates cover it like every other user-facing string.
What it does
t(), assembled withcompact()so the error clause only appears when there are errors.I18N_REPLACEMENTSround-trip that proves the route reads from the catalog rather than hard-coding.expectPartialRefundtest helper.refundError().Verification
deno task precommit(typecheck, lint, cpd, check:copy, build:edge, test:coverage): greensrc/features/admin/attendee-refunds.ts: 84 killed, 1 documented equivalentSummary by CodeRabbit
Bug Fixes
New Features
Tests