From 036effa12274dc73ddfd568dbef77773b43a1761 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Tue, 20 Sep 2016 21:51:14 +0800 Subject: [PATCH] removeSite should also remove its children fix #3763 fix #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 --- js/state/siteUtil.js | 11 +++++++ test/unit/state/siteUtilTest.js | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index a8a53a63b1d..d438c24ab7b 100644 --- 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 index 34a888b2e3c..c34a4dbf559 100644 --- 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 () {