From 9c14685411a2781dbd508b0c24e251f315d26a38 Mon Sep 17 00:00:00 2001
From: Dennis Snell <dennis.snell@automattic.com>
Date: Wed, 5 Feb 2020 17:32:36 -0700
Subject: [PATCH] Fix: Update selected note when making changes in the editor
 view (#1880)

* Fix: Update selected note when making changes in the editor view

Resolves #1879

It appears that we removed a flow in #1851 which updated app state in response
to updates from the note editor[1]. While this didn't impact the operation of
the editor it did present itself as a bug when previewing markdown-rendered
notes. The editor is stateful on its own and so continued to function
independently of the state of the selected note's content. The preview,
however, grabs the contents of the selected note whenever it's activated and
then renders those into a DOM node. This means that it showed the old contents
of the note unless you navigated to another note and back, which swapped out
the selected note with the new updated contents.

In this patch, which currently is probably wrong in some way, we're updating
the selected note when calling `onChangeContent` so that we don't get out of
sync with the data sources. Hopefully at the end of our state cleanup and
stabilization project this will disappear but for now this fix catches the
gap that left split data sources.

Probably there were other bugs we didn't discover as a result of this.

Mainly we need to test and audit for traces of cycles/loops in the update
so that we don't start spinning or overwriting data we just entered. In my
brief testing the easy cases worked as I expected them to simply and singly.

[1]: https://github.com/Automattic/simplenote-electron/pull/1851/files#diff-3c87750a7cc32e0127447ffc78f5c0d5L300

* Handle tag updates too by always re-selecting note on updates from the server

When tags were being updated in the pap they also came through this function
but they weren't coming through `onUpdateContent`, meaning that our fix
overlooked tags and probably also system tags and the like.

In this patch we're no longer selectively updating the note if it has a remote
patch (indicating that it came from another device). Instead, we're always
updating and selectively setting `hasRemoteUpdate` based on that knowledge.
This ensures that we maintain consistency with local updates.
---
 lib/app.tsx               | 21 +++++++++++++++++----
 lib/note-detail/index.tsx |  3 ++-
 lib/note-editor/index.tsx |  1 -
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/app.tsx b/lib/app.tsx
index 6c79c2752..9373c2549 100644
--- a/lib/app.tsx
+++ b/lib/app.tsx
@@ -100,6 +100,7 @@ const mapDispatchToProps: S.MapDispatch<
 
     openTagList: () => dispatch(actionCreators.toggleNavigation()),
     resetAuth: () => dispatch(reduxActions.auth.reset()),
+    selectNote: (note: T.NoteEntity) => dispatch(actions.ui.selectNote(note)),
     setAuthorized: () => dispatch(reduxActions.auth.setAuthorized()),
     setSearchFocus: () =>
       dispatch(actionCreators.setSearchFocus({ searchFocus: true })),
@@ -316,15 +317,25 @@ export const App = connect(
 
     onNoteRemoved = () => this.onNotesIndex();
 
-    onNoteUpdate = (noteId, data, remoteUpdateInfo = {}) => {
+    onNoteUpdate = (
+      noteId: T.EntityId,
+      data,
+      remoteUpdateInfo: { patch?: object } = {}
+    ) => {
       const {
         noteBucket,
         selectNote,
         ui: { note },
       } = this.props;
-      if (remoteUpdateInfo.patch && note && noteId === note.id) {
-        noteBucket.get(noteId, (e, updatedNote) => {
-          selectNote({ ...updatedNote, hasRemoteUpdate: true });
+      if (note && noteId === note.id) {
+        noteBucket.get(noteId, (e: unknown, storedNote: T.NoteEntity) => {
+          if (e) {
+            return;
+          }
+          const updatedNote = remoteUpdateInfo.patch
+            ? { ...storedNote, hasRemoteUpdate: true }
+            : storedNote;
+          selectNote(updatedNote);
         });
       }
     };
@@ -368,6 +379,8 @@ export const App = connect(
         },
       };
 
+      this.props.selectNote(updatedNote);
+
       const { noteBucket } = this.props;
       noteBucket.update(note.id, updatedNote.data, {}, { sync });
       if (sync) {
diff --git a/lib/note-detail/index.tsx b/lib/note-detail/index.tsx
index 1502ac4f6..62636918f 100644
--- a/lib/note-detail/index.tsx
+++ b/lib/note-detail/index.tsx
@@ -240,9 +240,10 @@ export class NoteDetail extends Component {
   }
 }
 
-const mapStateToProps = ({ appState: state, settings }) => ({
+const mapStateToProps = ({ appState: state, ui, settings }) => ({
   dialogs: state.dialogs,
   filter: state.filter,
+  note: state.revision || ui.note,
   shouldPrint: state.shouldPrint,
   showNoteInfo: state.showNoteInfo,
   spellCheckEnabled: settings.spellCheckEnabled,
diff --git a/lib/note-editor/index.tsx b/lib/note-editor/index.tsx
index 509e9de3c..6985f17db 100644
--- a/lib/note-editor/index.tsx
+++ b/lib/note-editor/index.tsx
@@ -146,7 +146,6 @@ export class NoteEditor extends Component<Props> {
           storeFocusEditor={this.storeFocusEditor}
           storeHasFocus={this.storeEditorHasFocus}
           filter={this.props.filter}
-          note={revision}
           noteBucket={noteBucket}
           previewingMarkdown={
             this.markdownEnabled() && editorMode === 'markdown'