Use patch for list updates (#1511)

* use patch for list updates

* i forget tbh

* create list if and only if it doesnt exist

* cleanup

* fix reference

* add peerDependency for amphora

inputs/autocomplete.vue

@@ -16,7 +16,7 @@
 <script>
   import _ from 'lodash';
   import item from './autocomplete-item.vue';
-  import { getProp, removeListItem } from '../lib/lists/helpers';
+  import { getItemIndex, getProp } from '../lib/lists/helpers';
 
   export default {
     props: ['args', 'select', 'query', 'focusIndex', 'updateFocusIndex', 'updateMatches', 'unselect', 'disabled'],
@@ -95,25 +95,21 @@
         });
       },
       removeFromList(item) {
-        let stringProperty;
-
         const listName = this.args.list;
 
         this.unselect();
 
-        return this.$store.dispatch('updateList', {
+        return this.$store.dispatch('patchList', {
           listName: listName,
           fn: (items) => {
-            stringProperty = getProp(items, 'text');
+            const stringProperty = getProp(items, 'text'),
+              index = getItemIndex(items, item, stringProperty);
 
-            if (stringProperty) {
-              return removeListItem(items, item, stringProperty);
-            } else {
-              return removeListItem(items, item);
+            if (index !== -1) {
+              return { remove: [items[index]] };
             }
           }
-        })
-          .then(this.fetchListItems);
+        }).then(list => this.listItems = _.map(list, item => _.isObject(item) ? item.text : item));
       }
     },
     mounted() {

inputs/simple-list.test.js

@@ -23,21 +23,22 @@ describe('SimpleList input', () => {
   test('Should decrease count when there is a removedItem', () => {
     wrapper.vm.removedItem = { text: 'test2' };
 
-    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual([
-      { text: 'test1', count: 1 },
-      { text: 'test2', count: 1 }
-    ]);
+    // since it exists, it decreases count
+    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual({
+      add: [{ text: 'test2', count: 1 }],
+      remove: [{ text: 'test2', count: 2 }]
+    });
   });
 
-  test("Should return same array if removedItem doesn't exist", () => {
+  test("Should return undefined removedItem doesn't exist", () => {
     wrapper.vm.removedItem = { text: 'something else' };
 
-    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual(items);
+    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual(undefined);
   });
 
-  test('Should return same array if removedItem is null', () => {
+  test('Should return undefined if removedItem is null', () => {
     wrapper.vm.removedItem = null;
 
-    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual(items);
+    expect(wrapper.vm.handleRemoveItem(items)).toStrictEqual(undefined);
   });
 });

inputs/simple-list.vue

@@ -108,7 +108,7 @@
   import simpleListItem from './simple-list-item.vue';
   import simpleListInput from './simple-list-input.vue';
   import attachedButton from './attached-button.vue';
-  import { addListItem, getItemIndex, getProp } from '../lib/lists/helpers';
+  import { getItemIndex, getProp } from '../lib/lists/helpers';
   import { DynamicEvents } from './mixins';
 
   const log = logger(__filename);
@@ -249,7 +249,7 @@
         if (this.args.autocomplete && this.args.autocomplete.allowRemove) {
           const listName = this.args.autocomplete.list;
 
-          return this.$store.dispatch('updateList', {
+          return this.$store.dispatch('patchList', {
             listName: listName,
             fn: this.handleRemoveItem
           });
@@ -268,57 +268,79 @@
           const itemIndex = getItemIndex(items, (this.removedItem || {}).text, 'text');
 
           if (itemIndex !== -1 && items[itemIndex][countProperty]) {
-          // decrease count if the item already exists in the list and count is more than 0
-            items[itemIndex][countProperty]--;
+            // decrease count if the item already exists in the list and count is more than 0
+
+            const old = items[itemIndex],
+              updated = _.cloneDeep(old);
+
+            updated[countProperty]--;
+
             this.removedItem = {};
+
+            return {
+              add: [updated],
+              remove: [old]
+            };
           }
         }
-
-        return items;
       },
       addItem(newItem) {
-        let countProperty,
-          itemIndex,
-          listName,
-          stringProperty;
-
         this.items.push(newItem);
         this.update(this.items);
 
         // only add items to the list if the schema allows it
         if (this.args.autocomplete && this.args.autocomplete.allowCreate) {
-          listName = this.args.autocomplete.list;
+          const listName = this.args.autocomplete.list;
 
-          return this.$store.dispatch('updateList', {
+          return this.$store.dispatch('patchList', {
             listName: listName,
             fn: (items) => {
-            // validate that the list has items with these properties
-              stringProperty = getProp(items, 'text');
-              countProperty = getProp(items, 'count');
+              const patch = {
+                  add: [],
+                  remove: []
+                },
+                // validate that the list has items with these properties
+                stringProperty = getProp(items, 'text'),
+                countProperty = getProp(items, 'count');
 
               if (stringProperty && countProperty) {
-                itemIndex = getItemIndex(items, newItem.text, 'text');
+                const itemIndex = getItemIndex(items, newItem.text, 'text');
 
                 if (itemIndex !== -1) {
-                // increase count if the item already exists in the list
-                  items[itemIndex][countProperty]++;
+                  // remove the old, add new object with new count
+                  const old = items[itemIndex],
+                    updated = _.cloneDeep(old);
 
-                  return items;
+                  patch.remove.push(old);
+
+                  // increase count if the item already exists in the list
+                  updated[countProperty]++;
+                  patch.add.push(updated);
+
+                  return patch;
                 } else {
-                // add item to the list
+                  // add item to the list
                   _.set(newItem, countProperty, 1);
 
-                  return addListItem(items, newItem);
+                  patch.add.push(newItem);
+
+                  return patch;
                 }
               } else if (_.isString(_.head(items))) {
-              // if the list is just an array of strings, just add the string
-              // property
-                return addListItem(items, newItem.text);
+                // if the list is just an array of strings, just add the string property
+
+                if (getItemIndex(items, newItem.text) === -1) {
+                  patch.add.push(newItem.text);
+
+                  return patch;
+                }
               } else if (items.length === 0) {
                 log.warn('The list is empty, unable to determine data structure. Adding item with default data structure.', { action: 'adding item to a list' });
                 _.set(newItem, 'count', 1);
 
-                return addListItem(items, newItem);
+                patch.add.push(newItem);
+
+                return patch;
               }
             }
           });

lib/core-data/api.js

@@ -399,6 +399,25 @@ export function saveList(prefix, listName, items) {
   })).then(expectJSONResult);
 }
 
+/**
+ * convenience function to save list data
+ * @param  {string} prefix
+ * @param  {string} listName
+ * @param {array} items
+ * @return {Promise}
+ */
+export function patchList(prefix, listName, patch) {
+  let uri = `${prefix}${listsRoute}${listName}`;
+
+  assertUri(uri);
+
+  return send(addJsonHeader({
+    method: 'PATCH',
+    url: uri,
+    body: JSON.stringify(patch)
+  })).then(expectJSONResult);
+}
+
 export function getMeta(uri) {
   assertUri(uri);
 

lib/lists/actions.js

@@ -1,6 +1,6 @@
 import _ from 'lodash';
 import { LIST_LOAD_PENDING, LIST_LOAD_SUCCESS, LIST_LOAD_FAIL } from './mutationTypes';
-import { getList as getListFromAPI, saveList } from '../core-data/api';
+import { getList as getListFromAPI, saveList, patchList as patchListAPI } from '../core-data/api';
 
 /**
  * @module lists
@@ -16,15 +16,52 @@ export function getList(store, listName) {
     .catch(error => store.commit(LIST_LOAD_FAIL, { listName, error }));
 }
 
-export function updateList(store, { listName, fn }) {
+/**
+ * if a list doesn't exist, creates the empty list
+ * @param {Object} store
+ * @param {string} listName
+ * @returns {Array<Object>}
+ */
+function getOrStartList(store, listName) {
   const prefix = _.get(store, 'state.site.prefix');
 
   store.commit(LIST_LOAD_PENDING, { listName });
 
   return getListFromAPI(prefix, listName)
-    .catch(() => []) // allow adding to a new list
+    .catch(error => {
+      // if it's a 404, that means the list doesn't exist and we need to create it
+      if (error && error.response && error.response.status === 404) {
+        return saveList(prefix, listName, []);
+      } else {
+        throw error;
+      }
+    });
+}
+
+export function updateList(store, { listName, fn }) {
+  const prefix = _.get(store, 'state.site.prefix');
+
+  return getOrStartList(store, listName)
     .then(listItems => fn(_.cloneDeep(listItems)))
     .then(listItems => saveList(prefix, listName, listItems))
     .then(listItems => store.commit(LIST_LOAD_SUCCESS, { listName, listItems }))
     .catch(error => store.commit(LIST_LOAD_FAIL, { listName, error }));
 }
+
+export function patchList(store, { listName, fn }) {
+  const prefix = _.get(store, 'state.site.prefix');
+
+  // get the list
+  return getOrStartList(store, listName)
+    // pass list to fn
+    .then(list => {
+      const patch = fn(list);
+
+      // fn needs to return a patch `{ remove: [], add: [] }`
+      return patch && (patch.add || patch.remove) ? patchListAPI(prefix, listName, patch) : list;
+    })
+    // patch api returns full list
+    .then(listItems => store.commit(LIST_LOAD_SUCCESS, { listName, listItems }))
+    // catch and log errors
+    .catch(error => store.commit(LIST_LOAD_FAIL, { listName, error }));
+}

lib/lists/actions.test.js

@@ -82,4 +82,31 @@ describe('list actions', () => {
       });
     });
   });
+
+  describe.only('patchList', () => {
+    const fn = lib.patchList,
+      prefix = 'domain.com',
+      listName = 'listName';
+
+    test('allows sends a PATCH request to api with remove and add in body', () => {
+      const store = {
+          commit: jest.fn(),
+          state: {
+            site: {
+              prefix
+            }
+          }
+        },
+        start = [1, 2, 3],
+        patch = { add: [4], remove: [1] },
+        end = [2, 3, 4];
+
+      api.getList.mockResolvedValue(start);
+      api.patchList.mockResolvedValue(end);
+
+      return fn(store, { listName: listName, fn: () => patch }).then(() => {
+        expect(api.patchList).toHaveBeenCalledWith(prefix, listName, patch);
+      });
+    });
+  });
 });

lib/lists/helpers.js

@@ -3,26 +3,6 @@ import logger from '../utils/log';
 
 const log = logger(__filename);
 
-/**
- * @param {array} items
- * @param {string | object} testItem
- * @param {string} testProperty (optional)
- * @returns {array}
- */
-export function addListItem(items, testItem) {
-  const index = getItemIndex(items, testItem);
-
-  if (index !== -1) {
-    log.info('This item already exists in this list.', { action: 'modifyList' });
-
-    return items;
-  } else {
-    items.push(testItem);
-
-    return items;
-  }
-}
-
 /**
 * check that item in a list have a property that is a string or number 0 and then return that property
 * @param {array} items
@@ -86,30 +66,6 @@ export function getItemIndex(items, testItem, testProperty) {
   return index;
 }
 
-/**
- * @param {array} items
- * @param {string | object} deletedItem
- * @param {string} testProperty (optional)
- * @returns {string}
- */
-export function removeListItem(items, deletedItem, testProperty) {
-  let index;
-
-  if (testProperty) {
-    index = getItemIndex(items, deletedItem, testProperty);
-  } else {
-    index = getItemIndex(items, deletedItem);
-  }
-
-  if (index !== -1) {
-    items.splice(index, 1);
-  } else {
-    log.error(`Cannot remove ${deletedItem} because it is not in the list.`, { action: 'modifyList' });
-  }
-
-  return items;
-}
-
 /**
  * when fetching or updating pages, make sure they're sorted
  * note: this will also place them into a General category if they don't currently have categories