From bfa2b5fa62d260b157c261e3fcf859f23c3c039e Mon Sep 17 00:00:00 2001 From: azivner Date: Sun, 21 Oct 2018 21:37:34 +0200 Subject: [PATCH 1/2] add a check for root parent --- src/services/consistency_checks.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/services/consistency_checks.js b/src/services/consistency_checks.js index 44abe110d..3671bf970 100644 --- a/src/services/consistency_checks.js +++ b/src/services/consistency_checks.js @@ -28,10 +28,7 @@ async function checkTreeCycles(errorList) { const childNoteId = row.noteId; const parentNoteId = row.parentNoteId; - if (!childToParents[childNoteId]) { - childToParents[childNoteId] = []; - } - + childToParents[childNoteId] = childToParents[childNoteId] || []; childToParents[childNoteId].push(parentNoteId); } @@ -58,6 +55,10 @@ async function checkTreeCycles(errorList) { for (const noteId of noteIds) { checkTreeCycle(noteId, [], errorList); } + + if (childToParents['root'].length !== 1 || childToParents['root'][0] !== 'none') { + errorList.push('Incorrect root parent: ' + JSON.stringify(childToParents['root'])); + } } async function runSyncRowChecks(table, key, errorList) { From 0c007566ad31a98ad585fa97435fc9306ec0a3c8 Mon Sep 17 00:00:00 2001 From: azivner Date: Sun, 21 Oct 2018 22:42:20 +0200 Subject: [PATCH 2/2] fixes for invalid operations on root note --- db/migrations/0114__fix_root_note_cycle.sql | 1 + src/public/javascripts/services/branches.js | 25 +++++++++++++++++-- .../javascripts/services/drag_and_drop.js | 4 +++ .../javascripts/services/tree_keybindings.js | 2 +- src/public/javascripts/services/tree_utils.js | 4 ++- src/public/javascripts/services/utils.js | 2 +- src/routes/api/branches.js | 6 ++--- src/services/app_info.js | 2 +- src/services/consistency_checks.js | 5 ++++ src/services/tree.js | 9 +++++++ 10 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 db/migrations/0114__fix_root_note_cycle.sql diff --git a/db/migrations/0114__fix_root_note_cycle.sql b/db/migrations/0114__fix_root_note_cycle.sql new file mode 100644 index 000000000..bd65c1b9c --- /dev/null +++ b/db/migrations/0114__fix_root_note_cycle.sql @@ -0,0 +1 @@ +update branches set parentNoteId = 'none' where branchId = 'root' \ No newline at end of file diff --git a/src/public/javascripts/services/branches.js b/src/public/javascripts/services/branches.js index 88deca265..062515f34 100644 --- a/src/public/javascripts/services/branches.js +++ b/src/public/javascripts/services/branches.js @@ -5,6 +5,13 @@ import infoService from "./info.js"; import treeCache from "./tree_cache.js"; async function moveBeforeNode(nodesToMove, beforeNode) { + nodesToMove = filterRootNote(nodesToMove); + + if (beforeNode.data.noteId === 'root') { + alert('Cannot move notes before root note.'); + return; + } + for (const nodeToMove of nodesToMove) { const resp = await server.put('branches/' + nodeToMove.data.branchId + '/move-before/' + beforeNode.data.branchId); @@ -18,6 +25,13 @@ async function moveBeforeNode(nodesToMove, beforeNode) { } async function moveAfterNode(nodesToMove, afterNode) { + nodesToMove = filterRootNote(nodesToMove); + + if (afterNode.data.noteId === 'root') { + alert('Cannot move notes after root note.'); + return; + } + nodesToMove.reverse(); // need to reverse to keep the note order for (const nodeToMove of nodesToMove) { @@ -33,6 +47,8 @@ async function moveAfterNode(nodesToMove, afterNode) { } async function moveToNode(nodesToMove, toNode) { + nodesToMove = filterRootNote(nodesToMove); + for (const nodeToMove of nodesToMove) { const resp = await server.put('branches/' + nodeToMove.data.branchId + '/move-to/' + toNode.data.noteId); @@ -58,8 +74,13 @@ async function moveToNode(nodesToMove, toNode) { } } +function filterRootNote(nodes) { + // some operations are not possible on root notes + return nodes.filter(node => node.data.noteId !== 'root'); +} + async function deleteNodes(nodes) { - nodes = nodes.filter(node => node.data.noteId !== 'root'); + nodes = filterRootNote(nodes); if (nodes.length === 0 || !confirm('Are you sure you want to delete select note(s) and all the sub-notes?')) { return; @@ -94,7 +115,7 @@ async function deleteNodes(nodes) { } async function moveNodeUpInHierarchy(node) { - if (utils.isTopLevelNode(node)) { + if (utils.isRootNode(node) || utils.isTopLevelNode(node)) { return; } diff --git a/src/public/javascripts/services/drag_and_drop.js b/src/public/javascripts/services/drag_and_drop.js index d9843ae27..8de46e49b 100644 --- a/src/public/javascripts/services/drag_and_drop.js +++ b/src/public/javascripts/services/drag_and_drop.js @@ -14,6 +14,10 @@ const dragAndDropSetup = { preventVoidMoves: true, // Prevent dropping nodes 'before self', etc. dragStart: (node, data) => { + if (node.data.noteId === 'root') { + return false; + } + // This function MUST be defined to enable dragging for the tree. // Return false to cancel dragging of node. return true; diff --git a/src/public/javascripts/services/tree_keybindings.js b/src/public/javascripts/services/tree_keybindings.js index 7219994d8..ec3d6b87a 100644 --- a/src/public/javascripts/services/tree_keybindings.js +++ b/src/public/javascripts/services/tree_keybindings.js @@ -105,7 +105,7 @@ const keyBindings = { return false; }, "backspace": node => { - if (!utils.isTopLevelNode(node)) { + if (!utils.isRootNode(node)) { node.getParent().setActive().then(treeService.clearSelectedNodes); } }, diff --git a/src/public/javascripts/services/tree_utils.js b/src/public/javascripts/services/tree_utils.js index 64a0dd5b6..1f05871ec 100644 --- a/src/public/javascripts/services/tree_utils.js +++ b/src/public/javascripts/services/tree_utils.js @@ -4,7 +4,7 @@ import treeCache from "./tree_cache.js"; const $tree = $("#tree"); function getParentProtectedStatus(node) { - return utils.isTopLevelNode(node) ? 0 : node.getParent().data.isProtected; + return utils.isRootNode(node) ? 0 : node.getParent().data.isProtected; } function getNodeByKey(key) { @@ -32,6 +32,8 @@ function getNotePath(node) { node = node.getParent(); } + path.push('root'); + return path.reverse().join("/"); } diff --git a/src/public/javascripts/services/utils.js b/src/public/javascripts/services/utils.js index 527b052ef..ea1f8edd1 100644 --- a/src/public/javascripts/services/utils.js +++ b/src/public/javascripts/services/utils.js @@ -59,7 +59,7 @@ function isTopLevelNode(node) { } function isRootNode(node) { - return node.key === "root_1"; + return node.data.noteId === "root"; } function escapeHtml(str) { diff --git a/src/routes/api/branches.js b/src/routes/api/branches.js index 67c88659a..cf0c91f3d 100644 --- a/src/routes/api/branches.js +++ b/src/routes/api/branches.js @@ -21,7 +21,7 @@ async function moveBranchToParent(req) { const validationResult = await tree.validateParentChild(parentNoteId, noteToMove.noteId, branchId); if (!validationResult.success) { - return [400, validationResult]; + return [200, validationResult]; } const maxNotePos = await sql.getValue('SELECT MAX(notePosition) FROM branches WHERE parentNoteId = ? AND isDeleted = 0', [parentNoteId]); @@ -45,7 +45,7 @@ async function moveBranchBeforeNote(req) { const validationResult = await tree.validateParentChild(beforeNote.parentNoteId, noteToMove.noteId, branchId); if (!validationResult.success) { - return [400, validationResult]; + return [200, validationResult]; } // we don't change dateModified so other changes are prioritized in case of conflict @@ -73,7 +73,7 @@ async function moveBranchAfterNote(req) { const validationResult = await tree.validateParentChild(afterNote.parentNoteId, noteToMove.noteId, branchId); if (!validationResult.success) { - return [400, validationResult]; + return [200, validationResult]; } // we don't change dateModified so other changes are prioritized in case of conflict diff --git a/src/services/app_info.js b/src/services/app_info.js index 133bbdba3..b1af8b454 100644 --- a/src/services/app_info.js +++ b/src/services/app_info.js @@ -3,7 +3,7 @@ const build = require('./build'); const packageJson = require('../../package'); -const APP_DB_VERSION = 113; +const APP_DB_VERSION = 114; const SYNC_VERSION = 1; module.exports = { diff --git a/src/services/consistency_checks.js b/src/services/consistency_checks.js index 3671bf970..b0a9191e5 100644 --- a/src/services/consistency_checks.js +++ b/src/services/consistency_checks.js @@ -37,6 +37,11 @@ async function checkTreeCycles(errorList) { return; } + if (!childToParents[noteId] || childToParents[noteId].length === 0) { + errorList.push(`No parents found for noteId=${noteId}`); + return; + } + for (const parentNoteId of childToParents[noteId]) { if (path.includes(parentNoteId)) { errorList.push(`Tree cycle detected at parent-child relationship: ${parentNoteId} - ${noteId}, whole path: ${path}`); diff --git a/src/services/tree.js b/src/services/tree.js index 53515784b..f51089d8b 100644 --- a/src/services/tree.js +++ b/src/services/tree.js @@ -7,6 +7,15 @@ const syncTableService = require('./sync_table'); const protectedSessionService = require('./protected_session'); async function validateParentChild(parentNoteId, childNoteId, branchId = null) { + if (childNoteId === 'root') { + return { success: false, message: 'Cannot move root note.'}; + } + + if (parentNoteId === 'none') { + // this shouldn't happen + return { success: false, message: 'Cannot move anything into root parent.' }; + } + const existing = await getExistingBranch(parentNoteId, childNoteId); if (existing && (branchId === null || existing.branchId !== branchId)) {