-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Goals]: Fix applying templates in tracking budget #4010
Conversation
✅ 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 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
|
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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/loot-core/src/server/budget/goaltemplates.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eslint-plugin". (The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request introduces a modification to the The new condition allows categories to be included in the template processing under two scenarios: when The modification is focused solely on the internal logic of the Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (2)packages/loot-core/src/server/budget/goaltemplates.ts (2)
The condition change from
While the logic change looks correct, let's verify the behavior for both tracking and regular budgets. ✅ Verification successfulBased on the results from our verification scripts, I can now provide a final response: Logic change is correctly handling both budget types The code change is verified to be correct for both tracking (reflect) and regular budgets. The evidence shows:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Analyze usage of isReflect across the codebase to understand the full impact
# Check how isReflectBudget is used in other files
echo "Checking isReflectBudget usage:"
rg "isReflectBudget" -A 5
# Look for other budget type checks that might be affected
echo "\nChecking for related budget type conditions:"
rg "is_income.*isReflect|isReflect.*is_income"
# Check for any tests related to this functionality
echo "\nChecking for related tests:"
fd -e test.ts -e test.js -x rg -l "isReflect|tracking.?budget"
Length of output: 6655 Script: #!/bin/bash
# Let's analyze the test coverage for both budget types
# Check for test cases related to budget types
echo "Checking test cases for budget types:"
rg -A 10 "describe.*budget.*type" packages/loot-core/src/server/budget/
# Look for test assertions involving isReflect and is_income
echo "\nChecking test assertions for budget behavior:"
rg "expect.*isReflect|expect.*is_income" packages/loot-core/src/server/budget/
# Check implementation of budget type switching
echo "\nChecking budget type implementation:"
ast-grep --pattern 'function $FUNC($$$) {
$$$
budgetType
$$$
}'
Length of output: 503 Script: #!/bin/bash
# Let's analyze the actual implementation and tests more thoroughly
# Check how budget types affect category handling
echo "Checking category handling with budget types:"
rg -A 5 "budgetType.*reflect|reflect.*budget" packages/loot-core/src/server/budget/
# Look for test files specifically testing goaltemplates
echo "\nChecking goaltemplates tests:"
fd "goaltemplates.test" -x cat {}
# Check for any budget type related migrations or schema
echo "\nChecking budget type in schema and migrations:"
rg "budgetType|reflect.*budget" packages/loot-core/src/server/db/
Length of output: 1517 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 (
|
my logic was wrong and made templates not apply to the tracking budget. Fixes #4006
To test:
Add any template to a tracking budget and try to apply it. It won't do anything in current edge.