Conversation
🦋 Changeset detectedLatest commit: fa184f4 The changes in this PR will be included in the next version bump. 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:
WalkthroughRestructures payment UI: removes legacy pay-mode components, adds a new pay folder (Pay, Repay, Calculator, Overdue/UpcomingPayments, PaymentSheet updates), centralizes card-mode mutation options, updates navigation (tab icons/titles and haptics), adds i18n keys, and updates Maestro subflows and changesets. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Home
participant Calculator
participant PaymentSheet
participant Server
User->>Home: Open Payments screen
Home->>Server: Fetch market data & maturities
Server-->>Home: Market data + maturities
Home->>Home: Aggregate & sort maturities
User->>Home: Tap "Installments calculator"
Home->>Calculator: Navigate to /calculator
User->>Calculator: Enter amount
Calculator->>Server: Fetch installment rates
Server-->>Calculator: Rates
Calculator-->>User: Show installment options (BEST APR)
User->>Home: Tap payment item
Home->>PaymentSheet: Open details
PaymentSheet->>Server: Submit repay/rollover mutation
Server-->>PaymentSheet: Updated card details
PaymentSheet->>Home: Refresh & close
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)
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 @dieguezguille, 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 introduces a comprehensive overhaul of the application's payment management features. It focuses on enhancing the user experience by providing a more intuitive and feature-rich payments screen, including a dedicated calculator for installment planning. The changes also streamline underlying logic for card mode management and refresh the application's navigation aesthetics. 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.
❌ 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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
home
branch
4 times, most recently
from
84b9ea9 to
943fa36
Compare
dieguezguille
force-pushed
the
home
branch
2 times, most recently
from
6e34c64 to
89412ce
Compare
dieguezguille
force-pushed
the
payments
branch
from
c90c1ce to
5977ffd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
home
branch
5 times, most recently
from
e574d09 to
1b889c2
Compare
dieguezguille
force-pushed
the
payments
branch
from
5977ffd to
161dcd6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
161dcd6 to
29e6706
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
29e6706 to
d14b4ad
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
payments
branch
from
d14b4ad to
c991ea7
Compare
![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.
Additional Comments (2)
Add the missing key to "{{date}}, {{amount}}": "{{date}}, {{amount}}"The same issue exists at
Wrap in a try-catch (or validate before calling) to prevent the crash: |
dieguezguille
force-pushed
the
home
branch
5 times, most recently
from
fe79b7b to
536e90b
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Additional Comments (3)
Two aria-label translation keys used in the redesigned payment lists are absent from
Both need Spanish translations added to
When the user clears the field (empty string), |
![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.
Additional Comments (5)
Consider adding an Then in PaymentSheet, check the action parameter to determine whether to show the rollover intro immediately.
Add a
The template string uses "Payment due {{date}}", but
Wrap the map construction in
Trace: "abc" → Guard against empty or non-numeric strings before parsing: |
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.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cde16ee9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!rolloverIntroShown && onRolloverIntro && displayMaturity) { | ||
| close(); | ||
| onRolloverIntro(displayMaturity); |
There was a problem hiding this comment.
The intro gate now depends on onRolloverIntro, so call sites that render PaymentSheet without that prop (for example the loans flow) bypass the first-time rollover intro entirely when rolloverIntroShown is false. Previously the sheet always showed the intro before rollover, so this changes user flow and skips the acknowledgment path in those contexts.
Useful? React with 👍 / 👎.
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.
2 participants
Summary by CodeRabbit
New Features
UI/UX Improvements
Refactor
Documentation
Greptile Summary
This PR revamps the Payments screen with per-payment detail cards, early-repayment discount/late-fee info sheets, and a redesigned payment list with clearer dates and amounts. The core payment logic (
Repay.tsx,RepayAmountSelector.tsx) is solid, and thecardModeMutationOptionsrefactor (server.ts) cleanly centralizes optimistic-update logic.However, three issues require attention:
RolloverIntroSheet.tsx:setStateis called directly during render (lines 43–46) instead of insideuseEffect, inconsistent withPaymentSheet.tsxand risky under concurrent rendering.isLatestPluginguard for returning users: The plugin-version check was moved intoRolloverIntroSheetbut is skipped whenrolloverIntroShown=true. Returning users navigate to/roll-debtwithout the guard, creating a gap if/roll-debtlacks its own check."Payment due {{date}}, {{amount}}"and"Overdue payment {{date}}, {{amount}}"are used in code but not translated ines.json, affecting screen-reader users.Confidence Score: 2/5
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD TAB[Payments Tab] --> PAY[Pay.tsx] PAY --> EMPTY{Has payments?} EMPTY -- No --> EMPTYVIEW[Empty.tsx] EMPTY -- Yes --> HEADER[TotalOutstandingCard] HEADER --> FIRST[FirstMaturityCard earliest maturity] FIRST --> OVERDUE[OverduePayments excludeMaturity=first] OVERDUE --> UPCOMING[UpcomingPayments excludeMaturity=first] FIRST -- Pay --> REPAY[Repay.tsx] FIRST -- Rollover introShown=false --> ROLLINTRO[RolloverIntroSheet] FIRST -- Rollover introShown=true --> ROLLDEBT[roll-debt screen] ROLLINTRO -- isLatestPlugin true --> ROLLDEBT ROLLINTRO -- isLatestPlugin false --> TOAST[Upgrade toast] OVERDUE -- onSelect --> SHEET[PaymentSheet maturity param] UPCOMING -- onSelect --> SHEET SHEET -- Pay --> REPAY SHEET -- Rollover introShown=false --> PAY_ROLLINTRO[onRolloverIntro callback] PAY_ROLLINTRO --> ROLLINTRO SHEET -- Rollover introShown=true --> ROLLDEBT CALC[Calculator.tsx] -.->|entry from InstallmentsSheet| CALC PAY --> INFOSHEET[InfoSheet total/discount/fees]Comments Outside Diff (3)
src/components/pay/RolloverIntroSheet.tsx, line 43-46 (link)setStateis called directly during render (lines 43–46) instead of insideuseEffect. This is a React anti-pattern that is inconsistent with how the identical pattern is correctly handled inPaymentSheet.tsx(lines 46–51).Calling
setStateduring render can cause subtle bugs under React 18's concurrent rendering — the render can be interrupted and retried, potentially triggering the state update more than once.src/components/pay/PaymentSheet.tsx, line 106-114 (link)isLatestPluginguard was removed from the pre-navigation check for returning users. In the new code,isLatestPluginis only checked insideRolloverIntroSheet(which is only shown when!rolloverIntroShown). This means returning users (whererolloverIntroShown === true) bypass the plugin guard entirely and navigate directly to/roll-debtwithout the version check.The same gap exists in
Pay.tsx'sonRolloverhandler (line 161–167). If/roll-debtdoes not independently checkisLatestPlugin, users with outdated account plugins may encounter runtime errors instead of receiving the "Upgrade account to rollover" message.Consider re-adding the
isLatestPlugincheck to thenavigateToRollovercallback for all users, not just first-time users:src/components/pay/UpcomingPayments.tsx, line 115 (link)Accessibility aria-label key
"Payment due {{date}}, {{amount}}"(used here) is missing fromes.json. The same issue exists for"Overdue payment {{date}}, {{amount}}"inOverduePayments.tsx(line 120). Spanish-speaking users relying on screen readers will hear the raw English key string instead of a localized description.Add the following entries to
es.json:Last reviewed commit: 08feec0