MD Bookmarks: Remove deleted nodes from state tree

chrome/test/data/webui/md_bookmarks/reducers_test.js

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 suite('selection state', function() {
   var state;
   var action;
 
   function select(items, anchor, add) {
     return {
@@ skipping 60 matching lines @@
   });
 
   test('is cleared when search cleared', function() {
     action = select(['1', '2', '3'], '3', false);
     state = bookmarks.SelectionState.updateSelection(state, action);
 
     action = bookmarks.actions.clearSearch();
     state = bookmarks.SelectionState.updateSelection(state, action);
     assertDeepEquals({}, state.items);
   });
+
+  test('deselects items when they are deleted', function() {
+    var nodeList = testTree(createFolder('0', [
+      createFolder(
+          '1',
+          [
+            createItem('2'),
+            createItem('3'),
+            createItem('4'),
+          ]),
+      createItem('5'),
+    ]));
+
+    action = select(['2', '4', '5'], '4', false);
+    state = bookmarks.SelectionState.updateSelection(state, action);
+
+    action = bookmarks.actions.removeBookmark('1', '0', 0, nodeList);
+    state = bookmarks.SelectionState.updateSelection(state, action);
+
+    assertDeepEquals({5: true}, state.items);
+    assertEquals(null, state.anchor);
+  });
 });
 
 suite('closed folder state', function() {
   var nodes;
   var state;
   var action;
 
   setup(function() {
     nodes = testTree(
         createFolder(
@@ skipping 48 matching lines @@
   });
 });
 
 suite('selected folder', function() {
   var nodes;
   var state;
   var action;
 
   setup(function() {
     nodes = testTree(createFolder('1', [
-      createFolder('2', []),
+      createFolder(
+          '2',
+          [
+            createFolder('3', []),
+          ]),
     ]));
 
     state = '1';
   });
 
   test('updates from selectFolder action', function() {
     action = bookmarks.actions.selectFolder('2');
     state = bookmarks.SelectedFolderState.updateSelectedFolder(
         state, action, nodes);
     assertEquals('2', state);
   });
 
   test('updates when parent of selected folder is closed', function() {
     action = bookmarks.actions.selectFolder('2');
     state = bookmarks.SelectedFolderState.updateSelectedFolder(
         state, action, nodes);
 
     action = bookmarks.actions.changeFolderOpen('1', false);
     state = bookmarks.SelectedFolderState.updateSelectedFolder(
         state, action, nodes);
     assertEquals('1', state);
   });
+
+  test('selects ancestor when selected folder is deleted', function() {
+    action = bookmarks.actions.selectFolder('3');
+    state = bookmarks.SelectedFolderState.updateSelectedFolder(
+        state, action, nodes);
+
+    // Delete the selected folder:
+    action = bookmarks.actions.removeBookmark('3', '2', 0, nodes);
+    state = bookmarks.SelectedFolderState.updateSelectedFolder(
+        state, action, nodes);
+
+    assertEquals('2', state);
+
+    action = bookmarks.actions.selectFolder('3');

[calamity, 2017/04/03 06:54:24]

This is technically selecting a deleted folder, but since we're just looking at
selection state, we don't care. However, this is a bit confusing between state
is used elsewhere to mean the actual store.data 'state'.

I might just take a sledgehammer to this test and rename all the 'state's to the
piece of the state they represent. WDYT?

[tsergeant, 2017/04/04 04:42:28]

Seems like a reasonable change.

In the meantime, I've added a new sibling folder to select here to make it a bit
less confusing.

+    state = bookmarks.SelectedFolderState.updateSelectedFolder(
+        state, action, nodes);
+
+    // Delete an ancestor of the selected folder:
+    action = bookmarks.actions.removeBookmark('2', '1', 0, nodes);
+    state = bookmarks.SelectedFolderState.updateSelectedFolder(
+        state, action, nodes);
+
+    assertEquals('1', state);
+  });
 });
 
 suite('node state', function() {
   var state;
   var action;
 
   setup(function() {
     state = testTree(
         createFolder(
             '1',
@@ skipping 26 matching lines @@
 
     // Cannot edit URL of a folder:
     action = bookmarks.actions.editBookmark('4', {url: 'folder.com'});
     state = bookmarks.NodeState.updateNodes(state, action);
 
     assertEquals('folder', state['4'].title);
     assertEquals(undefined, state['4'].url);
   });
 
   test('updates when a node is deleted', function() {
-    action = bookmarks.actions.removeBookmark('3', '1', 1);
+    action = bookmarks.actions.removeBookmark('3', '1', 1, state);
     state = bookmarks.NodeState.updateNodes(state, action);
 
     assertDeepEquals(['2', '4'], state['1'].children);
 
-    // TODO(tsergeant): Deleted nodes should be removed from the nodes map
-    // entirely.
+    assertDeepEquals(['2', '4'], state['1'].children);
+    assertEquals(undefined, state['3']);
+  });
+
+  test('removes all children of deleted nodes', function() {
+    action = bookmarks.actions.removeBookmark('1', '0', 0, state);
+    state = bookmarks.NodeState.updateNodes(state, action);
+
+    assertDeepEquals(['0', '5'], Object.keys(state).sort());
   });
 
   test('updates when a node is moved', function() {
     // Move within the same folder backwards.
     action = bookmarks.actions.moveBookmark('3', '1', 0, '1', 1);
     state = bookmarks.NodeState.updateNodes(state, action);
 
     assertDeepEquals(['3', '2', '4'], state['1'].children);
 
     // Move within the same folder forwards.
@@ skipping 63 matching lines @@
 
     // Case 2: Clear search by selecting a new folder.
     action = bookmarks.actions.selectFolder('2');
     var selectedState = bookmarks.reduceAction(searchedState, action);
 
     assertEquals('2', selectedState.selectedFolder);
     assertDeepEquals(['3'], bookmarks.util.getDisplayedList(selectedState));
     assertEquals('', selectedState.search.term);
     assertDeepEquals([], selectedState.search.results);
   });
+
+  test('removes deleted nodes', function() {
+    var action;
+
+    action = bookmarks.actions.setSearchTerm('test');
+    state = bookmarks.reduceAction(state, action);
+
+    action = bookmarks.actions.setSearchResults(['1', '3', '2']);
+    state = bookmarks.reduceAction(state, action);
+
+    action = bookmarks.actions.removeBookmark('3', '2', 0, state.nodes);
+    state = bookmarks.reduceAction(state, action);
+
+    assertDeepEquals(['1', '2'], state.search.results);

[calamity, 2017/04/03 06:54:24]

Also delete 1 and ensure 2 vanishes due to being a descendant.

[tsergeant, 2017/04/04 04:42:28]

Done, I changed it to delete 2 and check 2 and 3 vanish in one go.

+  });
 });