From 9f002fa8024e8ab60bba49da16cc610bfdad51f7 Mon Sep 17 00:00:00 2001 From: zadam Date: Sat, 6 Mar 2021 20:23:29 +0100 Subject: [PATCH] change the heuristics to choose the best note path when ambiguous/incomplete/just noteId is provided, #1711 --- src/public/app/entities/note_short.js | 26 ++++++++++++ src/public/app/services/branches.js | 2 +- src/public/app/services/hoisted_note.js | 6 +-- src/public/app/services/tree.js | 55 ++++++++++++------------- src/public/app/widgets/note_tree.js | 14 ++----- 5 files changed, 60 insertions(+), 43 deletions(-) diff --git a/src/public/app/entities/note_short.js b/src/public/app/entities/note_short.js index 32108e15c..a7b031334 100644 --- a/src/public/app/entities/note_short.js +++ b/src/public/app/entities/note_short.js @@ -253,6 +253,32 @@ class NoteShort { return noteAttributeCache.attributes[this.noteId]; } + getAllNotePaths() { + if (this.noteId === 'root') { + return [['root']]; + } + + const parentNotes = this.getParentNotes(); + let paths; + + if (parentNotes.length === 1) { // optimization for the most common case + paths = parentNotes[0].getAllNotePaths(); + } + else { + paths = []; + + for (const parentNote of parentNotes) { + paths.push(...parentNote.getAllNotePaths()); + } + } + + for (const path of paths) { + path.push(this.noteId); + } + + return paths; + } + __filterAttrs(attributes, type, name) { if (!type && !name) { return attributes; diff --git a/src/public/app/services/branches.js b/src/public/app/services/branches.js index b04e8602c..0ae9c0a96 100644 --- a/src/public/app/services/branches.js +++ b/src/public/app/services/branches.js @@ -125,7 +125,7 @@ async function deleteNotes(branchIdsToDelete) { } async function moveNodeUpInHierarchy(node) { - if (hoistedNoteService.isRootNode(node) + if (hoistedNoteService.isHoistedNode(node) || hoistedNoteService.isTopLevelNode(node) || node.getParent().data.noteType === 'search') { return; diff --git a/src/public/app/services/hoisted_note.js b/src/public/app/services/hoisted_note.js index fffd5d4ab..f7c97e103 100644 --- a/src/public/app/services/hoisted_note.js +++ b/src/public/app/services/hoisted_note.js @@ -16,10 +16,10 @@ async function unhoist() { } function isTopLevelNode(node) { - return isRootNode(node.getParent()); + return isHoistedNode(node.getParent()); } -function isRootNode(node) { +function isHoistedNode(node) { // even though check for 'root' should not be necessary, we keep it just in case return node.data.noteId === "root" || node.data.noteId === getHoistedNoteId(); @@ -55,6 +55,6 @@ export default { getHoistedNoteId, unhoist, isTopLevelNode, - isRootNode, + isHoistedNode, checkNoteAccess } diff --git a/src/public/app/services/tree.js b/src/public/app/services/tree.js index e7a97cf32..83b3a3cbb 100644 --- a/src/public/app/services/tree.js +++ b/src/public/app/services/tree.js @@ -75,14 +75,10 @@ async function resolveNotePathToSegments(notePath, hoistedNoteId = 'root', logEr console.log(utils.now(), `Did not find parent ${parentNoteId} (${parent ? parent.title : 'n/a'}) for child ${childNoteId} (${child.title}), available parents: ${parents.map(p => `${p.noteId} (${p.title})`)}`); } - const hoistedNote = await treeCache.getNote(hoistedNoteId); - - const chosenParent = parents.find(parent => parent.hasAncestor(hoistedNote)) - || parents[0]; // if no parent is in hoisted subtree then it just doesn't matter and pick any - const someNotePath = getSomeNotePath(chosenParent); + const someNotePath = getSomeNotePath(child, hoistedNoteId); if (someNotePath) { // in case it's root the path may be empty - const pathToRoot = someNotePath.split("/").reverse(); + const pathToRoot = someNotePath.split("/").reverse().slice(1); for (const noteId of pathToRoot) { effectivePath.push(noteId); @@ -100,29 +96,35 @@ async function resolveNotePathToSegments(notePath, hoistedNoteId = 'root', logEr return effectivePath.reverse(); } -function getSomeNotePath(note) { +function getSomeNotePathSegments(note, hoistedNotePath = 'root') { utils.assertArguments(note); - const path = []; + const notePaths = note.getAllNotePaths().map(path => ({ + notePath: path, + isInHoistedSubTree: path.includes(hoistedNotePath), + isArchived: path.find(noteId => treeCache.notes[noteId].hasLabel('archived')), + isSearch: path.find(noteId => treeCache.notes[noteId].type === 'search') + })); - let cur = note; - - while (cur.noteId !== 'root') { - path.push(cur.noteId); - - const parents = cur.getParentNotes().filter(note => note.type !== 'search'); - - if (!parents.length) { - logError(`Can't find parents for note ${cur.noteId}`); - return; + notePaths.sort((a, b) => { + if (a.isInHoistedSubTree !== b.isInHoistedSubTree) { + return a.isInHoistedSubTree ? -1 : 1; + } else if (a.isSearch !== b.isSearch) { + return a.isSearch ? 1 : -1; + } else if (a.isArchived !== b.isArchived) { + return a.isArchived ? 1 : -1; + } else { + return a.notePath.length - b.notePath.length; } + }); - cur = parents[0]; - } + return notePaths[0].notePath; +} - path.push('root'); +function getSomeNotePath(note, hoistedNotePath = 'root') { + const notePath = getSomeNotePathSegments(note, hoistedNotePath); - return path.reverse().join('/'); + return notePath.join('/'); } async function sortAlphabetically(noteId) { @@ -142,7 +144,7 @@ ws.subscribeToMessages(message => { }); function getParentProtectedStatus(node) { - return hoistedNoteService.isRootNode(node) ? 0 : node.getParent().data.isProtected; + return hoistedNoteService.isHoistedNode(node) ? 0 : node.getParent().data.isProtected; } function getNoteIdFromNotePath(notePath) { @@ -202,7 +204,7 @@ function getNotePath(node) { const path = []; - while (node && !hoistedNoteService.isRootNode(node)) { + while (node) { if (node.data.noteId) { path.push(node.data.noteId); } @@ -210,10 +212,6 @@ function getNotePath(node) { node = node.getParent(); } - if (node) { // null node can happen directly after unhoisting when tree is still hoisted but option has been changed already - path.push(node.data.noteId); // root or hoisted noteId - } - return path.reverse().join("/"); } @@ -317,6 +315,7 @@ export default { resolveNotePath, resolveNotePathToSegments, getSomeNotePath, + getSomeNotePathSegments, getParentProtectedStatus, getNotePath, getNoteIdFromNotePath, diff --git a/src/public/app/widgets/note_tree.js b/src/public/app/widgets/note_tree.js index 7d056e86e..357e3a8b4 100644 --- a/src/public/app/widgets/note_tree.js +++ b/src/public/app/widgets/note_tree.js @@ -1210,18 +1210,10 @@ export default class NoteTreeWidget extends TabAwareWidget { this.tree.clearFilter(); } else { - let found = false;console.log(this.tabContext.notePath, this.tabContext.hoistedNoteId, "ZZZ"); - // hack when hoisted note is cloned then it could be filtered multiple times while we want only 1 - this.tree.filterBranches(node => { - if (found) { - return false; - } - - found = node.data.noteId === this.tabContext.hoistedNoteId; - - return found; - }); + this.tree.filterBranches(node => + node.data.noteId === this.tabContext.hoistedNoteId // optimization to not having always resolve the node path + && treeService.getNotePath(node) === hoistedNotePath); } } }