removeSite should also remove its children

fix brave#3763 fix brave#4124 Auditors: @bsclifton Test Plan: 1. Add folder "folder1" under bookmarks toolbar 2. Add folder "folder2" under "folder1" 3. Bookmark "facebook.com" and put it under "folder1" 4. Bookmark "twitter.com" and put it under "folder2" 5. Delete "folder1" 6. "facebook.com" and "twitter.com" should be unstarred
darkdh · Sep 20, 2016 · 036effa · 036effa
1 parent 6e628fb
commit 036effa
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 0 deletions.
diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -159,7 +159,18 @@ module.exports.removeSite = function (sites, siteDetail, tag) {
   if (index === -1) {
     return sites
   }
+
   const tags = sites.getIn([index, 'tags'])
+  if (isBookmarkFolder(tags)) {
+    const folderId = sites.getIn([index, 'folderId'])
+    const childSites = sites.filter((site) => site.get('parentFolderId') === folderId)
+    childSites.forEach((site) => {
+      const tags = site.get('tags')
+      tags.forEach((tag) => {
+        sites = module.exports.removeSite(sites, site, tag)
+      })
+    })
+  }
   if (tags.size === 0 && !tag) {
     // If called without tags and entry has no tags, remove the entry
     return sites.splice(index, 1)

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -251,6 +251,60 @@ describe('siteUtil', function () {
           .setIn([0, 'tags'], Immutable.List([]))
         assert.deepEqual(processedSites, expectedSites)
       })
+      it('removes folder and its children', function () {
+        const sites = Immutable.fromJS([
+          {
+            folderId: 1,
+            parentFolderId: 0,
+            tags: [siteTags.BOOKMARK_FOLDER]
+          },
+          {
+            folderId: 2,
+            parentFolderId: 1,
+            tags: [siteTags.BOOKMARK_FOLDER]
+          },
+          {
+            parentFolderId: 1,
+            location: testUrl1,
+            tags: [siteTags.BOOKMARK]
+          },
+          {
+            parentFolderId: 2,
+            location: testUrl2,
+            tags: [siteTags.BOOKMARK]
+          }
+        ])
+        const siteDetail = {
+          folderId: 1,
+          parentFolderId: 0,
+          tags: [siteTags.BOOKMARK_FOLDER]
+        }
+        const processedSites = siteUtil.removeSite(sites, Immutable.fromJS(siteDetail), siteTags.BOOKMARK_FOLDER)
+        const expectedSites = Immutable.fromJS([
+          {
+            folderId: 1,
+            parentFolderId: 0,
+            tags: Immutable.List([])
+          },
+          {
+            folderId: 2,
+            parentFolderId: 0,
+            tags: Immutable.List([])
+          },
+          {
+            parentFolderId: 0,
+            location: testUrl1,
+            tags: Immutable.List([])
+          },
+          {
+            parentFolderId: 0,
+            location: testUrl2,
+            tags: Immutable.List([])
+          }
+        ])
+        // toJS needed because immutable ownerID :(
+        assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
+      })
     })
     describe('tag=falsey', function () {
       it('deletes a history entry (has no tags)', function () {