Only remove bookmark tag when it's pinned

filterOutNonRecents will do the cleanup fix #7283 Auditors: @bbondy Test Plan: Covered by automatic test
brave · Feb 16, 2017 · 576b66a · 576b66a
1 parent 0d3dacf
commit 576b66a
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 8 deletions.
diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -299,25 +299,26 @@ module.exports.removeSite = function (sites, siteDetail, tag, reorder = true, sy
       })
     })
   }
+  let site = sites.get(key)
+  if (!site) {
+    return sites
+  }
   if (isBookmark(tag)) {
+    if (isPinnedTab(tags)) {
+      const tags = site.get('tags').filterNot((tag) => tag === siteTags.BOOKMARK)
+      site = site.set('tags', tags)
+      return sites.set(key, site)
+    }
     if (sites.size && reorder) {
       const order = sites.getIn([key, 'order'])
       sites = reorderSite(sites, order)
     }
     return sites.delete(key)
   } else if (isPinnedTab(tag)) {
-    let site = sites.get(key)
-    if (!site) {
-      return sites
-    }
     const tags = site.get('tags').filterNot((tag) => tag === siteTags.PINNED)
     site = site.set('tags', tags)
     return sites.set(key, site)
   } else {
-    let site = sites.get(key)
-    if (!site) {
-      return sites
-    }
     site = site.set('lastAccessedTime', undefined)
     return sites.set(key, site)
   }

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -491,6 +491,40 @@ describe('siteUtil', function () {
         const expectedSites = new Immutable.Map()
         assert.deepEqual(processedSites, expectedSites)
       })
+      it('removes the bookmark tag when it is pinned', function () {
+        const siteDetail = {
+          tags: [siteTags.BOOKMARK, siteTags.PINNED],
+          location: testUrl1
+        }
+        const expectedSites = {
+          'https://brave.com/|0|0': {
+            tags: [siteTags.PINNED],
+            location: testUrl1
+          }
+        }
+        const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail))
+        let sites = {}
+        sites[siteKey] = siteDetail
+        const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.BOOKMARK)
+        assert.deepEqual(processedSites.toJS(), expectedSites)
+      })
+      it('removes the pinned tag when it is bookmarked', function () {
+        const siteDetail = {
+          tags: [siteTags.BOOKMARK, siteTags.PINNED],
+          location: testUrl1
+        }
+        const expectedSites = {
+          'https://brave.com/|0|0': {
+            tags: [siteTags.BOOKMARK],
+            location: testUrl1
+          }
+        }
+        const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail))
+        let sites = {}
+        sites[siteKey] = siteDetail
+        const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.PINNED)
+        assert.deepEqual(processedSites.toJS(), expectedSites)
+      })
       it('removes folder and its children', function () {
         let sites = {}
         const site1 = {