Refactor systemTag changes to stop directly mutating note object (#…

…1615)

Previously we have been directly mutating the note object in the action
creator when toggling a note's pinned status, markdown status, publish
status, and trash status.

This mutation can lead to concurrency defects which expose themselves as
inconsistent UI state. This breaks our Redux model which assumes that
all UI updates happen atomically.

In this patch we're building a new note object when toggling these
system tags in order to maintain our consistency.

At the same time I have removed the thunks from the action creators
since they weren't doing anything and didn't need to exist. This
shouldn't have a noticeable impact on the application.

lib/flux/app-state.js

@@ -7,6 +7,25 @@ import analytics from '../analytics';
 
 const debug = Debug('appState');
 
+const toggleSystemTag = (note, systemTag, shouldHaveTag) => {
+  const {
+    data: { systemTags = [] },
+  } = note;
+  const hasTagAlready = systemTags.includes(systemTag);
+
+  return hasTagAlready !== shouldHaveTag
+    ? {
+        ...note,
+        data: {
+          ...note.data,
+          systemTags: shouldHaveTag
+            ? [...systemTags, systemTag]
+            : systemTags.filter(tag => tag !== systemTag),
+        },
+      }
+    : note;
+};
+
 export const actionMap = new ActionMap({
   namespace: 'App',
 
@@ -291,21 +310,14 @@ export const actionMap = new ActionMap({
     },
 
     pinNote: {
-      creator({ noteBucket, note, pin }) {
-        return dispatch => {
-          let systemTags = note.data.systemTags || [];
-          let pinnedTagIndex = systemTags.indexOf('pinned');
+      creator({ noteBucket, note, pin: shouldPin }) {
+        const updated = toggleSystemTag(note, 'pinned', shouldPin);
 
-          if (pin && pinnedTagIndex === -1) {
-            note.data.systemTags.push('pinned');
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
-          } else if (!pin && pinnedTagIndex !== -1) {
-            note.data.systemTags = systemTags.filter(tag => tag !== 'pinned');
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
-          }
-        };
+        if (note !== updated) {
+          noteBucket.update(note.id, updated.data);
+        }
+
+        return this.action('selectNote', { note: updated });
       },
     },
 
@@ -334,44 +346,32 @@ export const actionMap = new ActionMap({
     },
 
     markdownNote: {
-      creator({ noteBucket, note, markdown }) {
-        return dispatch => {
-          let systemTags = note.data.systemTags || [];
-          let markdownTagIndex = systemTags.indexOf('markdown');
+      creator({ noteBucket, note, markdown: shouldEnableMarkdown }) {
+        const updated = toggleSystemTag(note, 'markdown', shouldEnableMarkdown);
 
-          if (markdown && markdownTagIndex === -1) {
-            note.data.systemTags.push('markdown');
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
-          } else if (!markdown && markdownTagIndex !== -1) {
-            note.data.systemTags = systemTags.filter(tag => tag !== 'markdown');
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
-          }
-        };
+        if (updated !== note) {
+          noteBucket.update(note.id, updated.data);
+        }
+
+        return this.action('selectNote', { note: updated });
       },
     },
 
     publishNote: {
-      creator({ noteBucket, note, publish }) {
-        return dispatch => {
-          let systemTags = note.data.systemTags || [];
-          let tagIndex = systemTags.indexOf('published');
+      creator({ noteBucket, note, publish: shouldPublish }) {
+        const updated = toggleSystemTag(note, 'published', shouldPublish);
 
-          if (publish && tagIndex === -1) {
-            note.data.systemTags.push('published');
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
+        if (updated !== note) {
+          noteBucket.update(note.id, updated.data);
+
+          if (shouldPublish) {
             analytics.tracks.recordEvent('editor_note_published');
-          } else if (!publish && tagIndex !== -1) {
-            note.data.systemTags = systemTags.filter(
-              tag => tag !== 'published'
-            );
-            noteBucket.update(note.id, note.data);
-            dispatch(this.action('selectNote', { note }));
+          } else {
             analytics.tracks.recordEvent('editor_note_unpublished');
           }
-        };
+        }
+
+        return this.action('selectNote', { note: updated });
       },
     },