Extract out note template logic from goaltemplates.ts, refactor and add unit tests (#3221)

Signed-off-by: Alex Walker <walker.c.alex@gmail.com>

packages/loot-core/src/server/budget/goaltemplates.ts

@@ -1,12 +1,11 @@
 // @ts-strict-ignore
 import { Notification } from '../../client/state-types/notifications';
 import * as monthUtils from '../../shared/months';
-import { integerToAmount, amountToInteger } from '../../shared/util';
+import { amountToInteger, integerToAmount } from '../../shared/util';
 import * as db from '../db';
 import { batchMessages } from '../sync';
 
-import { setBudget, getSheetValue, isReflectBudget, setGoal } from './actions';
-import { parse } from './goal-template.pegjs';
+import { getSheetValue, isReflectBudget, setBudget, setGoal } from './actions';
 import { goalsAverage } from './goals/goalsAverage';
 import { goalsBy } from './goals/goalsBy';
 import { goalsPercentage } from './goals/goalsPercentage';
@@ -15,9 +14,9 @@ import { goalsSchedule } from './goals/goalsSchedule';
 import { goalsSimple } from './goals/goalsSimple';
 import { goalsSpend } from './goals/goalsSpend';
 import { goalsWeek } from './goals/goalsWeek';
+import { checkTemplates, storeTemplates } from './template-notes';
 
 const TEMPLATE_PREFIX = '#template';
-const GOAL_PREFIX = '#goal';
 
 export async function applyTemplate({ month }) {
   await storeTemplates();
@@ -61,9 +60,9 @@ export function runCheckTemplates() {
 async function getCategories() {
   return await db.all(
     `
-    SELECT categories.* FROM categories 
-    INNER JOIN category_groups on categories.cat_group = category_groups.id 
-    WHERE categories.tombstone = 0 AND categories.hidden = 0 
+    SELECT categories.* FROM categories
+    INNER JOIN category_groups on categories.cat_group = category_groups.id
+    WHERE categories.tombstone = 0 AND categories.hidden = 0
     AND category_groups.hidden = 0
     `,
   );
@@ -125,27 +124,6 @@ async function resetCategoryTargets(month, category) {
   });
 }
 
-async function storeTemplates() {
-  //stores the template definitions to the database
-  const templates = await getCategoryTemplates(null);
-  const categories = await getCategories();
-
-  for (let c = 0; c < categories.length; c++) {
-    const template = templates[categories[c].id];
-    if (template) {
-      await db.update('categories', {
-        id: categories[c].id,
-        goal_def: JSON.stringify(template),
-      });
-    } else {
-      await db.update('categories', {
-        id: categories[c].id,
-        goal_def: null,
-      });
-    }
-  }
-}
-
 async function getTemplates(category, directive: string) {
   //retrieves template definitions from the database
   const goal_def = await db.all(
@@ -438,42 +416,6 @@ async function processGoals(goals, month, category?) {
     }
   }
 }
-async function getCategoryTemplates(category) {
-  const templates = {};
-
-  let notes = await db.all(
-    `
-    SELECT * FROM notes 
-    WHERE lower(note) like '%${TEMPLATE_PREFIX}%' 
-    OR lower(note) like '%${GOAL_PREFIX}%'
-     `,
-  );
-  if (category) notes = notes.filter(n => n.id === category.id);
-
-  for (let n = 0; n < notes.length; n++) {
-    const lines = notes[n].note.split('\n');
-    const template_lines = [];
-    for (let l = 0; l < lines.length; l++) {
-      const line = lines[l].trim();
-      if (
-        !line.toLowerCase().startsWith(TEMPLATE_PREFIX) &&
-        !line.toLowerCase().startsWith(GOAL_PREFIX)
-      ) {
-        continue;
-      }
-      try {
-        const parsed = parse(line);
-        template_lines.push(parsed);
-      } catch (e) {
-        template_lines.push({ type: 'error', line, error: e });
-      }
-    }
-    if (template_lines.length) {
-      templates[notes[n].id] = template_lines;
-    }
-  }
-  return templates;
-}
 
 async function applyCategoryTemplate(
   category,
@@ -709,54 +651,3 @@ async function applyCategoryTemplate(
   console.log(str);
   return { amount: to_budget, errors };
 }
-
-async function checkTemplates(): Promise<Notification> {
-  const category_templates = await getCategoryTemplates(null);
-  const errors = [];
-
-  const categories = await db.all(
-    'SELECT * FROM v_categories WHERE tombstone = 0',
-  );
-  let all_schedule_names = await db.all(
-    'SELECT name from schedules WHERE name NOT NULL AND tombstone = 0',
-  );
-  all_schedule_names = all_schedule_names.map(v => v.name);
-
-  // run through each line and see if its an error
-  for (let c = 0; c < categories.length; c++) {
-    const category = categories[c];
-    const template = category_templates[category.id];
-
-    if (template) {
-      for (let l = 0; l < template.length; l++) {
-        //check for basic error
-        if (template[l].type === 'error') {
-          errors.push(category.name + ': ' + template[l].line);
-        }
-        // check schedule name error
-        if (template[l].type === 'schedule') {
-          if (!all_schedule_names.includes(template[l].name)) {
-            errors.push(
-              category.name +
-                ': Schedule “' +
-                template[l].name +
-                '” does not exist',
-            );
-          }
-        }
-      }
-    }
-  }
-  if (errors.length) {
-    return {
-      sticky: true,
-      message: `There were errors interpreting some templates:`,
-      pre: errors.join('\n\n'),
-    };
-  } else {
-    return {
-      type: 'message',
-      message: 'All templates passed! 🎉',
-    };
-  }
-}

packages/loot-core/src/server/budget/statements.ts

@@ -0,0 +1,47 @@
+import * as db from '../db';
+import { Schedule } from '../db/types';
+
+import { GOAL_PREFIX, TEMPLATE_PREFIX } from './template-notes';
+
+/* eslint-disable rulesdir/typography */
+export async function resetCategoryGoalDefsWithNoTemplates(): Promise<void> {
+  await db.run(
+    `
+      UPDATE categories
+      SET goal_def = NULL
+      WHERE id NOT IN (SELECT n.id
+                       FROM notes n
+                       WHERE lower(note) LIKE '%${TEMPLATE_PREFIX}%'
+                          OR lower(note) LIKE '%${GOAL_PREFIX}%')
+    `,
+  );
+}
+
+/* eslint-enable rulesdir/typography */
+
+export type CategoryWithTemplateNote = {
+  id: string;
+  name: string;
+  note: string;
+};
+
+export async function getCategoriesWithTemplateNotes(): Promise<
+  CategoryWithTemplateNote[]
+> {
+  return await db.all(
+    `
+      SELECT c.id AS id, c.name as name, n.note AS note
+      FROM notes n
+             JOIN categories c ON n.id = c.id
+      WHERE c.id = n.id
+        AND (lower(note) LIKE '%${TEMPLATE_PREFIX}%'
+        OR lower(note) LIKE '%${GOAL_PREFIX}%')
+    `,
+  );
+}
+
+export async function getActiveSchedules(): Promise<Schedule[]> {
+  return await db.all(
+    'SELECT id, rule, active, completed, posts_transaction, tombstone, name from schedules WHERE name NOT NULL AND tombstone = 0',
+  );
+}

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

@@ -0,0 +1,229 @@
+import * as db from '../db';
+import { Schedule } from '../db/types';
+
+import {
+  CategoryWithTemplateNote,
+  getActiveSchedules,
+  getCategoriesWithTemplateNotes,
+  resetCategoryGoalDefsWithNoTemplates,
+} from './statements';
+import { checkTemplates, storeTemplates } from './template-notes';
+
+jest.mock('../db');
+jest.mock('./statements');
+
+function mockGetTemplateNotesForCategories(
+  templateNotes: CategoryWithTemplateNote[],
+) {
+  (getCategoriesWithTemplateNotes as jest.Mock).mockResolvedValue(
+    templateNotes,
+  );
+}
+
+function mockGetActiveSchedules(schedules: Schedule[]) {
+  (getActiveSchedules as jest.Mock).mockResolvedValue(schedules);
+}
+
+function mockDbUpdate() {
+  (db.update as jest.Mock).mockResolvedValue(undefined);
+}
+
+describe('storeTemplates', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const testCases = [
+    {
+      description: 'Stores templates for categories with valid template notes',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#template 10',
+        },
+      ],
+      expectedTemplates: [
+        {
+          type: 'simple',
+          monthly: 10,
+          limit: null,
+          priority: 0,
+          directive: 'template',
+        },
+      ],
+    },
+    {
+      description:
+        'Stores templates for categories with valid goal directive template notes',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#goal 10',
+        },
+      ],
+      expectedTemplates: [
+        {
+          type: 'simple',
+          amount: 10,
+          priority: null,
+          directive: 'goal',
+        },
+      ],
+    },
+    {
+      description: 'Does not store empty template notes',
+      mockTemplateNotes: [{ id: 'cat1', name: 'Category 1', note: '' }],
+      expectedTemplates: [],
+    },
+    {
+      description: 'Does not store non template notes',
+      mockTemplateNotes: [
+        { id: 'cat1', name: 'Category 1', note: 'Not a template note' },
+      ],
+      expectedTemplates: [],
+    },
+  ];
+
+  it.each(testCases)(
+    '$description',
+    async ({ mockTemplateNotes, expectedTemplates }) => {
+      // Given
+      mockGetTemplateNotesForCategories(mockTemplateNotes);
+      mockDbUpdate();
+
+      // When
+      await storeTemplates();
+
+      // Then
+      if (expectedTemplates.length === 0) {
+        expect(db.update).not.toHaveBeenCalled();
+        expect(resetCategoryGoalDefsWithNoTemplates).toHaveBeenCalled();
+        return;
+      }
+
+      mockTemplateNotes.forEach(({ id }) => {
+        expect(db.update).toHaveBeenCalledWith('categories', {
+          id,
+          goal_def: JSON.stringify(expectedTemplates),
+        });
+      });
+      expect(resetCategoryGoalDefsWithNoTemplates).toHaveBeenCalled();
+    },
+  );
+});
+
+describe('checkTemplates', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const testCases = [
+    {
+      description: 'Returns success message when templates pass',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#template 10',
+        },
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#template schedule Mock Schedule 1',
+        },
+      ],
+      mockSchedules: mockSchedules(),
+      expected: {
+        type: 'message',
+        message: 'All templates passed! 🎉',
+      },
+    },
+    {
+      description: 'Skips notes that are not templates',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: 'Not a template note',
+        },
+      ],
+      mockSchedules: mockSchedules(),
+      expected: {
+        type: 'message',
+        message: 'All templates passed! 🎉',
+      },
+    },
+    {
+      description: 'Returns errors for templates with parsing errors',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#template broken template',
+        },
+      ],
+      mockSchedules: mockSchedules(),
+      expected: {
+        sticky: true,
+        message: 'There were errors interpreting some templates:',
+        pre: 'Category 1: #template broken template',
+      },
+    },
+    {
+      description: 'Returns errors for non-existent schedules',
+      mockTemplateNotes: [
+        {
+          id: 'cat1',
+          name: 'Category 1',
+          note: '#template schedule Non-existent Schedule',
+        },
+      ],
+      mockSchedules: mockSchedules(),
+      expected: {
+        sticky: true,
+        message: 'There were errors interpreting some templates:',
+        pre: 'cat1: Schedule “Non-existent Schedule” does not exist',
+      },
+    },
+  ];
+
+  it.each(testCases)(
+    '$description',
+    async ({ mockTemplateNotes, mockSchedules, expected }) => {
+      // Given
+      mockGetTemplateNotesForCategories(mockTemplateNotes);
+      mockGetActiveSchedules(mockSchedules);
+
+      // When
+      const result = await checkTemplates();
+
+      // Then
+      expect(result).toEqual(expected);
+    },
+  );
+});
+
+function mockSchedules(): Schedule[] {
+  return [
+    {
+      id: 'mock-schedule-1',
+      rule: 'mock-rule',
+      active: 1,
+      completed: 0,
+      posts_transaction: 0,
+      tombstone: 0,
+      name: 'Mock Schedule 1',
+    },
+    {
+      id: 'mock-schedule-2',
+      rule: 'mock-rule',
+      active: 1,
+      completed: 0,
+      posts_transaction: 0,
+      tombstone: 0,
+      name: 'Mock Schedule 2',
+    },
+  ];
+}

