coderabbit again.

Author: matt-fidd

packages/desktop-client/src/components/budget/goals/TemplateSentence.tsx

@@ -1,6 +1,7 @@
 import { Trans } from 'react-i18next';
 
 import type { Template } from '@actual-app/core/types/models/templates';
+import type { TransObjectLiteral } from '@actual-app/core/types/util';
 
 import { BySaveAutomationReadOnly } from './editor/BySaveAutomationReadOnly';
 import { FixedAutomationReadOnly } from './editor/FixedAutomationReadOnly';
@@ -46,8 +47,14 @@ export function TemplateSentence({
     case 'simple':
     case 'spend':
     case 'goal':
-    case 'error':
-      return <Trans>Unsupported template type</Trans>;
+    case 'error': {
+      const type = template.type;
+      return (
+        <Trans>
+          Unsupported template type: {{ type } satisfies TransObjectLiteral}
+        </Trans>
+      );
+    }
     default:
       template satisfies never;
       return null;

packages/desktop-client/src/components/budget/goals/editor/FixedAutomationReadOnly.tsx

@@ -11,9 +11,9 @@ type FixedAutomationReadOnlyProps = {
   template: PeriodicTemplate;
 };
 
-export const FixedAutomationReadOnly = ({
+export function FixedAutomationReadOnly({
   template,
-}: FixedAutomationReadOnlyProps) => {
+}: FixedAutomationReadOnlyProps) {
   const format = useFormat();
   const amount = format(
     amountToInteger(template.amount, format.currency.decimalPlaces),
@@ -97,4 +97,4 @@ export const FixedAutomationReadOnly = ({
     default:
       return null;
   }
-};
+}

packages/desktop-client/src/components/modals/BudgetAutomationsModal/BudgetAutomationMigrationWarning.tsx

@@ -19,12 +19,7 @@ export function BudgetAutomationMigrationWarning({
   if (!notes) return null;
   const templates = notes
     .split('\n')
-    .flatMap(line => {
-      if (line.trim().startsWith('#template')) return line;
-      if (line.trim().startsWith('#goal')) return line;
-      if (line.trim().startsWith('#cleanup')) return line;
-      return [];
-    })
+    .filter(line => /^\s*#(template|goal|cleanup)\b/.test(line))
     .join('\n');
 
   if (!templates) return null;

packages/desktop-client/src/components/modals/BudgetAutomationsModal/BudgetAutomationsBody.tsx

@@ -165,6 +165,7 @@ export function BudgetAutomationsBody({
   ]);
   for (const group of categories) {
     for (const cat of group.categories ?? []) {
+      if (!cat.is_income) continue;
       validPercentageSources.add(cat.id);
       if (cat.name) validPercentageSources.add(cat.name.toLowerCase());
     }

packages/desktop-client/src/components/modals/BudgetAutomationsModal/BudgetAutomationsModal.test.ts

@@ -3,19 +3,40 @@ import type { Template } from '@actual-app/core/types/models/templates';
 import { migrateTemplatesToAutomations } from './migrateTemplatesToAutomations';
 
 describe('migrateTemplatesToAutomations', () => {
-  it('preserves simple templates that have no limit and no monthly amount', () => {
+  it('drops simple templates that have no limit and no monthly amount', () => {
+    // these would otherwise be pushed as a phantom 'fixed' entry that
+    // crashes FixedAutomationReadOnly (no .amount, no .period)
     const simpleTemplate = {
       type: 'simple',
       directive: 'template',
       priority: 5,
     } satisfies Template;
 
-    const result = migrateTemplatesToAutomations([simpleTemplate]);
+    expect(migrateTemplatesToAutomations([simpleTemplate])).toEqual([]);
+  });
 
-    expect(result).toHaveLength(1);
-    expect(result[0].displayType).toBe('fixed');
-    expect(result[0].template).toEqual(simpleTemplate);
-    expect(result[0].id).toMatch(/^automation-/);
+  it('drops simple templates whose monthly amount is zero with no limit', () => {
+    const simpleTemplate = {
+      type: 'simple',
+      directive: 'template',
+      priority: 5,
+      monthly: 0,
+    } satisfies Template;
+
+    expect(migrateTemplatesToAutomations([simpleTemplate])).toEqual([]);
+  });
+
+  it('throws when a goal directive reaches migration', () => {
+    const goalTemplate = {
+      type: 'goal',
+      amount: 1000,
+      directive: 'goal',
+      priority: null,
+    } satisfies Template;
+
+    expect(() => migrateTemplatesToAutomations([goalTemplate])).toThrow(
+      /Unsupported template type/,
+    );
   });
 
   it('expands a simple template with limit into limit and refill entries', () => {

packages/desktop-client/src/components/modals/BudgetAutomationsModal/TypePicker.tsx

@@ -1,3 +1,5 @@
+import { useTranslation } from 'react-i18next';
+
 import { Text } from '@actual-app/components/text';
 import { theme } from '@actual-app/components/theme';
 import { View } from '@actual-app/components/view';
@@ -13,9 +15,11 @@ type TypePickerProps = {
 };
 
 export function TypePicker({ active, disabledTypes, onPick }: TypePickerProps) {
+  const { t } = useTranslation();
   const entries = displayTemplateTypes.map(
     id => [id, getDisplayTemplateMeta(id)] as const,
   );
+  const disabledHint = t('Only one of this type allowed per category');
 
   return (
     <View
@@ -36,6 +40,7 @@ export function TypePicker({ active, disabledTypes, onPick }: TypePickerProps) {
             tabIndex={isDisabled ? -1 : 0}
             aria-pressed={isActive}
             aria-disabled={isDisabled}
+            title={isDisabled ? disabledHint : undefined}
             onClick={() => {
               if (!isDisabled) onPick(id);
             }}

packages/desktop-client/src/components/modals/BudgetAutomationsModal/UnsupportedDirectivesNotice.tsx

@@ -92,9 +92,9 @@ export function UnsupportedDirectivesNotice({
   );
 }
 
+const CLEANUP_DIRECTIVE = /^\s*#cleanup\b/;
+
 export function hasCleanupLine(notes: string | null | undefined): boolean {
   if (!notes) return false;
-  return notes
-    .split('\n')
-    .some(line => line.trimStart().startsWith('#cleanup'));
+  return notes.split('\n').some(line => CLEANUP_DIRECTIVE.test(line));
 }

packages/desktop-client/src/components/modals/BudgetAutomationsModal/migrateTemplatesToAutomations.ts

@@ -25,8 +25,16 @@ function getDisplayTypeFromTemplate(template: Template): DisplayTemplateType {
       return 'by';
     case 'remainder':
       return 'remainder';
-    default:
-      return 'fixed';
+    case 'goal':
+    case 'error':
+    case 'spend':
+      // filtered upstream by hasUnsupportedDirective; surface if it ever isn't
+      throw new Error(`Unsupported template type reached migration`);
+    default: {
+      const _exhaustive: never = template;
+      void _exhaustive;
+      throw new Error(`Unhandled template type`);
+    }
   }
 }
 
@@ -37,11 +45,10 @@ export function migrateTemplatesToAutomations(
 
   templates.forEach(template => {
     if (template.type === 'simple') {
-      let hasExpandedTemplate = false;
-      const hasMonthly = template.monthly != null && template.monthly !== 0;
+      const monthly = template.monthly;
+      const hasMonthly = monthly != null && monthly !== 0;
 
       if (template.limit) {
-        hasExpandedTemplate = true;
         entries.push(
           createAutomationEntry(
             {
@@ -73,13 +80,12 @@ export function migrateTemplatesToAutomations(
           );
         }
       }
-      if (template.monthly != null && template.monthly !== 0) {
-        hasExpandedTemplate = true;
+      if (hasMonthly) {
         entries.push(
           createAutomationEntry(
             {
               type: 'periodic',
-              amount: template.monthly,
+              amount: monthly,
               period: { period: 'month', amount: 1 },
               starting: dayFromDate(firstDayOfMonth(new Date())),
               directive: 'template',
@@ -90,11 +96,9 @@ export function migrateTemplatesToAutomations(
         );
       }
 
-      if (!hasExpandedTemplate) {
-        entries.push(
-          createAutomationEntry(template, getDisplayTypeFromTemplate(template)),
-        );
-      }
+      // a simple template with neither monthly nor limit is a no-op; drop it
+      // rather than passing through as a phantom 'fixed' entry that would
+      // crash FixedAutomationReadOnly (no .amount, no .period)
       return;
     }
 

packages/loot-core/src/server/budget/category-template-context.ts

@@ -301,13 +301,21 @@ export class CategoryTemplateContext {
     // projections reflect any clamping that occurred above. The limit branch
     // can produce a negative scale when the carried-over balance already
     // exceeds the cap; floor at 0 here so per-row UI projections never go
-    // negative (engine totals are unaffected).
+    // negative (engine totals are unaffected). Give the last entry the
+    // residual so the per-row sum equals toBudget exactly even when scale
+    // produces fractional cents.
     const perRowScale = Math.max(0, scale);
-    for (const [template, value] of perTemplateLocal) {
-      const scaled = Math.round(value * perRowScale);
+    const items = Array.from(perTemplateLocal);
+    let remaining = Math.max(0, toBudget);
+    items.forEach(([template, value], i) => {
+      const isLast = i === items.length - 1;
+      const share = isLast
+        ? remaining
+        : Math.max(0, Math.min(remaining, Math.round(value * perRowScale)));
       const existing = this.perTemplateContribution.get(template) ?? 0;
-      this.perTemplateContribution.set(template, existing + scaled);
-    }
+      this.perTemplateContribution.set(template, existing + share);
+      remaining -= share;
+    });
     return this.category.is_income ? -toBudget : toBudget;
   }
 

packages/loot-core/src/server/budget/goal-template.test.ts

@@ -67,8 +67,11 @@ function setupAqlForCategoryLookup(cat: CategoryEntity) {
     if (queryStr.includes('defaultCurrencyCode')) {
       return { data: [{ value: 'USD' }], dependencies: [] };
     }
-    if (queryStr.includes('categories')) {
-      return { data: [cat], dependencies: [] };
+    if (queryStr.includes('category_groups')) {
+      return {
+        data: [{ id: cat.group, hidden: false, categories: [cat] }],
+        dependencies: [],
+      };
     }
     return { data: [], dependencies: [] };
   });