fixes for invalid operations on root note

Author: zadam

db/migrations/0114__fix_root_note_cycle.sql

@@ -0,0 +1 @@
+update branches set parentNoteId = 'none' where branchId = 'root'

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;
     }
 

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;

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);
         }
     },

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("/");
 }
 

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) {

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

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 = {

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}`);

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)) {