packages/loot-core/src/server/budget/template-notes.ts

@@ -0,0 +1,117 @@
+import { Notification } from '../../client/state-types/notifications';
+import * as db from '../db';
+
+import { parse } from './goal-template.pegjs';
+import {
+  CategoryWithTemplateNote,
+  getActiveSchedules,
+  getCategoriesWithTemplateNotes,
+  resetCategoryGoalDefsWithNoTemplates,
+} from './statements';
+import { Template } from './types/templates';
+
+export const TEMPLATE_PREFIX = '#template';
+export const GOAL_PREFIX = '#goal';
+
+export async function storeTemplates(): Promise<void> {
+  const categoriesWithTemplates = await getCategoriesWithTemplates();
+
+  for (const { id, templates } of categoriesWithTemplates) {
+    const goalDefs = JSON.stringify(templates);
+
+    await db.update('categories', {
+      id,
+      goal_def: goalDefs,
+    });
+  }
+
+  await resetCategoryGoalDefsWithNoTemplates();
+}
+
+type CategoryWithTemplates = {
+  id: string;
+  name: string;
+  templates: Template[];
+};
+
+export async function checkTemplates(): Promise<Notification> {
+  const categoryWithTemplates = await getCategoriesWithTemplates();
+  const schedules = await getActiveSchedules();
+  const scheduleNames = schedules.map(({ name }) => name);
+  const errors: string[] = [];
+
+  categoryWithTemplates.forEach(({ id, name, templates }) => {
+    templates.forEach(template => {
+      if (template.type === 'error') {
+        errors.push(`${name}: ${template.line}`);
+      } else if (
+        template.type === 'schedule' &&
+        !scheduleNames.includes(template.name)
+      ) {
+        errors.push(`${id}: Schedule “${template.name}” does not exist`);
+      }
+    });
+  });
+
+  if (errors.length) {
+    return {
+      sticky: true,
+      message: 'There were errors interpreting some templates:',
+      pre: errors.join('\n\n'),
+    };
+  }
+
+  return {
+    type: 'message',
+    message: 'All templates passed! 🎉',
+  };
+}
+
+async function getCategoriesWithTemplates(): Promise<CategoryWithTemplates[]> {
+  const templatesForCategory: CategoryWithTemplates[] = [];
+  const templateNotes = await getCategoriesWithTemplateNotes();
+
+  templateNotes.forEach(({ id, name, note }: CategoryWithTemplateNote) => {
+    if (!note) {
+      return;
+    }
+
+    const parsedTemplates: Template[] = [];
+
+    note.split('\n').forEach(line => {
+      const trimmedLine = line.trim();
+
+      if (
+        !trimmedLine.startsWith(TEMPLATE_PREFIX) &&
+        !trimmedLine.startsWith(GOAL_PREFIX)
+      ) {
+        return;
+      }
+
+      try {
+        const parsedTemplate: Template = parse(trimmedLine);
+
+        parsedTemplates.push(parsedTemplate);
+      } catch (e: unknown) {
+        parsedTemplates.push({
+          type: 'error',
+          directive: 'error',
+          line,
+          error: (e as Error).message,
+        });
+      }
+    });
+
+    if (!parsedTemplates.length) {
+      return;
+    }
+
+    templatesForCategory.push({
+      id,
+      name,
+      templates: parsedTemplates,
+    });
+  });
+
+  return templatesForCategory;
+}

