Merge pull request #6211 from cezaraugusto/feature/history/6160

Keep bookmarked entries on history
brave · Jan 11, 2017 · 521d259 · 521d259
2 parents 7ce997b + b9139b5
commit 521d259
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 1 deletion.
diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -86,6 +86,7 @@ module.exports.getNextFolderId = (sites) => {
 // Some details can be copied from the existing siteDetail if null
 // ex: parentFolderId, partitionNumber, and favicon
 const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId) => {
+  const siteDetailExist = newSiteDetail.get('lastAccessedTime') !== undefined || oldSiteDetail && oldSiteDetail.get('lastAccessedTime')
   let tags = oldSiteDetail && oldSiteDetail.get('tags') || new Immutable.List()
   if (tag) {
     tags = tags.toSet().add(tag).toList()
@@ -97,7 +98,9 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId) => {
 
   let lastAccessedTime
   if (isBookmark(tag) || isBookmarkFolder(tag)) {
-    lastAccessedTime = newSiteDetail.get('lastAccessedTime') || 0
+    siteDetailExist
+      ? lastAccessedTime = newSiteDetail.get('lastAccessedTime') || oldSiteDetail.get('lastAccessedTime')
+      : lastAccessedTime = 0
   } else {
     lastAccessedTime = newSiteDetail.get('lastAccessedTime') || new Date().getTime()
   }

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -286,6 +286,11 @@ describe('siteUtil', function () {
           assert.equal(!!processedSites.getIn([0, 'lastAccessedTime']), true)
           assert.deepEqual(processedSites.getIn([0, 'tags']).toJS(), [])
         })
+        it('returns newSiteDetail value for lastAccessedTime when oldSite value is undefined', function () {
+          const processedSites = siteUtil.addSite(emptySites, bookmarkAllFields)
+          const expectedSites = Immutable.fromJS([bookmarkAllFields])
+          assert.deepEqual(processedSites.getIn([0, 'lastAccessedTime']), expectedSites.getIn([0, 'lastAccessedTime']))
+        })
       })
     })
     describe('for existing entries (oldSite is an existing siteDetail)', function () {
@@ -340,6 +345,23 @@ describe('siteUtil', function () {
         const expectedSites = Immutable.fromJS([newSiteDetail])
         assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
       })
+      it('returns oldSiteDetail value for lastAccessedTime when newSite value is undefined', function () {
+        const oldSiteDetail = Immutable.fromJS({
+          lastAccessedTime: 456,
+          location: testUrl1,
+          title: 'a brave title'
+        })
+        const newSiteDetail = Immutable.fromJS({
+          tags: [siteTags.BOOKMARK],
+          location: testUrl1,
+          title: 'a brave title'
+        })
+
+        const sites = Immutable.fromJS([oldSiteDetail])
+        const processedSites = siteUtil.addSite(sites, newSiteDetail, siteTags.BOOKMARK, oldSiteDetail)
+        const expectedSites = sites
+        assert.deepEqual(processedSites.getIn([0, 'lastAccessedTime']), expectedSites.getIn([0, 'lastAccessedTime']))
+      })
     })
   })