Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
64 changes: 58 additions & 6 deletions packages/app-mobile/components/screens/Note.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
}
Expand Down Expand Up @@ -68,11 +70,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({
Expand All @@ -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;
Expand Down Expand Up @@ -132,6 +139,7 @@ const openEditor = async () => {

describe('screens/Note', () => {
beforeEach(async () => {
await setupDatabaseAndSynchronizer(1);
await setupDatabaseAndSynchronizer(0);
await switchClient(0);

Expand Down Expand Up @@ -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(<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}.`);
}
});
});
});
18 changes: 11 additions & 7 deletions packages/app-mobile/components/screens/Note.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/lib/testing/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading