fix: fixed bug when editing implicit parent categories, improved testing

fixes ActivityWatch/activitywatch#580

Author: ErikBjare

src/store/modules/categories.js

@@ -1,9 +1,10 @@
-import * as _ from 'lodash';
+import _ from 'lodash';
 import {
   saveClasses,
   loadClasses,
   defaultCategories,
   build_category_hierarchy,
+  createMissingParents,
 } from '~/util/classes';
 
 // initial state
@@ -19,6 +20,7 @@ const getters = {
     return _.sortBy(hier, [c => c.id || 0]);
   },
   all_categories: state => {
+    // Returns a list of category names (a list of list of strings)
     return _.uniqBy(
       _.flatten(
         state.classes.map(c => {
@@ -52,6 +54,8 @@ const actions = {
 // mutations
 const mutations = {
   loadClasses(state, classes) {
+    classes = createMissingParents(classes);
+
     let i = 0;
     state.classes = classes.map(c => Object.assign(c, { id: i++ }));
     console.log('Loaded classes:', state.classes);
@@ -66,8 +70,8 @@ const mutations = {
   },
   updateClass(state, new_class) {
     console.log('Updating class:', new_class);
-
     const old_class = _.cloneDeep(state.classes[new_class.id]);
+
     if (new_class.id === undefined || new_class.id === null) {
       new_class.id = _.max(_.map(state.classes, 'id')) + 1;
       state.classes.push(new_class);
@@ -97,7 +101,11 @@ const mutations = {
   },
   restoreDefaultClasses(state) {
     let i = 0;
-    state.classes = defaultCategories.map(c => Object.assign(c, { id: i++ }));
+    state.classes = createMissingParents(defaultCategories).map(c => Object.assign(c, { id: i++ }));
+    state.classes_unsaved_changes = true;
+  },
+  clearAll(state) {
+    state.classes = [];
     state.classes_unsaved_changes = true;
   },
   saveCompleted(state) {

src/util/classes.ts

@@ -1,4 +1,4 @@
-const _ = require('lodash');
+import _ from 'lodash';
 
 const level_sep = '>';
 
@@ -20,6 +20,8 @@ export interface Category {
   children?: Category[];
 }
 
+// The default categories
+// Should be run through createMissingParents before being used in most cases.
 export const defaultCategories: Category[] = [
   {
     name: ['Work'],
@@ -53,38 +55,44 @@ export const defaultCategories: Category[] = [
   { name: ['Uncategorized'], rule: { type: null }, data: { color: '#ccc' } },
 ];
 
-export function build_category_hierarchy(classes: Category[]): Category[] {
-  function annotate(c: Category) {
-    const ch = c.name;
-    c.name_pretty = ch.join(level_sep);
-    c.subname = ch.slice(-1)[0];
-    c.parent = ch.length > 1 ? ch.slice(0, -1) : null;
-    c.depth = ch.length - 1;
-    return c;
-  }
-
-  const new_classes = classes.slice().map(c => annotate(c));
+function annotate(c: Category) {
+  const ch = c.name;
+  c.name_pretty = ch.join(level_sep);
+  c.subname = ch.slice(-1)[0];
+  c.parent = ch.length > 1 ? ch.slice(0, -1) : null;
+  c.depth = ch.length - 1;
+  return c;
+}
 
-  // Insert dangling/undefined parents
-  const all_full_names = new Set(new_classes.map(c => c.name.join(level_sep)));
+export function createMissingParents(classes: Category[]): Category[] {
+  // Creates parents for categories that are missing theirs (implicit parents)
+  classes = _.cloneDeep(classes);
+  classes = classes.slice().map(c => annotate(c));
+  const all_full_names = new Set(classes.map(c => c.name.join(level_sep)));
 
-  function createMissingParents(children) {
+  function _createMissing(children: Category[]) {
     children
       .map(c => c.parent)
       .filter(p => !!p)
       .map(p => {
         const name = p.join(level_sep);
         if (p && !all_full_names.has(name)) {
           const new_parent = annotate({ name: p, rule: { type: null } });
+          //console.log('Creating missing parent:', new_parent);
           classes.push(new_parent);
           all_full_names.add(name);
           // New parent might not be top-level, so we need to recurse
-          createMissingParents([new_parent]);
+          _createMissing([new_parent]);
         }
       });
   }
 
-  createMissingParents(new_classes);
+  _createMissing(classes);
+  return classes;
+}
+
+export function build_category_hierarchy(classes: Category[]): Category[] {
+  classes = createMissingParents(classes);
 
   function assignChildren(classes_at_level: Category[]) {
     return classes_at_level.map(cls => {

test/unit/store/categories.test.node.js

@@ -1,6 +1,11 @@
 import store from '~/store';
+import { createMissingParents, defaultCategories } from '~/util/classes';
 
-test('loads categories', () => {
+beforeEach(() => {
+  store.commit('categories/clearAll');
+});
+
+test('loads default categories', () => {
   // Load categories
   expect(store.state.categories.classes).toHaveLength(0);
   store.commit('categories/restoreDefaultClasses');
@@ -24,3 +29,46 @@ test('loads categories', () => {
   expect(store.getters['categories/all_categories']).not.toHaveLength(0);
   expect(store.getters['categories/classes_hierarchy']).not.toHaveLength(0);
 });
+
+test('loads custom categories', () => {
+  expect(store.state.categories.classes).toHaveLength(0);
+  store.commit('categories/loadClasses', [{ name: ['Test'] }]);
+  expect(store.getters['categories/all_categories']).toHaveLength(1);
+});
+
+test('get category hierarchy', () => {
+  store.commit('categories/restoreDefaultClasses');
+  const hier = store.getters['categories/classes_hierarchy'];
+  expect(hier).not.toHaveLength(0);
+});
+
+test('create missing parents', () => {
+  const cats = createMissingParents([{ name: ['Test', 'Subcat'] }]);
+  expect(cats).toHaveLength(2);
+});
+
+test('update implicit parent category', () => {
+  // The default categories have implicit Media and Comms categories (with 'No Rule')
+  // Tests against https://github.com/ActivityWatch/activitywatch/issues/580
+  store.commit('categories/restoreDefaultClasses');
+
+  // Check that the label is available
+  expect(store.getters['categories/all_categories']).toContainEqual(['Media']);
+
+  // Get category and modify it
+  const media_cat = store.getters['categories/get_category'](['Media']);
+  expect(media_cat.id).not.toBeUndefined();
+  const new_media_cat = { ...media_cat, name: ['Media2'], data: { test: true } };
+  store.commit('categories/updateClass', new_media_cat);
+
+  // Check that category was modified correctly
+  const media2_cat = store.getters['categories/get_category'](['Media2']);
+  expect(media2_cat.data.test).toBe(true);
+
+  // Check that child was modified correctly when parent name changed
+  const music_cat = store.getters['categories/get_category'](['Media2', 'Music']);
+  expect(music_cat.id).not.toBeUndefined();
+
+  // Check that defaultCategories haven't mutated
+  expect(defaultCategories.map(c => c.name)).toContainEqual(['Media', 'Music']);
+});