From b61467097d350c1b777ff605283cf9a540809e63 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Fri, 11 Oct 2024 14:14:18 -0700 Subject: [PATCH] Mobile: Fixes #11134: Fix automatic resource download mode (#11144) --- .../components/screens/Note.test.tsx | 64 +++++++++++++++++-- .../app-mobile/components/screens/Note.tsx | 18 ++++-- packages/lib/testing/test-utils.ts | 1 + 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/packages/app-mobile/components/screens/Note.test.tsx b/packages/app-mobile/components/screens/Note.test.tsx index a903803c5..1de9266c0 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, runWithFakeTimers } from '@joplin/lib/testing/test-utils'; +import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, supportDir, synchronizerStart, resourceFetcher, runWithFakeTimers } from '@joplin/lib/testing/test-utils'; import { waitFor as waitForWithRealTimers } from '@joplin/lib/testing/test-utils'; import Note from '@joplin/lib/models/Note'; import { AppState } from '../../utils/types'; @@ -27,6 +27,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 { } @@ -68,11 +70,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 +84,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; @@ -132,6 +139,7 @@ const openEditor = async () => { describe('screens/Note', () => { beforeEach(async () => { + await setupDatabaseAndSynchronizer(1); await setupDatabaseAndSynchronizer(0); await switchClient(0); @@ -279,4 +287,48 @@ describe('screens/Note', () => { cleanup(); }); + + it.each([ + 'auto', + 'manual', + ])('should correctly auto-download or not auto-download resources in %j mode', async (downloadMode) => { + 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}.`); + } + }); + }); }); diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index 04800651e..a5f4253d8 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 da0bdef06..3c7d630aa 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);