Desktop: Fixes #5955: Changing the currently opened note from plugins or the data API does not refresh the note content

packages/app-cli/tests/support/plugins/simple/index.js

@@ -1,6 +1,21 @@
 joplin.plugins.register({
 	onStart: async function() {
-		const folder = await joplin.data.post(['folders'], null, { title: "my plugin folder" });
+		// const folder = await joplin.data.post(['folders'], null, { title: "my plugin folder" });
-		await joplin.data.post(['notes'], null, { parent_id: folder.id, title: "testing plugin!" });
+		// await joplin.data.post(['notes'], null, { parent_id: folder.id, title: "testing plugin!" });
+
+		await joplin.commands.register({
+			name: 'updateCurrentNote',
+			label: 'Update current note via the data API',
+			iconName: 'fas fa-music',
+			execute: async () => {
+				const noteIds = await joplin.workspace.selectedNoteIds();
+				const noteId = noteIds.length === 1 ? noteIds[0] : null;
+				if (!noteId) return;
+				console.info('Modifying current note...');
+				await joplin.data.put(['notes', noteId], null, { body: "New note body " + Date.now() });
+			},
+		});
+
+		await joplin.views.toolbarButtons.create('updateCurrentNoteButton', 'updateCurrentNote', 'editorToolbar');		
 	},
 });

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

@@ -76,11 +76,18 @@ function NoteEditor(props: NoteEditorProps) {
 	}, []);
 
 	const effectiveNoteId = useEffectiveNoteId(props);
