✨ server: support many cards in the statement#893
Conversation
🦋 Changeset detectedLatest commit: aa5c7f8 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 |
WalkthroughThis PR refactors the statement generation flow to support multiple cards per account. It introduces a nested, card-centric data structure where purchases are grouped by card with lastFour identifier, updates the Statement React component's props to accept card arrays with nested purchases and payment information, and modifies the activity API to construct purchases data with card ID references for PDF and JSON statement outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant API as Activity API
participant DB as Database
participant Component as Statement Component
participant PDF as PDF/JSON Output
API->>DB: Query credentials with cards (id, lastFour)
DB-->>API: Cards with id & lastFour
API->>DB: Fetch transactions for each card
DB-->>API: Transactions data
API->>API: Build purchases per card<br/>(group by cardId)
API->>API: Create hash-to-cardId map<br/>(from purchases)
API->>API: Extract payments section<br/>(from response items)
API->>Component: Pass account, cards[{lastFour,<br/>purchases[]}], maturity,<br/>payments[]
Component->>Component: Calculate totals<br/>(totalSpent, totalPayments,<br/>dueBalance)
Component->>Component: Render per-card<br/>purchase tables
Component->>Component: Render payments section<br/>& summary
Component->>PDF: Serialize to PDF/JSON<br/>with multi-card layout
PDF-->>Component: Statement output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enables the generation of comprehensive user statements that can include transactions from multiple cards. The changes involve updating the data retrieval process in the API to fetch all relevant card data and refactoring the PDF rendering component to display this multi-card information in a structured and user-friendly format. This enhancement provides users with a more complete overview of their financial activity across all their associated cards. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 in PDF statements, which is a significant feature enhancement. The changes are well-implemented across the API, statement generation component, and tests. The refactoring in server/api/activity.ts to handle data fetching for multiple cards is more efficient than the previous implementation. The Statement.tsx component has been completely redesigned, resulting in a much-improved and more professional-looking statement PDF. The accompanying test updates are thorough. I have one minor suggestion to improve code maintainability. Overall, this is an excellent contribution.
| export function format(timestamp: number) { | ||
| return new Date(timestamp * 1000).toISOString().slice(0, 10); | ||
| return new Date(timestamp * 1000).toLocaleDateString("en-US", { month: "short", day: "2-digit", year: "numeric" }); | ||
| } | ||
|
|
||
| function formatDate(iso: string) { | ||
| return new Date(iso).toLocaleDateString("en-US", { month: "short", day: "2-digit", year: "numeric" }); | ||
| } |
There was a problem hiding this comment.
To improve maintainability and reduce code duplication, you can extract the Intl.DateTimeFormatOptions into a constant, as both format and formatDate functions use the same formatting options.
const dateFormatOptions: Intl.DateTimeFormatOptions = { month: "short", day: "2-digit", year: "numeric" };
export function format(timestamp: number) {
return new Date(timestamp * 1000).toLocaleDateString("en-US", dateFormatOptions);
}
function formatDate(iso: string) {
return new Date(iso).toLocaleDateString("en-US", dateFormatOptions);
}
|
✅ All tests passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10f80b4e-01f7-4d80-a553-f9c0d21d0758
📒 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
| {cards.map((card) => { | ||
| const cardTotal = card.purchases.reduce( | ||
| (sum, p) => sum + p.installments.reduce((a, installment) => a + installment.amount, 0), | ||
| 0, | ||
| ); | ||
| return ( | ||
| <View key={card.lastFour} style={styles.summaryRow}> | ||
| <Text style={styles.summaryLabel}>Card **** {card.lastFour} purchases</Text> | ||
| <Text style={styles.summaryAmount}>{currency.format(cardTotal)}</Text> | ||
| </View> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate cardTotal calculation; consider extracting.
The same cardTotal calculation appears at lines 83-86 and 106-109. Consider computing card totals once during the initial data processing to avoid duplication.
Additionally, using card.lastFour as the React key could cause issues if two cards share the same last four digits (rare but possible). Consider using a more unique identifier if available.
♻️ Suggested refactor
- const totalSpent = cards.reduce(
- (sum, card) =>
- sum +
- card.purchases.reduce((s, p) => s + p.installments.reduce((a, installment) => a + installment.amount, 0), 0),
- 0,
- );
+ const cardsWithTotals = cards.map((card) => ({
+ ...card,
+ total: card.purchases.reduce((s, p) => s + p.installments.reduce((a, i) => a + i.amount, 0), 0),
+ }));
+ const totalSpent = cardsWithTotals.reduce((sum, card) => sum + card.total, 0);Then use cardsWithTotals in both the summary and per-card sections, referencing card.total directly.
1 participant
Summary by CodeRabbit