Mobile: Fixes laurent22#11134: Fix automatic resource download mode

packages/app-mobile/components/screens/Note.test.tsx

@@ -7,7 +7,7 @@ import { Provider } from 'react-redux';
 
 import NoteScreen from './Note';
 import { MenuProvider } from 'react-native-popup-menu';
-import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, waitFor } from '@joplin/lib/testing/test-utils';
+import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, waitFor, supportDir, synchronizerStart, resourceFetcher, runWithFakeTimers } from '@joplin/lib/testing/test-utils';
 import Note from '@joplin/lib/models/Note';
 import { AppState } from '../../utils/types';
 import { Store } from 'redux';
@@ -26,6 +26,8 @@ import { LayoutChangeEvent } from 'react-native';
 import shim from '@joplin/lib/shim';
 import getWebViewWindowById from '../../utils/testing/getWebViewWindowById';
 import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
+import Setting from '@joplin/lib/models/Setting';
+import Resource from '@joplin/lib/models/Resource';
 
 interface WrapperProps {
 }
@@ -67,12 +69,8 @@ const waitForNoteToMatch = async (noteId: string, note: Partial<NoteEntity>) =>
 	}));
 };
 
-const openNewNote = async (noteProperties: NoteEntity) => {
-	const note = await Note.save({
-		parent_id: (await Folder.defaultFolder()).id,
-		...noteProperties,
-	});
-
+const openExistingNote = async (noteId: string) => {
+	const note = await Note.load(noteId);
 	const displayParentId = getDisplayParentId(note, await Folder.load(note.parent_id));
 
 	store.dispatch({
@@ -85,7 +83,15 @@ const openNewNote = async (noteProperties: NoteEntity) => {
 		id: note.id,
 		folderId: displayParentId,
 	});
+};
+
+const openNewNote = async (noteProperties: NoteEntity) => {
+	const note = await Note.save({
+		parent_id: (await Folder.defaultFolder()).id,
+		...noteProperties,
+	});
 
+	await openExistingNote(note.id);
 	await waitForNoteToMatch(note.id, { parent_id: note.parent_id, title: note.title, body: note.body });
 
 	return note.id;
@@ -106,21 +112,20 @@ const openNoteActionsMenu = async () => {
 		cursor = cursor.parent;
 	}
 
-	await userEvent.press(actionMenuButton);
+	await runWithFakeTimers(() => userEvent.press(actionMenuButton));
 };
 
 const openEditor = async () => {
 	const editButton = await screen.findByLabelText('Edit');
-	await userEvent.press(editButton);
+	fireEvent.press(editButton);
 };
 
 describe('screens/Note', () => {
-	beforeAll(() => {
+	beforeEach(async () => {
 		// advanceTimers: Needed by internal note save logic
 		jest.useFakeTimers({ advanceTimers: true });
-	});
 
-	beforeEach(async () => {
+		await setupDatabaseAndSynchronizer(1);
 		await setupDatabaseAndSynchronizer(0);
 		await switchClient(0);
 
@@ -265,4 +270,53 @@ describe('screens/Note', () => {
 
 		cleanup();
 	});
+
+	it.each([
+		'auto',
+		'manual',
+	])('should correctly auto-download or not auto-download resources in %j mode', async (downloadMode) => {
+		// This test interacts with the synchronizer, which seems to need real timers.
+		jest.useRealTimers();
+
+		let note = await Note.save({ title: 'Note 1', parent_id: (await Folder.defaultFolder()).id });
+		note = await shim.attachFileToNote(note, `${supportDir}/photo.jpg`);
+
+		await synchronizerStart();
+		await switchClient(1);
+		Setting.setValue('sync.resourceDownloadMode', downloadMode);
+		await synchronizerStart();
+
+		// Before opening the note, the resource should not be marked for download
+		const allResources = await Resource.all();
+		expect(allResources.length).toBe(1);
+		const resource = allResources[0];
+		expect(await Resource.localState(resource)).toMatchObject({ fetch_status: Resource.FETCH_STATUS_IDLE });
+
+		await openExistingNote(note.id);
+
+		render(<WrappedNoteScreen />);
+
+		// Note should render
+		const titleInput = await screen.findByDisplayValue('Note 1');
+		expect(titleInput).toBeVisible();
+
+		// Wrap in act() -- the component may update in the background during this.
+		await act(async () => {
+			await resourceFetcher().waitForAllFinished();
+
+			// After opening the note, the resource should be marked for download only in automatic mode
+			if (downloadMode === 'auto') {
+				await waitFor(async () => {
+					expect(await Resource.localState(resource.id)).toMatchObject({ fetch_status: Resource.FETCH_STATUS_DONE });
+				});
+			} else if (downloadMode === 'manual') {
+				// In manual mode, should not mark for download
+				expect(await Resource.localState(resource)).toMatchObject({ fetch_status: Resource.FETCH_STATUS_IDLE });
+			} else {
+				throw new Error(`Should not be testing downloadMode: ${downloadMode}.`);
+			}
+		});
+
+		screen.unmount();
+	});
 });

packages/app-mobile/components/screens/Note.tsx

@@ -493,11 +493,6 @@ class NoteScreenComponent extends BaseScreenComponent<Props, State> implements B
 		this.undoRedoService_ = new UndoRedoService();
 		this.undoRedoService_.on('stackChange', this.undoRedoService_stackChange);
 
-		if (this.state.note && this.state.note.body && Setting.value('sync.resourceDownloadMode') === 'auto') {
-			const resourceIds = await Note.linkedResourceIds(this.state.note.body);
-			await ResourceFetcher.instance().markForDownload(resourceIds);
-		}
-
 		// Although it is async, we don't wait for the answer so that if permission
 		// has already been granted, it doesn't slow down opening the note. If it hasn't
 		// been granted, the popup will open anyway.
@@ -509,8 +504,12 @@ class NoteScreenComponent extends BaseScreenComponent<Props, State> implements B
 		void ResourceFetcher.instance().markForDownload(event.resourceId);
 	}
 
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-	public componentDidUpdate(prevProps: any, prevState: any) {
+	public async markAllAttachedResourcesForDownload() {
+		const resourceIds = await Note.linkedResourceIds(this.state.note.body);
+		await ResourceFetcher.instance().markForDownload(resourceIds);
+	}
+
+	public componentDidUpdate(prevProps: Props, prevState: State) {
 		if (this.doFocusUpdate_) {
 			this.doFocusUpdate_ = false;
 			this.scheduleFocusUpdate();
@@ -528,6 +527,11 @@ class NoteScreenComponent extends BaseScreenComponent<Props, State> implements B
 			void promptRestoreAutosave((drawingData: string) => {
 				void this.attachNewDrawing(drawingData);
 			});
+
+			// Handle automatic resource downloading
+			if (this.state.note.body && Setting.value('sync.resourceDownloadMode') === 'auto') {
+				void this.markAllAttachedResourcesForDownload();
+			}
 		}
 
 		// Disable opening/closing the side menu with touch gestures

packages/lib/testing/test-utils.ts

@@ -286,6 +286,7 @@ async function switchClient(id: number, options: any = null) {
 	BaseItem.encryptionService_ = encryptionServices_[id];
 	Resource.encryptionService_ = encryptionServices_[id];
 	BaseItem.revisionService_ = revisionServices_[id];
+	ResourceFetcher.instance_ = resourceFetchers_[id];
 
 	await Setting.reset();
 	Setting.settingFilename = settingFilename(id);
@@ -1125,7 +1126,8 @@ export const runWithFakeTimers = async (callback: ()=> Promise<void>) => {
 		throw new Error('Fake timers are only supported in jest.');
 	}
 
-	jest.useFakeTimers();
+	// advanceTimers: true is needed for certain Joplin APIs to work.
+	jest.useFakeTimers({ advanceTimers: true });
 
 	// The shim.setTimeout and similar functions need to be changed to
 	// use fake timers.