packages/loot-core/src/server/budget/types/templates.d.ts

@@ -0,0 +1,82 @@
+interface BaseTemplate {
+  type: string;
+  priority?: number;
+  directive: string;
+}
+
+interface PercentageTemplate extends BaseTemplate {
+  type: 'percentage';
+  percent: number;
+  previous: boolean;
+  category: string;
+}
+
+interface WeekTemplate extends BaseTemplate {
+  type: 'week';
+  amount: number;
+  weeks: number | null;
+  starting: string;
+  limit?: { amount: number; hold: boolean };
+}
+
+interface ByTemplate extends BaseTemplate {
+  type: 'by';
+  amount: number;
+  month: string;
+  repeat?: { annual: boolean; repeat?: number };
+  from?: string;
+}
+
+interface SpendTemplate extends BaseTemplate {
+  type: 'spend';
+  amount: number;
+  month: string;
+  from: string;
+  repeat?: { annual: boolean; repeat?: number };
+}
+
+interface SimpleTemplate extends BaseTemplate {
+  type: 'simple';
+  monthly?: number;
+  limit?: { amount: number; hold: boolean };
+}
+
+interface ScheduleTemplate extends BaseTemplate {
+  type: 'schedule';
+  name: string;
+  full?: boolean;
+}
+
+interface RemainderTemplate extends BaseTemplate {
+  type: 'remainder';
+  weight: number;
+  limit?: { amount: number; hold: boolean };
+}
+
+interface AverageTemplate extends BaseTemplate {
+  type: 'average';
+  amount: number;
+}
+
+interface GoalTemplate extends BaseTemplate {
+  type: 'simple';
+  amount: number;
+}
+
+interface ErrorTemplate extends BaseTemplate {
+  type: 'error';
+  line: string;
+  error: string;
+}
+
+export type Template =
+  | PercentageTemplate
+  | WeekTemplate
+  | ByTemplate
+  | SpendTemplate
+  | SimpleTemplate
+  | ScheduleTemplate
+  | RemainderTemplate
+  | AverageTemplate
+  | GoalTemplate
+  | ErrorTemplate;

packages/loot-core/src/server/db/types.d.ts

@@ -0,0 +1,9 @@
+export type Schedule = {
+  id: string;
+  rule: string;
+  active: number;
+  completed: number;
+  posts_transaction: number;
+  tombstone: number;
+  name: string | null;
+};

upcoming-release-notes/3221.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [ ACWalker ]
+---
+
+Extract, refactor and test note handling logic from `goaltemplates.ts` file.