✨ server: support multiple cards in statement#920
Conversation
🦋 Changeset detectedLatest commit: 3639b18 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughRefactored activity API and statement generation to support multiple cards per account. Changed transaction querying from two-step process to eager loading with filtered joins. Updated statement data structure to include card-specific purchases and payments, and modified PDF generation to handle multiple cards with grouped transaction data. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for multiple cards within the generated PDF statements. Key changes include refactoring the activity API to group transactions by card using Map.groupBy, updating the Statement component with a professional table-based layout and SVG branding, and enhancing the test suite to cover multi-card scenarios and financial calculations like discounts and penalties. I have no feedback to provide.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/test/api/activity.test.ts (1)
257-295: 🧹 Nitpick | 🔵 TrivialAssert the PDF grouping, not just the MIME type.
The new
purchasesByCardlogic only runs in this PDF path, but these tests still stop atcontent-typeandbyteLength. A regression that drops one card or groups every purchase under the same card would still pass.Please assert both last-fours are present in the rendered output, or mock
Statementand inspect the props passed intorenderToBuffer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 141b917e-cca7-4de8-9649-498061fb41a4
📒 Files selected for processing (5)
.changeset/soft-trains-dig.mdserver/api/activity.tsserver/test/api/activity.test.tsserver/test/utils/statement.test.tsserver/utils/Statement.tsx
| const hashes = borrows | ||
| .entries() | ||
| .filter(([_, { events }]) => events.some(({ maturity: m }) => Number(m) === maturity)) | ||
| .map(([hash]) => hash) | ||
| .toArray(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== runtime targets =="
fd '^(package\.json|\.nvmrc|\.tool-versions)$' -x sh -c 'echo "--- $1"; cat "$1"' sh {}
fd '.*\.(yml|yaml)$' .github/workflows -x sh -c 'echo "--- $1"; rg -n "setup-node|node-version|bun-version|deno-version" "$1" || true' sh {}
echo
echo "== typescript target/lib =="
fd '^tsconfig.*\.json$' -x sh -c 'echo "--- $1"; jq ".compilerOptions | {target, lib, module}" "$1"' sh {}
echo
echo "== usages introduced in this path =="
rg -n '\.toArray\(|\bMap\.groupBy\b' --type ts --type tsxRepository: exactly/exa
Length of output: 33794
🏁 Script executed:
# Read tsconfig files directly (jq syntax was incorrect)
cat server/tsconfig.json
echo "---"
cat tsconfig.jsonRepository: exactly/exa
Length of output: 1627
🏁 Script executed:
# Read the actual code at the specified lines
echo "=== activity.ts lines 265-285 ==="
sed -n '265,285p' server/api/activity.ts
echo ""
echo "=== activity.ts lines 423-533 (first 50 lines) ==="
sed -n '423,473p' server/api/activity.tsRepository: exactly/exa
Length of output: 3575
🌐 Web query:
Node.js 24.13.0 ES2025 Map.groupBy iterator helpers support
💡 Result:
Node.js 24.13.0 fully supports Map.groupBy as it uses V8 13.6 which includes ES2024 features like Map.groupBy (stable since Node.js 21). However, ES2025 iterator helpers (e.g., Iterator.prototype.map, .filter, etc.) are not supported. V8 shipped iterator helpers in v12.2 (Chrome 122), but Node.js 24's V8 13.6 does not list them as shipped features, and sources indicate no Node.js support as of V8 12.2 docs. No harmony flags needed for Map.groupBy; iterator helpers remain experimental/post-ES2025.
Citations:
- 1: https://nodejs.org/en/download/archive/v24.13.0
- 2: https://nodejs.org/blog/release/v24.0.0
- 3: 2025-05-06, Version 24.0.0 (Current) nodejs/node#57609
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/groupBy
- 5: javascript.builtins.Object.groupBy - Now supported by node.js mdn/browser-compat-data#21429
- 6: http://www.v8.dev/features/iterator-helpers
- 7: https://caniuse.com/mdn-javascript_builtins_map_groupby
- 8: https://blog.logrocket.com/iterator-helpers-es2025
- 9: https://nodesource.com/blog/Node.js-version-24
- 10: https://docs.redhat.com/en/documentation/red_hat_build_of_node.js/24/html-single/release_notes_for_node.js_24/index
Iterator helpers (.entries().filter().map().toArray() and .values().toArray()) are not supported in Node.js 24.13.0 and will fail at runtime.
This code uses ES2025 iterator helpers despite the runtime targeting Node.js 24.13.0. While Map.groupBy() is available (ES2024, stable in Node.js 21+), the iterator methods on lines 268–272 and 429–463 will throw at runtime. TypeScript with target: "ESNext" allows these patterns to type-check but does not transpile them to equivalent ES2024 code.
Rewrite using .entries().toArray().filter().map() (convert iterator to array first) or use traditional Array.from() and array methods.
| cards: purchases | ||
| .filter(({ id }) => purchasesByCard.has(id)) | ||
| .map(({ id, lastFour }) => ({ | ||
| id, | ||
| lastFour, | ||
| purchases: (purchasesByCard.get(id) ?? []).map(({ cardId: _, ...rest }) => rest), | ||
| })), |
There was a problem hiding this comment.
Sort statement.cards before rendering the PDF.
Once an account has multiple cards, relying on query result order makes the summary and card sections unstable across requests. Sorting here keeps the PDF deterministic.
Proposed fix
cards: purchases
.filter(({ id }) => purchasesByCard.has(id))
+ .toSorted((a, b) => a.lastFour.localeCompare(b.lastFour) || a.id.localeCompare(b.id))
.map(({ id, lastFour }) => ({
id,
lastFour,
purchases: (purchasesByCard.get(id) ?? []).map(({ cardId: _, ...rest }) => rest),
})),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cards: purchases | |
| .filter(({ id }) => purchasesByCard.has(id)) | |
| .map(({ id, lastFour }) => ({ | |
| id, | |
| lastFour, | |
| purchases: (purchasesByCard.get(id) ?? []).map(({ cardId: _, ...rest }) => rest), | |
| })), | |
| cards: purchases | |
| .filter(({ id }) => purchasesByCard.has(id)) | |
| .toSorted((a, b) => a.lastFour.localeCompare(b.lastFour) || a.id.localeCompare(b.id)) | |
| .map(({ id, lastFour }) => ({ | |
| id, | |
| lastFour, | |
| purchases: (purchasesByCard.get(id) ?? []).map(({ cardId: _, ...rest }) => rest), | |
| })), |
| const totalPayments = payments.reduce((sum, p) => sum + p.positionAmount, 0); | ||
| const dueBalance = totalSpent - totalPayments; |
There was a problem hiding this comment.
Show the actual payment cash flow, not the relieved balance.
Keep positionAmount for dueBalance, but the rows labeled "Payments" should render amount; otherwise an early-payment discount can show up as a larger payment than the user actually made.
Proposed fix
- const totalPayments = payments.reduce((sum, p) => sum + p.positionAmount, 0);
- const dueBalance = totalSpent - totalPayments;
+ const totalApplied = payments.reduce((sum, p) => sum + p.positionAmount, 0);
+ const totalPaid = payments.reduce((sum, p) => sum + p.amount, 0);
+ const dueBalance = totalSpent - totalApplied;
...
- <Text style={styles.summaryAmount}>-{currency.format(totalPayments)}</Text>
+ <Text style={styles.summaryAmount}>-{currency.format(totalPaid)}</Text>
...
- <Text style={styles.colTotal}>-{currency.format(payment.positionAmount)}</Text>
+ <Text style={styles.colTotal}>-{currency.format(payment.amount)}</Text>
...
- <Text style={styles.totalAmount}>-{currency.format(totalPayments)}</Text>
+ <Text style={styles.totalAmount}>-{currency.format(totalPaid)}</Text>Also applies to: 113-118, 170-198
1 participant
Summary by CodeRabbit
Release Notes