[WIP] Template UI and progress bar combined#7276
Open
ohnefrust wants to merge 9 commits intoactualbudget:masterfrom
Open
[WIP] Template UI and progress bar combined#7276ohnefrust wants to merge 9 commits intoactualbudget:masterfrom
ohnefrust wants to merge 9 commits intoactualbudget:masterfrom
Conversation
added 9 commits
February 17, 2026 00:00
…ate-ui-and-progress-bar-combined
…and-progress-bar-combined # Conflicts: # packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx # packages/loot-core/src/server/budget/goal-template.ts
…se the template value from the colum
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
👋 Hello contributor! We would love to review your PR! Before we can do that, please make sure:
We do this to reduce the TOIL the core contributor team has to go through for each PR and to allow for speedy reviews and merges. For more information, please see our Contributing Guide. |
✅ Deploy Preview for actualbudget-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR introduces a "Goal Templates" feature for the budget UI, enabling visual progress bars below expense category rows and editable template amount columns. The implementation spans frontend component updates (progress bar rendering, layout adjustments), backend template goal handlers (fetching, saving), a context provider for template data management, and supporting documentation. New UI controls include a status bars visibility toggle in the titlebar. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Budget Page
participant Provider as TemplateGoalProvider
participant Server as RPC Server
participant DB as Database
Client->>Provider: Wrap with enabled=true
Provider->>Server: send('budget/get-template-goals', {month})
Server->>DB: Query categories & templates
DB-->>Server: Template settings & notes
Server->>Server: Compute goal preview
Server-->>Provider: {categoryId → goal}
Provider-->>Client: Store in context
Client->>Client: Render CategoryProgressBar
Client->>Provider: useTemplateGoal(categoryId, month)
Provider-->>Client: goal value
Client->>Client: User edits TemplateAmountCell
Client->>Server: send('budget/set-single-category-template', {categoryId, amount})
Server->>DB: Update category goal_def
DB-->>Server: Persisted
Client->>Client: Invalidate queries & re-render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/loot-core/src/server/budget/template-notes.ts (1)
100-127:⚠️ Potential issue | 🟠 MajorDon’t persist validation failures as synthetic templates.
goal-template.tslater deserializesgoal_def, butCategoryTemplateContext.init()only consumesdirective === 'template' | 'goal'. Thesedirective: 'error'entries will therefore be silently skipped during preview/apply, so an invalid adjustment now behaves like “no template” instead of surfacing a failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/src/server/budget/template-notes.ts` around lines 100 - 127, Do not persist validation failures as synthetic templates; instead, stop parsing and surface the failure. Replace the block that pushes an object with directive: 'error' into parsedTemplates (the code that references parsedTemplates and parsedTemplate and sets validationError) with behavior that throws/returns a validation error (include the line number and validationError text) so callers (like goal-template deserialization / CategoryTemplateContext.init) see the failure immediately rather than silently skipping it. Ensure the thrown error uses a clear message referencing the template type/line and validationError so it surfaces in preview/apply flows.packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (1)
223-229:⚠️ Potential issue | 🟠 MajorUse
resolvedGoalValuefor the goal UI, notgoalValue.Line 228 still shows the persisted goal, and Lines 279-283 / 308-310 still hide the tooltip and inline goal status when only the template override exists. That makes template-driven categories show stale or missing goal details even though the balance styling already uses the override.
Also applies to: 279-283, 308-310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx` around lines 223 - 229, The UI is rendering the persisted goal (goalValue) instead of the effective value (resolvedGoalValue) and the tooltip/inline goal status logic still hides when only a template override exists; update the Trans block that sets amount (currently using goalValue) to use resolvedGoalValue and change the conditional checks around showing the tooltip and inline goal status (the branches controlling visibility around the goal display) to base visibility on resolvedGoalValue (or an existence check of the effective goal) rather than goalValue so template-driven overrides display the correct goal UI; look for usages in BalanceWithCarryover.tsx around the goal display/tooltip rendering and replace goalValue checks with resolvedGoalValue.packages/loot-core/src/server/budget/goal-template.ts (2)
313-318:⚠️ Potential issue | 🟠 MajorDon’t surface the no-op path as a template error.
When
forceis false and every eligible category is already budgeted,templateContextsis empty even though nothing failed. This branch now returns a sticky “There were errors…” notification with an emptypre, which regresses the normal “nothing to do” case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/src/server/budget/goal-template.ts` around lines 313 - 318, The current branch conflates an empty templateContexts (no work) with template parsing errors; change the conditional so you only return the sticky "There were errors..." object when errors.length > 0 (i.e., there are actual errors). If errors.length === 0 and templateContexts.length === 0 (common when force is false and everything is already budgeted), return the no-op result (e.g., null/undefined or a non-error response) instead of the error notification. Update the branch around errors and templateContexts (and respect the force flag) so only real errors produce the sticky error message.
223-230:⚠️ Potential issue | 🔴 CriticalUse
for...ofloop withawaitinstead offorEachto ensure all promises complete beforebatchMessagesexits.
Array.prototype.forEachnever awaits promises. Since bothsetBudget()andsetGoal()returnPromise<void>, the current code queues all operations synchronously, then exits the async callback before any writes finish. This breaks the batching mechanism.Applies to lines 223–232 and 241–250.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/src/server/budget/goal-template.ts` around lines 223 - 230, The batchMessages callback is using templateBudget.forEach which does not await promises, so change the callback in setBudgets to a for...of loop and await each setBudget call (await setBudget({...})) to ensure each Promise<void> completes before batchMessages returns; do the same in the analogous setGoals function (replace forEach with for...of and await each setGoal call), keeping the calls to batchMessages, setBudget, and setGoal as the unique symbols to locate and update.
🧹 Nitpick comments (3)
packages/desktop-client/src/components/budget/CategoryProgressBar.tsx (1)
135-144: Consider removinguseMemo(optional).Per the desktop-client coding guidelines, the React Compiler automatically memoizes component bodies. The manual
useMemowrapper aroundcomputeCategoryProgressis not strictly necessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/budget/CategoryProgressBar.tsx` around lines 135 - 144, The useMemo wrapper around computeCategoryProgress in CategoryProgressBar is unnecessary; replace the memoized call with a direct invocation (const progress = computeCategoryProgress({ assigned, activity, balance, template })), remove the useMemo dependency array, and delete the useMemo import from React if it becomes unused; ensure the symbol computeCategoryProgress and the variables assigned/activity/balance/template are used exactly as before.packages/desktop-client/src/components/Titlebar.tsx (1)
110-135: ExtractStatusBarsButtonto its own file.This is a new component rather than a tiny one-line helper, and keeping it inline will make
Titlebar.tsxharder to scan as more route-specific actions accumulate. As per coding guidelines, "packages/desktop-client/src/components/**/*.{ts,tsx}: Create new components in their own files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/Titlebar.tsx` around lines 110 - 135, Extract the StatusBarsButton component into its own file: create a new component file exporting the StatusBarsButton (with the same StatusBarsButtonProps type), move the implementation (useTranslation, useFeatureFlag('goalTemplatesEnabled'), useGlobalPref('showProgressBars'), aria-label logic, onPress toggling setShowProgressBarsPref, Button and SvgChartBar usage and CSSProperties import) into that file preserving behavior and props, and then replace the inline function in Titlebar.tsx with an import of the new StatusBarsButton component.packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx (1)
84-140: ExtractTemplateAmountCellinto a shared component file.This is now a near-copy of the helper in
packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx, so formatter/parser fixes will drift between budget types.As per coding guidelines, "Create new components in their own files" and "Prefer iteration and modularization over code duplication".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx` around lines 84 - 140, Duplicate TemplateAmountCell implementation should be extracted into a shared component file: create a new component (e.g., TemplateAmountCell) in its own file and move the current function there, export it, and replace the inline copies in TrackingBudgetComponents.tsx and EnvelopeBudgetComponents.tsx with imports of that shared component. Ensure the new file imports/use the same dependencies referenced in the diff (useFormat, InputCell, integerToAmount, theme, styles, format.currency.decimalPlaces) and preserves the same props type TemplateAmountCellProps and behavior for onSave, editing state, formatter, onUpdate logic and value/valueStyle/inputProps; update both original files to remove duplicated code and import the shared component instead. Run TypeScript build/lint to fix any imports or typing issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/desktop-client/src/components/budget/CategoryProgressBar.tsx`:
- Around line 150-158: The tooltip text is misleading: in CategoryProgressBar
the percent is computed as Math.round((progress.spentRatio +
progress.overflowRatio) * 100) (which is relative to the assigned/budgeted
amount), but when template > 0 the code pushes "X% of template spent"; update
the logic in the tooltipParts push (the block using progress.baselineAmount,
percent, template) to either (a) change the string to accurately say "X% of
assigned/budget spent" when keeping the current percent calculation, or (b) if
you intend the percent to be relative to the template amount, recompute percent
using template (e.g. use spent / template to derive ratio) and then keep "X% of
template spent"; modify the conditional around template and the percent
calculation accordingly in CategoryProgressBar.tsx where tooltipParts is
populated.
In
`@packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx`:
- Around line 388-389: The current code forces templateValue to 0 via
templateAmount = templateValue ?? 0 which makes BalanceWithCarryover treat
non-null overrides incorrectly; change the usage so that the value passed into
BalanceWithCarryover (and any underfunded calculation like isUnderfunded)
preserves null/undefined (i.e., use templateValue directly), and only apply the
0 fallback when rendering the CategoryProgressBar prop that requires a numeric
value; update references to templateAmount in EnvelopeBudgetComponents (and the
similar occurrences around lines 686-689) so BalanceWithCarryover gets the raw
templateValue while CategoryProgressBar gets templateValue ?? 0.
In `@packages/desktop-client/src/components/budget/TemplateGoalContext.tsx`:
- Around line 44-53: The memoized fingerprint (categoryTemplateFingerprint) only
depends on categories and their template_settings.goal_def but the backend call
budget/get-template-goals and CategoryTemplateContext.init() also rely on
sheet-derived state (last month leftover/carryover), so include sheet-related
state in the fingerprint and dependency array so previews refetch when sheets
change; update the useMemo that builds categoryTemplateFingerprint (and the
similar logic in the block covering lines 55-96) to incorporate a unique
sheet/version marker (e.g., include a sheetsVersion, currentSheet.id, or
relevant per-category carryover/leftover fields such as
category.last_month_leftover or a top-level sheetsTimestamp) into the string
keys and into the dependencies so the template previews and goalOverride
recompute after carryover/balance changes.
In `@packages/loot-core/src/server/budget/goal-template.ts`:
- Around line 152-182: setSingleCategoryTemplate currently writes the UI edit
only to category fields (goal_def and template_settings) via storeTemplates, but
getTemplateGoalPreview/storeNoteTemplates later re-writes those fields from
note-backed templates; to fix this, ensure setSingleCategoryTemplate also
updates or clears the category's note-backed template so note sync won't
overwrite the UI change — e.g., after computing templates in
setSingleCategoryTemplate call the same persistence used for note-backed
templates (storeNoteTemplates or the code path that updates the category's note
content) to remove or update the template in the note, or modify storeTemplates
to persist both category fields and the associated note template simultaneously;
reference setSingleCategoryTemplate, getTemplateGoalPreview, storeNoteTemplates,
storeTemplates, goal_def, template_settings, and categoriesWithTemplates when
making the change.
In `@packages/loot-core/src/server/budget/template-notes.ts`:
- Around line 258-260: The 'copy' branch in the template rendering switch drops
the optional limit (it currently returns `${prefix} copy from
${template.lookBack} months ago`); update the case 'copy' branch so it preserves
and appends the optional limit when present (e.g., append " up to
{template.limit}" only if template.limit is defined) so round-tripping
`#template copy from … up to …` rules doesn't lose the limit value.
---
Outside diff comments:
In `@packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx`:
- Around line 223-229: The UI is rendering the persisted goal (goalValue)
instead of the effective value (resolvedGoalValue) and the tooltip/inline goal
status logic still hides when only a template override exists; update the Trans
block that sets amount (currently using goalValue) to use resolvedGoalValue and
change the conditional checks around showing the tooltip and inline goal status
(the branches controlling visibility around the goal display) to base visibility
on resolvedGoalValue (or an existence check of the effective goal) rather than
goalValue so template-driven overrides display the correct goal UI; look for
usages in BalanceWithCarryover.tsx around the goal display/tooltip rendering and
replace goalValue checks with resolvedGoalValue.
In `@packages/loot-core/src/server/budget/goal-template.ts`:
- Around line 313-318: The current branch conflates an empty templateContexts
(no work) with template parsing errors; change the conditional so you only
return the sticky "There were errors..." object when errors.length > 0 (i.e.,
there are actual errors). If errors.length === 0 and templateContexts.length ===
0 (common when force is false and everything is already budgeted), return the
no-op result (e.g., null/undefined or a non-error response) instead of the error
notification. Update the branch around errors and templateContexts (and respect
the force flag) so only real errors produce the sticky error message.
- Around line 223-230: The batchMessages callback is using
templateBudget.forEach which does not await promises, so change the callback in
setBudgets to a for...of loop and await each setBudget call (await
setBudget({...})) to ensure each Promise<void> completes before batchMessages
returns; do the same in the analogous setGoals function (replace forEach with
for...of and await each setGoal call), keeping the calls to batchMessages,
setBudget, and setGoal as the unique symbols to locate and update.
In `@packages/loot-core/src/server/budget/template-notes.ts`:
- Around line 100-127: Do not persist validation failures as synthetic
templates; instead, stop parsing and surface the failure. Replace the block that
pushes an object with directive: 'error' into parsedTemplates (the code that
references parsedTemplates and parsedTemplate and sets validationError) with
behavior that throws/returns a validation error (include the line number and
validationError text) so callers (like goal-template deserialization /
CategoryTemplateContext.init) see the failure immediately rather than silently
skipping it. Ensure the thrown error uses a clear message referencing the
template type/line and validationError so it surfaces in preview/apply flows.
---
Nitpick comments:
In `@packages/desktop-client/src/components/budget/CategoryProgressBar.tsx`:
- Around line 135-144: The useMemo wrapper around computeCategoryProgress in
CategoryProgressBar is unnecessary; replace the memoized call with a direct
invocation (const progress = computeCategoryProgress({ assigned, activity,
balance, template })), remove the useMemo dependency array, and delete the
useMemo import from React if it becomes unused; ensure the symbol
computeCategoryProgress and the variables assigned/activity/balance/template are
used exactly as before.
In
`@packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx`:
- Around line 84-140: Duplicate TemplateAmountCell implementation should be
extracted into a shared component file: create a new component (e.g.,
TemplateAmountCell) in its own file and move the current function there, export
it, and replace the inline copies in TrackingBudgetComponents.tsx and
EnvelopeBudgetComponents.tsx with imports of that shared component. Ensure the
new file imports/use the same dependencies referenced in the diff (useFormat,
InputCell, integerToAmount, theme, styles, format.currency.decimalPlaces) and
preserves the same props type TemplateAmountCellProps and behavior for onSave,
editing state, formatter, onUpdate logic and value/valueStyle/inputProps; update
both original files to remove duplicated code and import the shared component
instead. Run TypeScript build/lint to fix any imports or typing issues.
In `@packages/desktop-client/src/components/Titlebar.tsx`:
- Around line 110-135: Extract the StatusBarsButton component into its own file:
create a new component file exporting the StatusBarsButton (with the same
StatusBarsButtonProps type), move the implementation (useTranslation,
useFeatureFlag('goalTemplatesEnabled'), useGlobalPref('showProgressBars'),
aria-label logic, onPress toggling setShowProgressBarsPref, Button and
SvgChartBar usage and CSSProperties import) into that file preserving behavior
and props, and then replace the inline function in Titlebar.tsx with an import
of the new StatusBarsButton component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5381ed8-133f-4791-9e2d-ee4907e195a4
📒 Files selected for processing (23)
packages/component-library/src/Tooltip.tsxpackages/desktop-client/src/budget/mutations.tspackages/desktop-client/src/components/Titlebar.tsxpackages/desktop-client/src/components/budget/BalanceWithCarryover.tsxpackages/desktop-client/src/components/budget/BudgetCategories.tsxpackages/desktop-client/src/components/budget/BudgetTable.tsxpackages/desktop-client/src/components/budget/CategoryProgressBar.test.tsxpackages/desktop-client/src/components/budget/CategoryProgressBar.tsxpackages/desktop-client/src/components/budget/ExpenseCategory.tsxpackages/desktop-client/src/components/budget/GOAL_TEMPLATES_FEATURES.mdpackages/desktop-client/src/components/budget/IncomeCategory.tsxpackages/desktop-client/src/components/budget/PROGRESS_BAR_IMPLEMENTATION.mdpackages/desktop-client/src/components/budget/SidebarCategory.tsxpackages/desktop-client/src/components/budget/TemplateGoalContext.tsxpackages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsxpackages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsxpackages/desktop-client/src/components/budget/util.tspackages/loot-core/src/server/budget/app.tspackages/loot-core/src/server/budget/goal-template.tspackages/loot-core/src/server/budget/template-notes.tspackages/loot-core/src/shared/util.test.tspackages/loot-core/src/types/prefs.tsupcoming-release-notes/7000.md
Comment on lines
+150
to
+158
| if (progress.baselineAmount > 0) { | ||
| const percent = Math.round( | ||
| (progress.spentRatio + progress.overflowRatio) * 100, | ||
| ); | ||
| tooltipParts.push( | ||
| template && template > 0 | ||
| ? `${percent}% of template spent` | ||
| : `${percent}% of budget spent`, | ||
| ); |
Contributor
There was a problem hiding this comment.
Tooltip percentage text may be misleading when a template exists.
When template > 0, the tooltip displays "X% of template spent", but the percentage is calculated from spentRatio + overflowRatio, which is relative to the assigned/budgeted amount, not the template amount. This could mislead users when assigned !== template.
Consider clarifying the text to accurately reflect what the percentage represents:
💡 Suggested fix
tooltipParts.push(
template && template > 0
- ? `${percent}% of template spent`
- : `${percent}% of budget spent`,
+ ? `${percent}% of budget spent`
+ : `${percent}% of budget spent`,
);Or, if the intent is to show percentage relative to template, adjust the calculation accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/desktop-client/src/components/budget/CategoryProgressBar.tsx` around
lines 150 - 158, The tooltip text is misleading: in CategoryProgressBar the
percent is computed as Math.round((progress.spentRatio + progress.overflowRatio)
* 100) (which is relative to the assigned/budgeted amount), but when template >
0 the code pushes "X% of template spent"; update the logic in the tooltipParts
push (the block using progress.baselineAmount, percent, template) to either (a)
change the string to accurately say "X% of assigned/budget spent" when keeping
the current percent calculation, or (b) if you intend the percent to be relative
to the template amount, recompute percent using template (e.g. use spent /
template to derive ratio) and then keep "X% of template spent"; modify the
conditional around template and the percent calculation accordingly in
CategoryProgressBar.tsx where tooltipParts is populated.
Comment on lines
+388
to
+389
| const templateAmount = templateValue ?? 0; | ||
| const isUnderfunded = |
Contributor
There was a problem hiding this comment.
Preserve null when there is no template override.
BalanceWithCarryover treats any non-null goalOverride as authoritative. With templateValue ?? 0, categories that do not have a template now render against a zero goal whenever the feature flag is on, which skews funded/underfunded styling and tooltip math. Keep the 0 fallback only for CategoryProgressBar.
🛠️ Proposed fix
- const templateAmount = templateValue ?? 0;
+ const templateAmount = templateValue ?? 0;
+ const templateGoalOverride = templateValue ?? null;
...
- goalOverride={templateAmount}
+ goalOverride={templateGoalOverride}Also applies to: 686-689
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx`
around lines 388 - 389, The current code forces templateValue to 0 via
templateAmount = templateValue ?? 0 which makes BalanceWithCarryover treat
non-null overrides incorrectly; change the usage so that the value passed into
BalanceWithCarryover (and any underfunded calculation like isUnderfunded)
preserves null/undefined (i.e., use templateValue directly), and only apply the
0 fallback when rendering the CategoryProgressBar prop that requires a numeric
value; update references to templateAmount in EnvelopeBudgetComponents (and the
similar occurrences around lines 686-689) so BalanceWithCarryover gets the raw
templateValue while CategoryProgressBar gets templateValue ?? 0.
Comment on lines
+44
to
+53
| const categoryTemplateFingerprint = useMemo( | ||
| () => | ||
| categories | ||
| .map( | ||
| category => | ||
| `${category.id}:${category.template_settings?.source || ''}:${category.goal_def || ''}`, | ||
| ) | ||
| .join('|'), | ||
| [categories], | ||
| ); |
Contributor
There was a problem hiding this comment.
Refetch template previews when sheet state changes, not just template definitions.
The effect key only tracks goal_def / template_settings, but budget/get-template-goals also depends on budget sheets; CategoryTemplateContext.init() reads last month’s leftover and carryover. After carryover or other balance-affecting changes, the Template column and goalOverride values can stay stale until the categories query changes or the page remounts.
Also applies to: 55-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/desktop-client/src/components/budget/TemplateGoalContext.tsx` around
lines 44 - 53, The memoized fingerprint (categoryTemplateFingerprint) only
depends on categories and their template_settings.goal_def but the backend call
budget/get-template-goals and CategoryTemplateContext.init() also rely on
sheet-derived state (last month leftover/carryover), so include sheet-related
state in the fingerprint and dependency array so previews refetch when sheets
change; update the useMemo that builds categoryTemplateFingerprint (and the
similar logic in the block covering lines 55-96) to incorporate a unique
sheet/version marker (e.g., include a sheetsVersion, currentSheet.id, or
relevant per-category carryover/leftover fields such as
category.last_month_leftover or a top-level sheetsTimestamp) into the string
keys and into the dependencies so the template previews and goalOverride
recompute after carryover/balance changes.
Comment on lines
+152
to
+182
| export async function setSingleCategoryTemplate({ | ||
| categoryId, | ||
| amount, | ||
| }: { | ||
| categoryId: CategoryEntity['id']; | ||
| amount: number | null; | ||
| }): Promise<void> { | ||
| if (amount === null) { | ||
| // Clear template by removing goal_def and template_settings | ||
| await db.updateWithSchema('categories', { | ||
| id: categoryId, | ||
| goal_def: null, | ||
| template_settings: null, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const templates: Template[] = [ | ||
| { | ||
| type: 'simple', | ||
| monthly: amount, | ||
| directive: 'template', | ||
| priority: 0, | ||
| }, | ||
| ]; | ||
|
|
||
| await storeTemplates({ | ||
| categoriesWithTemplates: [{ id: categoryId, templates }], | ||
| source: 'ui', | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
UI-edited templates are overwritten by the next note sync.
setSingleCategoryTemplate() stores the edit in goal_def / template_settings, but getTemplateGoalPreview() immediately calls storeNoteTemplates(), which rewrites those same fields from category notes. Any category that still has a note-backed template will revert on the next preview/apply round, so direct grid edits and clears do not persist.
Also applies to: 189-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/loot-core/src/server/budget/goal-template.ts` around lines 152 -
182, setSingleCategoryTemplate currently writes the UI edit only to category
fields (goal_def and template_settings) via storeTemplates, but
getTemplateGoalPreview/storeNoteTemplates later re-writes those fields from
note-backed templates; to fix this, ensure setSingleCategoryTemplate also
updates or clears the category's note-backed template so note sync won't
overwrite the UI change — e.g., after computing templates in
setSingleCategoryTemplate call the same persistence used for note-backed
templates (storeNoteTemplates or the code path that updates the category's note
content) to remove or update the template in the note, or modify storeTemplates
to persist both category fields and the associated note template simultaneously;
reference setSingleCategoryTemplate, getTemplateGoalPreview, storeNoteTemplates,
storeTemplates, goal_def, template_settings, and categoriesWithTemplates when
making the change.
Comment on lines
258
to
+260
| case 'copy': { | ||
| // #template copy from <lookBack> months ago [limit] | ||
| const result = `${prefix} copy from ${template.lookBack} months ago`; | ||
| return result; | ||
| return `${prefix} copy from ${template.lookBack} months ago`; |
Contributor
There was a problem hiding this comment.
Preserve limit when round-tripping copy templates.
This branch currently drops the optional limit, so saving/rendering a #template copy from … up to … rule will strip part of the user’s template.
Suggested fix
case 'copy': {
// `#template` copy from <lookBack> months ago [limit]
- return `${prefix} copy from ${template.lookBack} months ago`;
+ let result = `${prefix} copy from ${template.lookBack} months ago`;
+ if (template.limit) {
+ result += ` ${limitToString(template.limit)}`;
+ }
+ return result;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case 'copy': { | |
| // #template copy from <lookBack> months ago [limit] | |
| const result = `${prefix} copy from ${template.lookBack} months ago`; | |
| return result; | |
| return `${prefix} copy from ${template.lookBack} months ago`; | |
| case 'copy': { | |
| // `#template` copy from <lookBack> months ago [limit] | |
| let result = `${prefix} copy from ${template.lookBack} months ago`; | |
| if (template.limit) { | |
| result += ` ${limitToString(template.limit)}`; | |
| } | |
| return result; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/loot-core/src/server/budget/template-notes.ts` around lines 258 -
260, The 'copy' branch in the template rendering switch drops the optional limit
(it currently returns `${prefix} copy from ${template.lookBack} months ago`);
update the case 'copy' branch so it preserves and appends the optional limit
when present (e.g., append " up to {template.limit}" only if template.limit is
defined) so round-tripping `#template copy from … up to …` rules doesn't lose
the limit value.
Contributor
|
🤖 Auto-generated Release Notes Hey @ohnefrust! I've automatically created a release notes file based on CodeRabbit's analysis: Category: Features If you're happy with this release note, you can add it to your pull request. If not, you'll need to add your own before a maintainer can review your change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This branch adds two related budgeting improvements behind the existing goal templates feature flag.
The first addition is a new Template column in the budget table for both envelope and tracking budgets. It shows template totals at the overall and group level, lets users edit a single category’s template directly in the grid, and uses those template values consistently when rendering goal/balance states. To support that, the branch adds backend actions for setting a single category template and fetching computed template goal previews for a given month.
The second addition is a per-category progress bar shown below expense rows. The bar visualizes spending against the assigned budget, supports template-aware underfunded states, shows overspending with a separate overflow segment, and exposes a tooltip with budget/template/spent/balance details. There is also a new budget-page titlebar toggle to show or hide these progress bars, with the preference stored globally.
In addition to the feature work, the branch includes supporting polish and fixes such as tooltip wrapper styling support, improved template note validation/error handling, and tests covering progress bar calculation and rendering.
Related issue(s)
Relates to #2965: Progress bar for targets/templates
Testing
New Feature. Ran yarn and vibe-coded tests.
Checklist