Refactor: Move revision-fetching into new Simperium middleware (#1921)

As part of our efforts to replace the "wonky app state" we have come across
a few different elements of state that are closer matches to middleware behaviors
than reducer behaviors. Fetching the revisions for a note is one such activity.

Revisions should be gathered whenever we open the revision slider. Previously
when opening the slider we were making two dispatches: one to set the slider
visibility and another to fetch the revisions. Two dispatches are not necessary
and the fetching is purely as side-effect of opening the panel.

In this patch we're creating a new middleware file where we plan to move all
Simperium side-effects eventually. Right now it only watches for appropriate
changes to the revision slider visibility.

Note that the use of middleware has removed the need to chain dispatch calls
as was previously done between `noteRevisions` and `noteRevisionsLoaded`. We
now have a reactive response to `REVISIONS_TOGGLE` and a more declarative
`STORE_REVISIONS` action.

Because revision state is so closely related to this work we are also moving
the revision state into Redux proper and fixing a bug or two in the process.

 - Previously the revision slider would stay open when selecting another
   note or when creating a new note through the keyboard shortcuts. It did
   this because the React component was imperatively calling to close the
   panel on `outsideClick`. Now it properly closes when using the keyboard
   shortcuts because the `showRevisions` reducer is now listening for those
   actions. We could have left it open but I made the decision that it was
   best to prevent confusion to close it. In `develop`, even though the
   slider stays open it fails to fetch the revisions when changing notes.
   This too is due to the existing imperative and confusing data flows.

 - Previously it was possible to select another note in the time it took
   to download the note revisions. In this race you could potentially see
   the revisions for another note. Now this has been resolved by only
   storing the note revisions if we are still on the same note as when
   they were requested.

lib/app-layout/index.tsx

@@ -27,7 +27,6 @@ type OwnProps = {
   isSmallScreen: boolean;
   note: T.NoteEntity;
   noteBucket: T.Bucket<T.Note>;
-  revisions: T.NoteEntity[];
   onUpdateContent: Function;
   syncNote: Function;
 };
@@ -45,7 +44,6 @@ export const AppLayout: FunctionComponent<Props> = ({
   isNoteOpen,
   isSmallScreen,
   noteBucket,
-  revisions,
   onUpdateContent,
   syncNote,
 }) => {
@@ -72,10 +70,7 @@ export const AppLayout: FunctionComponent<Props> = ({
           <NoteList noteBucket={noteBucket} isSmallScreen={isSmallScreen} />
         </div>
         <div className="app-layout__note-column theme-color-bg theme-color-fg theme-color-border">
-          <RevisionSelector
-            revisions={revisions || []}
-            onUpdateContent={onUpdateContent}
-          />
+          <RevisionSelector onUpdateContent={onUpdateContent} />
           <NoteToolbarContainer
             noteBucket={noteBucket}
             toolbar={<NoteToolbar />}

lib/app.tsx

@@ -473,7 +473,6 @@ export const App = connect(
                 isNoteInfoOpen={showNoteInfo}
                 isSmallScreen={isSmallScreen}
                 noteBucket={noteBucket}
-                revisions={state.revisions}
                 onUpdateContent={this.onUpdateContent}
                 syncNote={this.syncNote}
               />

lib/boot.ts

@@ -10,6 +10,7 @@ import Debug from 'debug';
 import { initClient } from './client';
 import getConfig from '../get-config';
 import store from './state';
+import * as simperiumMiddleware from './state/simperium/middleware';
 import {
   reset as resetAuth,
   setAuthorized,
@@ -231,6 +232,9 @@ if (cookie.email && config.is_app_engine) {
 }
 
 Modal.setAppElement('#root');
+simperiumMiddleware.storeBuckets({
+  note: client.bucket('note'),
+});
 
 render(
   React.createElement(Provider, { store }, React.createElement(App, props)),

lib/flux/app-state.ts

@@ -3,6 +3,7 @@ import update from 'react-addons-update';
 import Debug from 'debug';
 import ActionMap from './action-map';
 import analytics from '../analytics';
+import actions from '../state/actions';
 
 import { AppState, State } from '../state';
 import * as T from '../types';
@@ -36,7 +37,6 @@ const initialState: AppState = {
   previousIndex: -1,
   notes: null,
   tags: [],
-  revision: null,
   dialogs: [],
   nextDialogKey: 0,
   unsyncedNoteIds: [], // note bucket only
@@ -232,7 +232,7 @@ export const actionMap = new ActionMap({
       creator({ noteBucket, noteId, hasRemoteUpdate = false }) {
         return dispatch => {
           noteBucket.get(noteId, (e, note) => {
-            dispatch(this.action('selectNote', { note, hasRemoteUpdate }));
+            dispatch(actions.ui.selectNote(note, { hasRemoteUpdate }));
           });
         };
       },
@@ -246,16 +246,10 @@ export const actionMap = new ActionMap({
           noteBucket.update(note.id, updated.data);
         }
 
-        return this.action('selectNote', { note: updated });
+        return actions.ui.selectNote(updated);
       },
     },
 
-    setRevision(state: AppState, { revision }) {
-      return update(state, {
-        revision: { $set: revision },
-      });
-    },
-
     markdownNote: {
       creator({ noteBucket, note, markdown: shouldEnableMarkdown }) {
         const updated = toggleSystemTag(note, 'markdown', shouldEnableMarkdown);
@@ -264,7 +258,7 @@ export const actionMap = new ActionMap({
           noteBucket.update(note.id, updated.data);
         }
 
-        return this.action('selectNote', { note: updated });
+        return actions.ui.selectNote(updated);
       },
     },
 
@@ -282,17 +276,10 @@ export const actionMap = new ActionMap({
           }
         }
 
-        return this.action('selectNote', { note: updated });
+        return actions.ui.selectNote(updated);
       },
     },
 
-    selectNote(state: AppState) {
-      return update(state, {
-        revision: { $set: null },
-        revisions: { $set: null },
-      });
-    },
-
     /**
      * A note is being changed from somewhere else! If the same
      * note is also open and being edited, we need to make sure
@@ -380,26 +367,6 @@ export const actionMap = new ActionMap({
       },
     },
 
-    noteRevisions: {
-      creator({
-        noteBucket,
-        note,
-      }: {
-        noteBucket: T.Bucket<T.Note>;
-        note: T.NoteEntity;
-      }) {
-        return dispatch => {
-          noteBucket.getRevisions(note.id, (e, revisions) => {
-            if (e) {
-              return console.warn('Failed to load revisions', e); // eslint-disable-line no-console
-            }
-
-            dispatch(this.action('noteRevisionsLoaded', { revisions }));
-          });
-        };
-      },
-    },
-
     emptyTrash: {
       creator({ noteBucket }: { noteBucket: T.Bucket<T.Note> }) {
         return (dispatch, getState: () => State) => {
@@ -414,12 +381,6 @@ export const actionMap = new ActionMap({
       },
     },
 
-    noteRevisionsLoaded(state: AppState, { revisions }) {
-      return update(state, {
-        revisions: { $set: revisions },
-      });
-    },
-
     tagsLoaded(
       state: AppState,
       { tags, sortTagsAlpha }: { tags: T.TagEntity[]; sortTagsAlpha: boolean }

lib/note-detail/index.tsx

@@ -239,7 +239,7 @@ const mapStateToProps: S.MapState<StateProps> = ({
   settings,
 }) => ({
   dialogs: state.dialogs,
-  note: state.revision || ui.note,
+  note: ui.selectedRevision || ui.note,
   showNoteInfo: ui.showNoteInfo,
   spellCheckEnabled: settings.spellCheckEnabled,
 });

lib/note-editor/index.tsx

@@ -168,14 +168,14 @@ export class NoteEditor extends Component<Props> {
 const mapStateToProps: S.MapState<StateProps> = ({
   appState: state,
   settings,
-  ui: { note, editMode },
+  ui: { note, editMode, selectedRevision },
 }) => ({
   allTags: state.tags,
   fontSize: settings.fontSize,
   editMode,
   isEditorActive: !state.showNavigation,
   note,
-  revision: state.revision,
+  revision: selectedRevision,
 });
 
 const mapDispatchToProps: S.MapDispatch<DispatchProps> = dispatch => ({

lib/note-toolbar-container.ts

@@ -18,7 +18,6 @@ type OwnProps = {
 type StateProps = {
   isViewingRevisions: boolean;
   notes: T.NoteEntity[];
-  revisionOrNote: T.NoteEntity | null;
 };
 
 type NoteChanger = {
@@ -31,7 +30,6 @@ type ListChanger = NoteChanger & { previousIndex: number };
 type DispatchProps = {
   closeNote: () => any;
   deleteNoteForever: (args: ListChanger) => any;
-  noteRevisions: (args: NoteChanger) => any;
   restoreNote: (args: ListChanger) => any;
   toggleRevisions: () => any;
   shareNote: () => any;
@@ -70,24 +68,17 @@ export class NoteToolbarContainer extends Component<Props> {
     analytics.tracks.recordEvent('editor_note_restored');
   };
 
-  onShowRevisions = (note: T.NoteEntity) => {
-    const { noteBucket } = this.props;
-    this.props.noteRevisions({ noteBucket, note });
-    analytics.tracks.recordEvent('editor_versions_accessed');
-  };
-
   onShareNote = () => {
     this.props.shareNote();
     analytics.tracks.recordEvent('editor_share_dialog_viewed');
   };
 
   render() {
-    const { isViewingRevisions, toolbar, revisionOrNote } = this.props;
+    const { isViewingRevisions, toolbar } = this.props;
 
     const handlers = {
       onDeleteNoteForever: this.onDeleteNoteForever,
       onRestoreNote: this.onRestoreNote,
-      onShowRevisions: this.onShowRevisions,
       onShareNote: this.onShareNote,
       onTrashNote: this.onTrashNote,
       toggleFocusMode: this.props.toggleFocusMode,
@@ -97,26 +88,19 @@ export class NoteToolbarContainer extends Component<Props> {
       return null;
     }
 
-    const markdownEnabled = revisionOrNote
-      ? revisionOrNote.data.systemTags.includes('markdown')
-      : false;
-
-    return cloneElement(toolbar, { ...handlers, markdownEnabled });
+    return cloneElement(toolbar, { ...handlers });
   }
 }
 
 const mapStateToProps: S.MapState<StateProps> = ({
-  appState,
-  ui: { filteredNotes, note, showRevisions },
+  ui: { filteredNotes, showRevisions },
 }) => ({
   isViewingRevisions: showRevisions,
   notes: filteredNotes,
-  revisionOrNote: appState.revision || note,
 });
 
 const {
   deleteNoteForever,
-  noteRevisions,
   restoreNote,
   showDialog,
   trashNote,
@@ -125,7 +109,6 @@ const {
 const mapDispatchToProps: S.MapDispatch<DispatchProps> = dispatch => ({
   closeNote: () => dispatch(closeNote()),
   deleteNoteForever: args => dispatch(deleteNoteForever(args)),
-  noteRevisions: args => dispatch(noteRevisions(args)),
   restoreNote: args => dispatch(restoreNote(args)),
   shareNote: () => dispatch(showDialog({ dialog: DialogTypes.SHARE })),
   toggleFocusMode: () => dispatch(toggleFocusMode()),

lib/note-toolbar/index.tsx

@@ -30,16 +30,13 @@ type DispatchProps = {
   toggleRevisions: () => any;
 };
 
-type OwnProps = {
-  onShowRevisions: (note: T.NoteEntity | null) => any;
-};
-
 type StateProps = {
   editMode: boolean;
+  markdownEnabled: boolean;
   note: T.NoteEntity | null;
 };
 
-type Props = DispatchProps & OwnProps & StateProps;
+type Props = DispatchProps & StateProps;
 
 export class NoteToolbar extends Component<Props> {
   static displayName = 'NoteToolbar';
@@ -50,7 +47,6 @@ export class NoteToolbar extends Component<Props> {
     onDeleteNoteForever: PropTypes.func,
     onShareNote: PropTypes.func,
     toggleFocusMode: PropTypes.func.isRequired,
-    markdownEnabled: PropTypes.bool,
   };
 
   static defaultProps = {
@@ -61,11 +57,6 @@ export class NoteToolbar extends Component<Props> {
     toggleFocusMode: noop,
   };
 
-  showRevisions = () => {
-    this.props.toggleRevisions();
-    this.props.onShowRevisions(this.props.note);
-  };
-
   render() {
     const { note } = this.props;
     const isTrashed = !!(note && note.data.deleted);
@@ -112,7 +103,7 @@ export class NoteToolbar extends Component<Props> {
           <div className="note-toolbar__button">
             <IconButton
               icon={<RevisionsIcon />}
-              onClick={this.showRevisions}
+              onClick={this.props.toggleRevisions}
               title="History"
             />
           </div>
@@ -180,11 +171,18 @@ export class NoteToolbar extends Component<Props> {
 }
 
 const mapStateToProps: S.MapState<StateProps> = ({
-  ui: { note, editMode },
-}) => ({
-  editMode,
-  note,
-});
+  ui: { note, editMode, selectedRevision },
+}) => {
+  const revisionOrNote = selectedRevision || note;
+
+  return {
+    editMode,
+    markdownEnabled: revisionOrNote
+      ? revisionOrNote.data.systemTags.includes('markdown')
+      : false,
+    note,
+  };
+};
 
 const mapDispatchToProps: S.MapDispatch<DispatchProps> = {
   closeNote,