Desktop, Mobile: Fixes #13258: Prevent new notes from being created in trashed or missing notebooks in certain cases#13575
Conversation
mrjo118
changed the title
| let folderId = Setting.value('activeFolderId'); | ||
| if (!folderId) return; | ||
|
|
||
| const folder = await Folder.load(folderId); | ||
| if (!folder || !!folder.deleted_time) { | ||
| const defaultFolder = await Folder.defaultFolder(); | ||
| if (!defaultFolder) return; | ||
| folderId = defaultFolder.id; | ||
| } |
There was a problem hiding this comment.
The whole idea with this "activeFolderId" was to have a guaranteed, valid, target whenever a notebook is needed. It seems the concept is broken now since the introduction of the trash folder, and we don't want to have this checking logic duplicate everywhere.
Maybe an alternative would be to created a helper function like "getValidNotebook" that is guaranteed to return such a notebook?
Or maybe this command should just throw if someone attempts to create a notebook when the trash is selected?
In fact, based on newNoteEnabledConditions, this command is not even enabled in the trash, so maybe that's the issue, that it can be called even though it's disabled?
There was a problem hiding this comment.
"Or maybe this command should just throw if someone attempts to create a notebook when the trash is selected?"
That's not really suitable, because the issue isn't when the 'trash' is selected, the issue is when a 'trashed notebook' is selected. The existing behaviour to prevent creating notebooks in the trash directly is working.
Using a helper method makes sense though.
|
@laurent22 I've updated the PR to use a new helper function |
|
that looks good now, thank you! |
2 participants
On both desktop and mobile, when switching to a notebook which is in the trash, then switching to the note list view by selecting a tag, the new note button allows you to create the note in the trashed notebook. This addresses the issue by setting the default notebook as the note parent if the current 'active notebook' is trashed or missing. In addition, the default notebook function is (incorrectly) able to return a notebook which is trashed, so I have amended this function to exclude trashed notebooks
This fixes #13258
Testing
Video before the change:
new.note.trashed.notebook.before.mp4
Video after on desktop:
new.note.trashed.notebook.after.mp4
Video after on mobile:
new.note.trashed.notebook.after.mobile.mp4