LOW-32: Make savings goal deposits first-class in transaction flow#74
LOW-32: Make savings goal deposits first-class in transaction flow#74soliktomasz wants to merge 2 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR extends the transaction form to optionally associate transactions with savings goals. API client payloads now accept ChangesSavings Goal Association for Transactions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/TransactionForm.test.tsx`:
- Around line 76-77: Replace the nonstandard programmatic submit with a user
action and wait for the async side effect: change the test to click the Save
transaction button (use fireEvent.click on the button obtained via
screen.getByRole) instead of fireEvent.submit, and wrap the assertion about
apiClient.createTransaction (the mocked function) in waitFor so the test awaits
the async handleSubmit path and avoids the race condition.
In `@frontend/src/components/TransactionForm.tsx`:
- Around line 80-82: The object literal in TransactionForm that contains the
properties `note: ""` and `savingsGoalId: ""` is missing a comma after `note:
""`; update the object (the initializer inside the TransactionForm component
where `note` and `savingsGoalId` are defined) by adding a trailing comma after
`note: ""` so the properties are correctly separated and the file parses.
In `@frontend/src/state/I18nContext.tsx`:
- Around line 1201-1202: The Polish, German and Spanish dictionaries in
I18nContext.tsx still have English values for the keys
transactionForm.savingsGoal and transactionForm.noSavingsGoal; update those key
values in the Polish section to "Cel oszczędności" and "Brak celu oszczędności",
in the German section to "Sparziel" and "Kein Sparziel", and in the Spanish
section to "Objetivo de ahorro" and "Sin objetivo de ahorro" so each locale
shows the correct translations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34054068-66ad-4917-900f-8a741360d6a7
📒 Files selected for processing (6)
frontend/src/api/client.tsfrontend/src/components/TransactionForm.test.tsxfrontend/src/components/TransactionForm.tsxfrontend/src/pages/GoalsPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/src/state/I18nContext.tsx
| fireEvent.submit(screen.getByRole("button", { name: "Save transaction" })); | ||
| expect(apiClient.createTransaction).toHaveBeenCalledWith("token", expect.objectContaining({ savingsGoalId: "g1" })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the test file
fd -t f "TransactionForm.test.tsx"Repository: soliktomasz/Ledgerra
Length of output: 113
🏁 Script executed:
# Check file size and content
if [ -f "frontend/src/components/TransactionForm.test.tsx" ]; then
wc -l "frontend/src/components/TransactionForm.test.tsx"
fiRepository: soliktomasz/Ledgerra
Length of output: 116
🏁 Script executed:
# Read the test file focusing on lines around 76-77
if [ -f "frontend/src/components/TransactionForm.test.tsx" ]; then
cat -n "frontend/src/components/TransactionForm.test.tsx" | sed -n '1,100p'
fiRepository: soliktomasz/Ledgerra
Length of output: 3429
🏁 Script executed:
# Check the TransactionForm component to understand the form submission behavior
fd -t f "TransactionForm.tsx" | grep -v testRepository: soliktomasz/Ledgerra
Length of output: 108
🏁 Script executed:
# Read the TransactionForm component to understand form submission
cat -n "frontend/src/components/TransactionForm.tsx" | head -150Repository: soliktomasz/Ledgerra
Length of output: 5417
🏁 Script executed:
# Search for the form submission handler in the component
rg -A 20 "createTransaction" "frontend/src/components/TransactionForm.tsx"Repository: soliktomasz/Ledgerra
Length of output: 725
🏁 Script executed:
# Let's see more of the TransactionForm component, especially the form/button structure
rg -B 5 -A 10 "Save transaction|submit" "frontend/src/components/TransactionForm.tsx" | head -50Repository: soliktomasz/Ledgerra
Length of output: 1854
🏁 Script executed:
# Check the complete handleSubmit and form structure
cat -n "frontend/src/components/TransactionForm.tsx" | sed -n '139,200p'Repository: soliktomasz/Ledgerra
Length of output: 2573
Use a user-triggered submit path and await the async side effect.
Line 76 dispatches submit on the button element, which is non-standard with Testing Library; Line 77 also asserts immediately without awaiting, creating a race condition since handleSubmit is async and the mocked createTransaction resolves a promise. Use fireEvent.click on the button and wrap the assertion in waitFor.
Suggested test hardening
-import { cleanup, fireEvent, render, screen } from "`@testing-library/react`";
+import { cleanup, fireEvent, render, screen, waitFor } from "`@testing-library/react`";
...
- fireEvent.submit(screen.getByRole("button", { name: "Save transaction" }));
- expect(apiClient.createTransaction).toHaveBeenCalledWith("token", expect.objectContaining({ savingsGoalId: "g1" }));
+ fireEvent.click(screen.getByRole("button", { name: "Save transaction" }));
+ await waitFor(() => {
+ expect(apiClient.createTransaction).toHaveBeenCalledWith(
+ "token",
+ expect.objectContaining({ savingsGoalId: "g1" })
+ );
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/TransactionForm.test.tsx` around lines 76 - 77,
Replace the nonstandard programmatic submit with a user action and wait for the
async side effect: change the test to click the Save transaction button (use
fireEvent.click on the button obtained via screen.getByRole) instead of
fireEvent.submit, and wrap the assertion about apiClient.createTransaction (the
mocked function) in waitFor so the test awaits the async handleSubmit path and
avoids the race condition.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f9d3c60d5
ℹ️ 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".
| note: "" | ||
| savingsGoalId: "" |
There was a problem hiding this comment.
Add a comma after note in buildDefaultValues; without it, this object literal is invalid syntax and TransactionForm.tsx cannot compile, which blocks the transaction UI (and related tests/build steps) from running at all.
Useful? React with 👍 / 👎.
soliktomasz
force-pushed
the
codex/linear-mention-low-32-make-savings-goal-deposits-first-cla
branch
from
1f9d3c6 to
3b02220
Compare
Updates the "Savings goal" and "No savings goal" phrases to their correct Polish, German, and Spanish localizations. Previously, these phrases appeared in English within their respective language blocks.
1 participant
Motivation
TransferOuttransactions but the UI did not carry a selected goal into the transaction flow so users could not easily create linked deposits.Description
savingsGoalIdin the Goals page deposit and auto-transfer actions via the URL (/transactions?form=transfer&savingsGoalId=<id>).TransactionFormto tracksavingsGoalId, render a transfer-only savings goal selector, and includesavingsGoalIdin create/update payloads.savingsGoalIdURL param, and propagate the field when editing/duplicating transfers.savingsGoalIdto theapiClient.createTransactionandapiClient.updateTransactionpayload typings and add UI labels for the new selector inI18nContext.TransactionForm.test.tsxthat verifies a transfer submission includes the selectedsavingsGoalId.Testing
TransactionForm.test.tsxthat assertsapiClient.createTransactionis called withsavingsGoalIdwhen submitting a transfer, and the test file is included in the change set.npm test -- TransactionForm.test.tsxbut the environment does not havevitestinstalled and the test run failed withsh: 1: vitest: not found.Codex Task
Summary by CodeRabbit
New Features
Tests