+	const effectiveNote = props.notes.find(n => n.id === effectiveNoteId);
 
 	const { formNote, setFormNote, isNewNote, resourceInfos } = useFormNote({
 		syncStarted: props.syncStarted,
 		decryptionStarted: props.decryptionStarted,
 		noteId: effectiveNoteId,
+
+		// The effective updated_time property of the note. It may be different
+		// from the last time the note was saved, if it was modified outside the
+		// editor (eg. via API).
+		dbNote: effectiveNote ? { id: effectiveNote.id, updated_time: effectiveNote.updated_time } : { id: '', updated_time: 0 },
+
 		isProvisional: props.isProvisional,
 		titleInputRef: titleInputRef,
 		editorRef: editorRef,
@@ -120,12 +127,32 @@ function NoteEditor(props: NoteEditorProps) {
 			return async function() {
 				const note = await formNoteToNote(formNote);
 				reg.logger().debug('Saving note...', note);
-				const savedNote: any = await Note.save(note);
+				const noteUpdatedTime = Date.now();
+
+				// First we set the formNote object, then we save the note. We
+				// do it in that order, otherwise `useFormNote` will be rendered
+				// with the newly saved note and the timestamp of that note will
+				// be more recent that the one in the editor, which will trigger
+				// an update. We do not want this since we already have the
+				// latest changes.
+				//
+				// It also means that we manually set the timestamp, so that we
+				// have it before the note is saved.
 
 				setFormNote((prev: FormNote) => {
-					return { ...prev, user_updated_time: savedNote.user_updated_time, hasChanged: false };
+					return {
+						...prev,
+						user_updated_time: noteUpdatedTime,
+						updated_time: noteUpdatedTime,
+						hasChanged: false,
+					};
 				});
 
+				const savedNote = await Note.save({
+					...note,
+					updated_time: noteUpdatedTime,
+				}, { autoTimestamp: false });
+
 				void ExternalEditWatcher.instance().updateNoteFile(savedNote);
 
 				props.dispatch({

packages/app-desktop/gui/NoteEditor/utils/types.ts

@@ -5,6 +5,7 @@ import { MarkupLanguage } from '@joplin/renderer';
 import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/MarkupToHtml';
 import { MarkupToHtmlOptions } from './useMarkupToHtml';
 import { Dispatch } from 'redux';
+import { NoteEntity } from '@joplin/lib/services/database/types';
 
 export interface AllAssetsOptions {
 	contentMaxWidthTarget?: string;
@@ -20,7 +21,7 @@ export interface NoteEditorProps {
 	dispatch: Dispatch;
 	selectedNoteIds: string[];
 	selectedFolderId: string;
-	notes: any[];
+	notes: NoteEntity[];
 	watchedNoteFiles: string[];
 	isProvisional: boolean;
 	editorNoteStatuses: any;
@@ -104,6 +105,7 @@ export interface FormNote {
 	markup_language: number;
 	user_updated_time: number;
 	encryption_applied: number;
+	updated_time: number;
 
 	hasChanged: boolean;
 
@@ -154,6 +156,7 @@ export function defaultFormNote(): FormNote {
 		hasChanged: false,
 		user_updated_time: 0,
 		encryption_applied: 0,
+		updated_time: 0,
 	};
 }
 

packages/app-desktop/gui/NoteEditor/utils/useFormNote.test.ts

@@ -1,8 +1,19 @@
 import Note from '@joplin/lib/models/Note';
 import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
 import { renderHook } from '@testing-library/react-hooks';
-import useFormNote, { HookDependencies } from './useFormNote';
+import useFormNote, { DbNote, HookDependencies } from './useFormNote';
 
+const defaultFormNoteProps: HookDependencies = {
+	syncStarted: false,
+	decryptionStarted: false,
+	noteId: '',
+	isProvisional: false,
+	titleInputRef: null,
+	editorRef: null,
+	onBeforeLoad: ()=>{},
+	onAfterLoad: ()=>{},
+	dbNote: { id: '', updated_time: 0 },
+};
 
 describe('useFormNote', () => {
 	beforeEach(async () => {
@@ -15,14 +26,10 @@ describe('useFormNote', () => {
 
 		const makeFormNoteProps = (syncStarted: boolean, decryptionStarted: boolean): HookDependencies => {
 			return {
+				...defaultFormNoteProps,
 				syncStarted,
 				decryptionStarted,
 				noteId: testNote.id,
-				isProvisional: false,
-				titleInputRef: null,
-				editorRef: null,
-				onBeforeLoad: ()=>{},
-				onAfterLoad: ()=>{},
 			};
 		};
 
@@ -70,4 +77,35 @@ describe('useFormNote', () => {
 			});
 		});
 	});
+
+	it('should reload the note when it is changed outside of the editor', async () => {
+		const note = await Note.save({ title: 'Test Note!' });
+
+		const makeFormNoteProps = (dbNote: DbNote): HookDependencies => {
+			return {
+				...defaultFormNoteProps,
+				noteId: note.id,
+				dbNote,
+			};
+		};
+
+		const formNote = renderHook(props => useFormNote(props), {
+			initialProps: makeFormNoteProps({ id: note.id, updated_time: note.updated_time }),
+		});
+
+		await formNote.waitFor(() => {
+			expect(formNote.result.current.formNote.title).toBe('Test Note!');
+		});
+
+		// Simulate the note being modified outside the editor
+		const modifiedNote = await Note.save({ id: note.id, title: 'Modified' });
+
+		// NoteEditor then would update `dbNote`
+		formNote.rerender(makeFormNoteProps({ id: note.id, updated_time: modifiedNote.updated_time }));
+
+		await formNote.waitFor(() => {
+			expect(formNote.result.current.formNote.title).toBe('Modified');
+		});
+	});
+
 });

packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts

@@ -7,21 +7,30 @@ import { splitHtml } from '@joplin/renderer/HtmlToHtml';
 import Setting from '@joplin/lib/models/Setting';
 import usePrevious from '../../hooks/usePrevious';
 import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher/index';
-
+import { MarkupToHtml } from '@joplin/renderer';
-const { MarkupToHtml } = require('@joplin/renderer');
 import Note from '@joplin/lib/models/Note';
-import { reg } from '@joplin/lib/registry';
 import ResourceFetcher from '@joplin/lib/services/ResourceFetcher';
 import DecryptionWorker from '@joplin/lib/services/DecryptionWorker';
+import { NoteEntity } from '@joplin/lib/services/database/types';
+import Logger from '@joplin/utils/Logger';
+
+const logger = Logger.create('useFormNote');
 
 export interface OnLoadEvent {
 	formNote: FormNote;
+	updated_time: number;
+}
+
+export interface DbNote {
+	id: string;
+	updated_time: number;
 }
 
 export interface HookDependencies {
 	syncStarted: boolean;
 	decryptionStarted: boolean;
 	noteId: string;
+	dbNote: DbNote;
 	isProvisional: boolean;
 	titleInputRef: any;
 	editorRef: any;
@@ -63,7 +72,7 @@ function resourceInfosChanged(a: ResourceInfos, b: ResourceInfos): boolean {
 
 export default function useFormNote(dependencies: HookDependencies) {
 	const {
-		syncStarted, decryptionStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad,
+		syncStarted, decryptionStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad, dbNote,
 	} = dependencies;
 
 	const [formNote, setFormNote] = useState<FormNote>(defaultFormNote());
@@ -77,7 +86,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 	// a new refresh.
 	const [formNoteRefeshScheduled, setFormNoteRefreshScheduled] = useState<number>(0);
 
-	async function initNoteState(n: any) {
+	async function initNoteState(n: NoteEntity) {
 		let originalCss = '';
 
 		if (n.markup_language === MarkupToHtml.MARKUP_LANGUAGE_HTML) {
@@ -85,7 +94,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 			originalCss = splitted.css;
 		}
 
-		const newFormNote = {
+		const newFormNote: FormNote = {
 			id: n.id,
 			title: n.title,
 			body: n.body,
@@ -99,6 +108,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 			hasChanged: false,
 			user_updated_time: n.user_updated_time,
 			encryption_applied: n.encryption_applied,
+			updated_time: n.updated_time,
 		};
 
 		// Note that for performance reason,the call to setResourceInfos should
@@ -116,7 +126,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 	useEffect(() => {
 		if (formNoteRefeshScheduled <= 0) return () => {};
 
-		reg.logger().info('Sync has finished and note has never been changed - reloading it');
+		logger.info('Sync has finished and note has never been changed - reloading it');
 
 		let cancelled = false;
 
@@ -128,7 +138,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 			// it would not have been loaded in the editor (due to note selection changing
 			// on delete)
 			if (!n) {
-				reg.logger().warn('Trying to reload note that has been deleted:', noteId);
+				logger.warn('Trying to reload note that has been deleted:', noteId);
 				return;
 			}
 
@@ -150,19 +160,19 @@ export default function useFormNote(dependencies: HookDependencies) {
 	}, [formNoteRefeshScheduled]);
 
 	useEffect(() => {
-		// Check that synchronisation has just finished - and
+		// Check that synchronisation has just finished - and if the note has
-		// if the note has never been changed, we reload it.
+		// never been changed, we reload it. If the note has already been
-		// If the note has already been changed, it's a conflict
+		// changed, it's a conflict that's already been handled by the
-		// that's already been handled by the synchronizer.
+		// synchronizer.
 		const decryptionJustEnded = prevDecryptionStarted && !decryptionStarted;
 		const syncJustEnded = prevSyncStarted && !syncStarted;
 
 		if (!decryptionJustEnded && !syncJustEnded) return;
 		if (formNote.hasChanged) return;
 
-		// Refresh the form note.
+		// Refresh the form note. This is kept separate from the above logic so
-		// This is kept separate from the above logic so that when prevSyncStarted is changed
+		// that when prevSyncStarted is changed from true to false, it doesn't
-		// from true to false, it doesn't cancel the note from loading.
+		// cancel the note from loading.
 		refreshFormNote();
 	}, [
 		prevSyncStarted, syncStarted,
@@ -170,6 +180,18 @@ export default function useFormNote(dependencies: HookDependencies) {
 		formNote.hasChanged, refreshFormNote,
 	]);
 
+	useEffect(() => {
+		// Something's not fully initialised - we skip the check
+		if (!dbNote.id || !dbNote.updated_time || !formNote.updated_time) return;
+
+		// If the note in the database is more recent that the note in editor,
+		// it was modified outside the editor, so we refresh it.
+		if (dbNote.id === formNote.id && dbNote.updated_time > formNote.updated_time) {
+			logger.info('Note has been changed outside the editor - reloading it');
+			refreshFormNote();
+		}
+	}, [dbNote.id, dbNote.updated_time, formNote.updated_time, formNote.id, refreshFormNote]);
+
 	useEffect(() => {
 		if (!noteId) {
 			if (formNote.id) setFormNote(defaultFormNote());
@@ -180,7 +202,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 
 		let cancelled = false;
 
-		reg.logger().debug('Loading existing note', noteId);
+		logger.debug('Loading existing note', noteId);
 
 		function handleAutoFocus(noteIsTodo: boolean) {
 			if (!isProvisional) return;
@@ -200,15 +222,15 @@ export default function useFormNote(dependencies: HookDependencies) {
 			const n = await Note.load(noteId);
 			if (cancelled) return;
 			if (!n) throw new Error(`Cannot find note with ID: ${noteId}`);
-			reg.logger().debug('Loaded note:', n);
+			logger.debug('Loaded note:', n);
 
-			await onBeforeLoad({ formNote });
+			await onBeforeLoad({ formNote, updated_time: 0 });
 
 			const newFormNote = await initNoteState(n);
 
 			setIsNewNote(isProvisional);
 
-			await onAfterLoad({ formNote: newFormNote });
+			await onAfterLoad({ formNote: newFormNote, updated_time: n.updated_time });
 
 			handleAutoFocus(!!n.is_todo);
 		}

packages/app-desktop/testPluginDemo.sh

@@ -5,17 +5,22 @@
 
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 TEMP_PATH=~/src/plugin-tests
-PLUGIN_PATH=~/src/joplin/packages/app-cli/tests/support/plugins/note_list_renderer
+NEED_COMPILING=0
+PLUGIN_PATH=~/src/joplin/packages/app-cli/tests/support/plugins/simple
 
-mkdir -p "$TEMP_PATH"
+if [[ $NEED_COMPILING == 1 ]]; then
-PLUGIN_NAME=$(echo "$PLUGIN_PATH" | awk -F/ '{print $NF}')
+	mkdir -p "$TEMP_PATH"
-TEMP_PLUGIN_PATH="$TEMP_PATH/$PLUGIN_NAME" 
+	PLUGIN_NAME=$(echo "$PLUGIN_PATH" | awk -F/ '{print $NF}')
+	TEMP_PLUGIN_PATH="$TEMP_PATH/$PLUGIN_NAME" 
 
-echo "Copying from: $PLUGIN_PATH"
+	echo "Copying from: $PLUGIN_PATH"
-echo "To: $TEMP_PLUGIN_PATH"
+	echo "To: $TEMP_PLUGIN_PATH"
 
-rsync -a --delete "$PLUGIN_PATH/" "$TEMP_PLUGIN_PATH/" 
+	rsync -a --delete "$PLUGIN_PATH/" "$TEMP_PLUGIN_PATH/" 
 
-npm install --prefix="$TEMP_PLUGIN_PATH" && yarn start --dev-plugins "$TEMP_PLUGIN_PATH"
+	npm install --prefix="$TEMP_PLUGIN_PATH" && yarn start --dev-plugins "$TEMP_PLUGIN_PATH"
+else
+	yarn start --dev-plugins "$PLUGIN_PATH"
+fi
 
 # Add eg "--profile $HOME/.config/joplindev-desktop-1" to test with a different profile