From b9139b54ac1235c7bbf713445bca5c9522056462 Mon Sep 17 00:00:00 2001
From: Cezar Augusto <cezaraugusto@outlook.it>
Date: Fri, 16 Dec 2016 13:54:14 -0200
Subject: [PATCH] Keep bookmarked entries on history

Auditor: @bsclifton, @darkdh

Fix #6160

Test plan:

Automated tests must pass
npm run test -- --grep="returns newSiteDetail value for lastAccessedTime when oldSite value is undefined"

npm run test -- --grep="returns oldSiteDetail value for lastAccessedTime when newSite value is undefined"
---
 js/state/siteUtil.js            |  5 ++++-
 test/unit/state/siteUtilTest.js | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
index b85aaaad1ae..2887202feee 100644
--- 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
index a323cd47b66..c440dc4f4b9 100644
--- 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']))
+      })
     })
   })