✨ server: support many cards in the statement#893
Conversation
🦋 Changeset detectedLatest commit: c33064d 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReworks statement generation to support multiple cards: Activity API now queries credential cards and nests transactions as purchases per card, extracts payments separately, and the Statement component signature/layout changed to accept account, cards (with purchases), maturity, and payments for PDF/JSON output. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Activity API
participant DB as Database
participant Statement as Statement Component
participant Output as PDF/JSON Output
Client->>API: Request statement (account, format, maturity)
API->>DB: Query credential cards (id, lastFour) for account
DB-->>API: Cards list
API->>DB: Query transactions/items (filter by hashes/credential)
DB-->>API: Transactions/items
API->>API: Attach cardId to items, group purchases by card, extract payments
API->>Statement: Send payload {account, cards[{id,lastFour,purchases[]}], maturity, payments}
Statement->>Statement: Compute totals, format dates/currency, render pages
Statement->>Output: Emit PDF or JSON
Output-->>Client: Return statement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #893 +/- ##
==========================================
+ Coverage 71.69% 71.76% +0.07%
==========================================
Files 228 228
Lines 8277 8285 +8
Branches 2661 2657 -4
==========================================
+ Hits 5934 5946 +12
Misses 2113 2113
+ Partials 230 226 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
server/utils/Statement.tsx (2)
99-111: 🧹 Nitpick | 🔵 TrivialDuplicate
cardTotalcalculation; consider extracting.The same
cardTotalreduction appears at lines 100-103 and 125-127. Pre-compute card totals once to avoid duplication and improve maintainability.♻️ Suggested refactor
+ 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); - const totalSpent = cards.reduce( - (sum, card) => - sum + - card.purchases.reduce((s, p) => s + p.installments.reduce((a, installment) => a + installment.amount, 0), 0), - 0, - );Then use
cardsWithTotalsin both the summary and per-card sections, referencingcard.totaldirectly.Also applies to: 124-128
105-105:⚠️ Potential issue | 🟡 MinorConsider a more unique React key.
Using
card.lastFouras the key could cause issues if two cards share the same last four digits. Since the component doesn't receive a unique card ID, consider using the array index as a fallback:-<View key={card.lastFour} style={styles.summaryRow}> +<View key={`${card.lastFour}-${index}`} style={styles.summaryRow}>Apply similarly at line 130.
Also applies to: 130-130
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85ea069e-c24d-4dbc-919a-71cb4c2744bf
📒 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
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/utils/Statement.tsx (1)
99-128: 🧹 Nitpick | 🔵 TrivialDuplicate
cardTotalcalculation.The same calculation appears at lines 100-103 (Summary section) and lines 125-128 (per-card section). Consider computing card totals once during initial data processing.
Additionally, using
card.lastFouras the React key (lines 105, 130) could cause issues if two cards share the same last four digits.♻️ Suggested refactor
+ 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); - const totalSpent = cards.reduce( - (sum, card) => - sum + - card.purchases.reduce((s, p) => s + p.installments.reduce((a, installment) => a + installment.amount, 0), 0), - 0, - );Then reference
cardsWithTotalsandcard.totaldirectly in both sections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 17e1f6dc-adbd-449f-9f30-770174c13e19
📒 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
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nfmelendez
force-pushed
the
statement
branch
2 times, most recently
from
6842c51 to
a0e9a6c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🚩 tsconfig.json include list doesn't explicitly cover .tsx files
The server tsconfig.json include patterns use **/*.ts (e.g., utils/**/*.ts) which does not glob-match .tsx files. Statement.tsx is the only .tsx file in the server package. It is still type-checked because it's transitively imported from api/activity.ts, but adding utils/**/*.tsx to the include list would make this explicit and prevent the file from being silently excluded if the import chain were to change.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/utils/Statement.tsx (1)
101-113: 🧹 Nitpick | 🔵 TrivialDuplicate
cardTotalcalculation still present.This was flagged in a previous review. The same reduction logic appears twice (lines 102-105 and 127-130). Consider computing card totals once during initial processing.
♻️ Optional refactor
+ 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 = cards.reduce( - (sum, card) => - sum + - card.purchases.reduce((s, p) => s + p.installments.reduce((a, installment) => a + installment.amount, 0), 0), - 0, - ); + const totalSpent = cardsWithTotals.reduce((sum, card) => sum + card.total, 0);Then use
cardsWithTotalsandcard.totalin both the summary and per-card sections.Also applies to: 126-130
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad2f03be-c2dd-47f2-929d-8c5c4d3bc5fd
📒 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
1 participant
closes #880
Summary by CodeRabbit
New Features
UI
Tests