Fix #2932: Schedule reset amount to ten (10) when amount is zero (0) #3732
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
lelemm
changed the title
WalkthroughThe pull request introduces changes to two components: In Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Learnings (1)
🔇 Additional comments (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (4)
packages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-10-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-11-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-12-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3732.mdis excluded by!**/*.md
📒 Files selected for processing (2)
- packages/desktop-client/src/components/schedules/ScheduleDetails.jsx (1 hunks)
- packages/desktop-client/src/components/util/AmountInput.tsx (5 hunks)
🔇 Additional comments (6)
packages/desktop-client/src/components/util/AmountInput.tsx (4)
113-114: LGTM: Proper state synchronization on blur.The blur handling correctly uses the new sign-aware amount calculation.
89-91: Verify handling of undefined/null values.The amount calculation looks correct, but we should verify edge case handling.
Let's check for proper null/undefined handling:
#!/bin/bash # Search for tests covering AmountInput edge cases rg -A 5 "test.*AmountInput.*undefined|test.*AmountInput.*null" --type typescript --type typescript-react
128-128: LGTM: Proper UI feedback and accessibility.The UI changes correctly reflect the current symbol state and maintain accessibility.
Let's verify accessibility attributes:
#!/bin/bash # Search for accessibility-related tests rg -l "aria-label.*AmountInput" --type typescript --type typescript-reactAlso applies to: 133-137, 148-149
58-60: LGTM: Proper zero amount sign handling implemented.The new
symbolstate correctly handles sign preservation for zero amounts, which is essential for fixing the schedule amount reset issue.Let's verify the component's integration with ScheduleDetails:
✅ Verification successful
Based on the comprehensive analysis of the codebase, I can now provide the final response:
The new
symbolstate implementation is correct and properly integrated.The changes to
AmountInputare well-integrated with the scheduling system:
- The component is used in
ScheduleDetailsfor handling scheduled amounts- The sign handling aligns with the schedule amount validation logic
- The zero amount handling with
zeroSignprop is properly respected in the schedule contextThe implementation correctly preserves the sign for zero amounts, which directly addresses the bug where scheduled amounts were being reset to 10 when the amount is zero.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for AmountInput usage in ScheduleDetails rg -l "AmountInput.*ScheduleDetails" --type typescript --type typescript-reactLength of output: 226
Script:
#!/bin/bash # Search for AmountInput usage in the codebase rg "AmountInput" --type tsx -A 5 -B 5Length of output: 65
Script:
#!/bin/bash # Search for AmountInput usage in the codebase rg "AmountInput" -A 5 -B 5Length of output: 33885
Script:
#!/bin/bash # Search for ScheduleDetails usage and implementation rg "ScheduleDetails" -A 5 -B 5Length of output: 3078
Script:
#!/bin/bash # Search for schedule amount handling in ScheduleDetails rg "amount" -A 5 -B 5 packages/desktop-client/src/components/schedules/ScheduleDetails.jsxLength of output: 5544
packages/desktop-client/src/components/schedules/ScheduleDetails.jsx (2)
Line range hint
449-486: Verify amount handling in save logic.The save logic processes the amount field but doesn't explicitly handle zero values. Consider adding a test case to verify that zero amounts are preserved during save operations.
Let's examine the save logic in the codebase:
105-105: Verify if changing default amount to 0 fully fixes the reset issue.While changing the default amount from
-1000to0is a step in the right direction, we need to verify if this change alone prevents the schedule from resetting to 10 when the amount is zero. The bug description specifically mentions a reset to 10, but this change only modifies the initial default value.Let's search for potential reset logic or hardcoded values:
✅ Verification successful
Let me analyze the results and generate additional verification scripts to understand the amount handling better:
The previous results show the usage of
schedule._amountbut we need to understand how it's validated and processed, especially during updates. These new scripts will help us verify if there's any logic that could cause the amount to reset to 10.
Let me search for one more critical piece - the state updates and reducer logic in ScheduleDetails that might affect the amount:
This will help us understand how the amount state is managed and if there's any logic that could cause it to reset to 10 during state updates or form submission.
Let me search for one final piece - the reducer implementation and initial state setup:
This will help us understand how the amount state is initialized and updated through the reducer, which is crucial for understanding if there's any logic that could cause the amount to reset to 10.
Based on the analysis of the code and shell script results, I can now provide a final response:
The change from
-1000to0as default amount is correct and sufficient.The investigation reveals:
- The amount handling is done through controlled state management with proper validation
- The value
10doesn't appear in any amount-related logic in schedule components- The
schedule._amountis properly handled in multiple places:
- Initial state from the schedule object
- State updates through reducer's
set-fieldaction- Form submission preserving the amount value
- The bug was likely caused by the default
-1000being considered invalid, leading to a fallback value. Setting it to0is a valid default that won't trigger unwanted resets.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential reset logic or hardcoded values in schedule-related files # Test 1: Look for files containing both "schedule" and "amount" in their names echo "=== Schedule & Amount Related Files ===" fd -t f -i "schedule.*amount|amount.*schedule" # Test 2: Search for occurrences of the number 10 in schedule-related files echo -e "\n=== Occurrences of '10' in Schedule Files ===" rg -C 3 "\b10\b" -g "*schedule*" # Test 3: Search for amount reset logic in schedule-related files echo -e "\n=== Amount Reset Logic in Schedule Files ===" rg -C 3 "amount.*=.*(?:10|0)" -g "*schedule*" # Test 4: Search for schedule amount handling in the codebase echo -e "\n=== Schedule Amount Handling ===" ast-grep --pattern 'schedule._amount'Length of output: 17360
Script:
#!/bin/bash # Search for amount validation and processing logic # Look for amount validation in schedule-related files echo "=== Amount Validation Logic ===" rg -C 5 "amount.*valid|valid.*amount" -g "*schedule*" # Look for amount operations and conditions echo -e "\n=== Amount Operations ===" ast-grep --pattern '{ op: $_, field: "amount", value: $_ }' # Look for getScheduledAmount implementation echo -e "\n=== getScheduledAmount Implementation ===" rg -C 5 "function getScheduledAmount" # Look for amount state updates in ScheduleDetails echo -e "\n=== Amount State Updates in ScheduleDetails ===" rg -C 5 "amount.*state" packages/desktop-client/src/components/schedules/ScheduleDetails.jsxLength of output: 1971
Script:
#!/bin/bash # Search for state management in ScheduleDetails # Look for reducer and state updates echo "=== State Management in ScheduleDetails ===" rg -C 10 "function reducer|type: '.*'" packages/desktop-client/src/components/schedules/ScheduleDetails.jsx # Look for amount initialization echo -e "\n=== Amount Initialization ===" rg -C 5 "initialState|useState.*amount" packages/desktop-client/src/components/schedules/ScheduleDetails.jsx # Look for form submission handling echo -e "\n=== Form Submission Logic ===" rg -C 10 "onSubmit|handleSubmit" packages/desktop-client/src/components/schedules/ScheduleDetails.jsxLength of output: 12443
Script:
#!/bin/bash # Search for reducer implementation and initial state # Look for reducer function and initial state echo "=== Reducer Implementation ===" rg -A 20 "function reducer" packages/desktop-client/src/components/schedules/ScheduleDetails.jsx # Look for amount-related state updates in reducer echo -e "\n=== Amount State Updates in Reducer ===" rg -B 5 -A 10 "case '.*':" packages/desktop-client/src/components/schedules/ScheduleDetails.jsx # Look for initial state declaration echo -e "\n=== Initial State Declaration ===" rg -B 2 -A 10 "const initialState = {" packages/desktop-client/src/components/schedules/ScheduleDetails.jsxLength of output: 4526
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Functionally this seem good. I still need to go through the code though. Thanks for making a better fix then my simple fix |
|
No problem. I'm with some spare time at work. Helping cleaning up the issue log ;) |
Fix #2932