From bb66e81abe9049a7e9c086485c5a3b53fe0efc3f Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Sat, 16 Nov 2024 13:07:34 -0800 Subject: [PATCH 1/2] Mobile: Fixes #11384: Fix switching notes then unloading app causes blank screen (#11396) --- .../components/ScreenHeader/index.tsx | 11 +++++++---- .../app-mobile/components/screens/Note.tsx | 19 +++++++++++++++---- .../app-mobile/components/screens/Notes.tsx | 4 ++-- .../components/screens/SearchScreen/index.tsx | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/app-mobile/components/ScreenHeader/index.tsx b/packages/app-mobile/components/ScreenHeader/index.tsx index 0afebcaaa..7fe3bccc9 100644 --- a/packages/app-mobile/components/ScreenHeader/index.tsx +++ b/packages/app-mobile/components/ScreenHeader/index.tsx @@ -37,7 +37,8 @@ const PADDING_V = 10; type OnPressCallback=()=> void; export interface FolderPickerOptions { - enabled: boolean; + visible: boolean; + disabled?: boolean; selectedFolderId?: string; onValueChange?: OnValueChangedListener; mustSelect?: boolean; @@ -517,10 +518,12 @@ class ScreenHeaderComponent extends PureComponent { + const createTitleComponent = (hideableAfterTitleComponents: ReactElement) => { const folderPickerOptions = this.props.folderPickerOptions; - if (folderPickerOptions && folderPickerOptions.enabled) { + if (folderPickerOptions && folderPickerOptions.visible) { + const hasSelectedNotes = this.props.selectedNoteIds.length > 0; + const disabled = this.props.folderPickerOptions.disabled ?? !hasSelectedNotes; return ( ; - const titleComp = createTitleComponent(headerItemDisabled, hideableRightComponents); + const titleComp = createTitleComponent(hideableRightComponents); const contextMenuStyle: ViewStyle = { paddingTop: PADDING_V, diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index a10bc2e0a..f12aefebf 100644 --- a/packages/app-mobile/components/screens/Note.tsx +++ b/packages/app-mobile/components/screens/Note.tsx @@ -50,7 +50,7 @@ import { isSupportedLanguage } from '../../services/voiceTyping/vosk'; import { ChangeEvent as EditorChangeEvent, SelectionRangeChangeEvent, UndoRedoDepthChangeEvent } from '@joplin/editor/events'; import { join } from 'path'; import { Dispatch } from 'redux'; -import { RefObject } from 'react'; +import { RefObject, useRef } from 'react'; import { SelectionRange } from '../NoteEditor/types'; import { getNoteCallbackUrl } from '@joplin/lib/callbackUrlUtils'; import { AppState } from '../../utils/types'; @@ -1372,7 +1372,8 @@ class NoteScreenComponent extends BaseScreenComponent implements B public folderPickerOptions() { const options = { - enabled: !this.state.readOnly, + visible: !this.state.readOnly, + disabled: false, selectedFolderId: this.state.folder ? this.state.folder.id : null, onValueChange: this.folderPickerOptions_valueChanged, }; @@ -1380,7 +1381,7 @@ class NoteScreenComponent extends BaseScreenComponent implements B if ( this.folderPickerOptions_ && options.selectedFolderId === this.folderPickerOptions_.selectedFolderId - && options.enabled === this.folderPickerOptions_.enabled + && options.visible === this.folderPickerOptions_.visible ) { return this.folderPickerOptions_; } @@ -1639,8 +1640,18 @@ class NoteScreenComponent extends BaseScreenComponent implements B // which can cause some bugs where previously set state to another note would interfere // how the new note should be rendered const NoteScreenWrapper = (props: Props) => { + const lastNonNullNoteIdRef = useRef(props.noteId); + if (props.noteId) { + lastNonNullNoteIdRef.current = props.noteId; + } + + // This keeps the current note open even if it's no longer present in selectedNoteIds. + // This might happen, for example, if the selected note is moved to an unselected + // folder. + const noteId = lastNonNullNoteIdRef.current; + return ( - + ); }; diff --git a/packages/app-mobile/components/screens/Notes.tsx b/packages/app-mobile/components/screens/Notes.tsx index a3550c160..d4d01e58b 100644 --- a/packages/app-mobile/components/screens/Notes.tsx +++ b/packages/app-mobile/components/screens/Notes.tsx @@ -222,11 +222,11 @@ class NotesScreenComponent extends BaseScreenComponent { public folderPickerOptions() { const options = { - enabled: this.props.noteSelectionEnabled, + visible: this.props.noteSelectionEnabled, mustSelect: true, }; - if (this.folderPickerOptions_ && options.enabled === this.folderPickerOptions_.enabled) return this.folderPickerOptions_; + if (this.folderPickerOptions_ && options.visible === this.folderPickerOptions_.visible) return this.folderPickerOptions_; this.folderPickerOptions_ = options; return this.folderPickerOptions_; diff --git a/packages/app-mobile/components/screens/SearchScreen/index.tsx b/packages/app-mobile/components/screens/SearchScreen/index.tsx index 4e365585d..6166499b7 100644 --- a/packages/app-mobile/components/screens/SearchScreen/index.tsx +++ b/packages/app-mobile/components/screens/SearchScreen/index.tsx @@ -85,7 +85,7 @@ const SearchScreenComponent: React.FC = props => { Date: Mon, 9 Dec 2024 07:52:45 -0800 Subject: [PATCH 2/2] Desktop: Fixes #11457: Fix crash on startup if old read-only items are in the trash (#11458) --- .../trash/permanentlyDeleteOldItems.test.ts | 40 ++++++++++++++++- .../trash/permanentlyDeleteOldItems.ts | 43 +++++++++++++++++-- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/lib/services/trash/permanentlyDeleteOldItems.test.ts b/packages/lib/services/trash/permanentlyDeleteOldItems.test.ts index a0643c22b..5b92cd6fd 100644 --- a/packages/lib/services/trash/permanentlyDeleteOldItems.test.ts +++ b/packages/lib/services/trash/permanentlyDeleteOldItems.test.ts @@ -1,7 +1,7 @@ import { Day, msleep } from '@joplin/utils/time'; import Folder from '../../models/Folder'; import Note from '../../models/Note'; -import { setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils'; +import { setupDatabaseAndSynchronizer, simulateReadOnlyShareEnv, switchClient } from '../../testing/test-utils'; import permanentlyDeleteOldItems from './permanentlyDeleteOldItems'; import Setting from '../../models/Setting'; @@ -75,4 +75,42 @@ describe('permanentlyDeleteOldItems', () => { expect(await Folder.count()).toBe(1); }); + it('should not auto-delete read-only items', async () => { + const shareId = 'testShare'; + + // Simulates a folder having been deleted a long time ago + const longTimeAgo = 1000; + + const readOnlyFolder = await Folder.save({ + title: 'Read-only folder', + share_id: shareId, + deleted_time: longTimeAgo, + }); + const readOnlyNote1 = await Note.save({ + title: 'Read-only note', + parent_id: readOnlyFolder.id, + share_id: shareId, + deleted_time: longTimeAgo, + }); + const readOnlyNote2 = await Note.save({ + title: 'Read-only note 2', + share_id: shareId, + deleted_time: longTimeAgo, + }); + const writableNote = await Note.save({ + title: 'Editable note', + deleted_time: longTimeAgo, + }); + + const cleanup = simulateReadOnlyShareEnv(shareId); + await permanentlyDeleteOldItems(Day); + + // Should preserve only the read-only items. + expect(await Folder.load(readOnlyFolder.id)).toBeTruthy(); + expect(await Note.load(readOnlyNote1.id)).toBeTruthy(); + expect(await Note.load(readOnlyNote2.id)).toBeTruthy(); + expect(await Note.load(writableNote.id)).toBeFalsy(); + + cleanup(); + }); }); diff --git a/packages/lib/services/trash/permanentlyDeleteOldItems.ts b/packages/lib/services/trash/permanentlyDeleteOldItems.ts index 8fbccc106..73ddaa977 100644 --- a/packages/lib/services/trash/permanentlyDeleteOldItems.ts +++ b/packages/lib/services/trash/permanentlyDeleteOldItems.ts @@ -4,9 +4,44 @@ import Setting from '../../models/Setting'; import Note from '../../models/Note'; import { Day, Hour } from '@joplin/utils/time'; import shim from '../../shim'; +import { itemIsReadOnlySync } from '../../models/utils/readOnly'; +import BaseItem from '../../models/BaseItem'; +import { ModelType } from '../../BaseModel'; +import ItemChange from '../../models/ItemChange'; const logger = Logger.create('permanentlyDeleteOldData'); +const readOnlyItemsRemoved = async (itemIds: string[], itemType: ModelType) => { + const result = []; + for (const id of itemIds) { + const item = await BaseItem.loadItem(itemType, id); + + // Only do the share-related read-only checks. If other checks are done, + // readOnly will always be true because the item is in the trash. + const shareChecksOnly = true; + const readOnly = itemIsReadOnlySync( + itemType, + ItemChange.SOURCE_UNSPECIFIED, + item, + Setting.value('sync.userId'), + BaseItem.syncShareCache, + shareChecksOnly, + ); + if (!readOnly) { + result.push(id); + } + } + return result; +}; + +const itemsToDelete = async (ttl: number|null = null) => { + const result = await Folder.trashItemsOlderThan(ttl); + const folderIds = await readOnlyItemsRemoved(result.folderIds, ModelType.Folder); + const noteIds = await readOnlyItemsRemoved(result.noteIds, ModelType.Note); + + return { folderIds, noteIds }; +}; + const permanentlyDeleteOldItems = async (ttl: number = null) => { ttl = ttl === null ? Setting.value('trash.ttlDays') * Day : ttl; @@ -17,13 +52,13 @@ const permanentlyDeleteOldItems = async (ttl: number = null) => { return; } - const result = await Folder.trashItemsOlderThan(ttl); - logger.info('Items to permanently delete:', result); + const toDelete = await itemsToDelete(ttl); + logger.info('Items to permanently delete:', toDelete); - await Note.batchDelete(result.noteIds, { sourceDescription: 'permanentlyDeleteOldItems' }); + await Note.batchDelete(toDelete.noteIds, { sourceDescription: 'permanentlyDeleteOldItems' }); // We only auto-delete folders if they are empty. - for (const folderId of result.folderIds) { + for (const folderId of toDelete.folderIds) { const noteIds = await Folder.noteIds(folderId, { includeDeleted: true }); if (!noteIds.length) { logger.info(`Deleting empty folder: ${folderId}`);