From 1e27fe4c17a8794462f1c2ac0a98f743387fa244 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 27 Sep 2024 16:29:25 -0700 Subject: [PATCH] Mobile: Fixes #11134: Fix automatic resource download mode --- .../components/screens/Note.test.tsx | 78 ++++++++++++++++--- .../app-mobile/components/screens/Note.tsx | 18 +++-- packages/lib/testing/test-utils.ts | 4 +- 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/packages/app-mobile/components/screens/Note.test.tsx b/packages/app-mobile/components/screens/Note.test.tsx index 1997a945951..5edabc9eba7 100644 --- a/packages/app-mobile/components/screens/Note.test.tsx +++ b/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) => })); }; -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(); + + // 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(); + }); }); diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index 04800651e60..7dc1079275a 100644 --- a/packages/app-mobile/components/screens/Note.tsx +++ b/packages/app-mobile/components/screens/Note.tsx @@ -493,11 +493,6 @@ class NoteScreenComponent extends BaseScreenComponent 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 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 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 diff --git a/packages/lib/testing/test-utils.ts b/packages/lib/testing/test-utils.ts index cc5f558ce5d..df2cbaca18b 100644 --- a/packages/lib/testing/test-utils.ts +++ b/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) => { 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.