1. Add default lastAccessTime for history entries only

2. Add creationTime field fix #5183 Auditors: @bsclifton, @bbondy Test Plan: 1. Import bookmarks 2. They should not appear in about:history until you visit them
brave · Nov 2, 2016 · c6eed6d · c6eed6d · bsclifton · Nov 3, 2016
1 parent 425cbf4
commit c6eed6d
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 7 deletions.
diff --git a/app/importer.js b/app/importer.js
@@ -106,7 +106,8 @@ const getParentFolderId = (path, pathMap, sites, topLevelFolderId, nextFolderIdO
       customTitle: parentFolder,
       folderId: parentFolderId,
       parentFolderId: getParentFolderId(path, pathMap, sites, topLevelFolderId, nextFolderIdObject),
-      lastAccessedTime: (new Date()).getTime(),
+      lastAccessedTime: 0,
+      creationTime: (new Date()).getTime(),
       tags: [siteTags.BOOKMARK_FOLDER]
     }
     sites.push(folder)
@@ -126,7 +127,8 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
       customTitle: topLevelFolder,
       folderId: topLevelFolderId,
       parentFolderId: 0,
-      lastAccessedTime: (new Date()).getTime(),
+      lastAccessedTime: 0,
+      creationTime: (new Date()).getTime(),
       tags: [siteTags.BOOKMARK_FOLDER]
     })
   } else {
@@ -150,7 +152,8 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
         customTitle: bookmarks[i].title,
         folderId: folderId,
         parentFolderId: parentFolderId,
-        lastAccessedTime: bookmarks[i].creation_time * 1000,
+        lastAccessedTime: 0,
+        creationTime: bookmarks[i].creation_time * 1000,
         tags: [siteTags.BOOKMARK_FOLDER]
       }
       sites.push(folder)
@@ -160,7 +163,8 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
         customTitle: bookmarks[i].title,
         location: bookmarks[i].url,
         parentFolderId: parentFolderId,
-        lastAccessedTime: bookmarks[i].creation_time * 1000,
+        lastAccessedTime: 0,
+        creationTime: bookmarks[i].creation_time * 1000,
         tags: [siteTags.BOOKMARK]
       }
       sites.push(site)

diff --git a/docs/state.md b/docs/state.md
@@ -42,6 +42,7 @@ AppStore
     favicon: string, // URL of the favicon
     themeColor: string, // css compatible color string
     lastAccessedTime: number, // datetime.getTime()
+    creationTime: number, //creation time of bookmark
     partitionNumber: number, // Optionally specifies a specific session
     folderId: number, // Set for bookmark folders only
     parentFolderId: number // Set for bookmarks and bookmark folders only

diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -93,8 +93,15 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId) => {
     ? newSiteDetail.get('customTitle')
     : (newSiteDetail.get('customTitle') || oldSiteDetail && oldSiteDetail.get('customTitle'))
 
+  let lastAccessedTime
+  if (isBookmark(tag) || isBookmarkFolder(tag)) {
+    lastAccessedTime = newSiteDetail.get('lastAccessedTime') || 0
+  } else {
+    lastAccessedTime = newSiteDetail.get('lastAccessedTime') || new Date().getTime()
+  }
+
   let site = Immutable.fromJS({
-    lastAccessedTime: newSiteDetail.get('lastAccessedTime') || new Date().getTime(),
+    lastAccessedTime: lastAccessedTime,
     tags,
     title: newSiteDetail.get('title')
   })

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -200,9 +200,9 @@ describe('siteUtil', function () {
           const expectedSites = Immutable.fromJS([bookmarkAllFields])
           assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
         })
-        it('sets default values for lastAccessedTime and tag when they are missing', function () {
+        it('sets 0 for lastAccessedTime if not specified', function () {
           const processedSites = siteUtil.addSite(emptySites, bookmarkMinFields, siteTags.BOOKMARK)
-          assert.equal(!!processedSites.getIn([0, 'lastAccessedTime']), true)
+          assert.equal(processedSites.getIn([0, 'lastAccessedTime']), 0)
           assert.deepEqual(processedSites.getIn([0, 'tags']).toJS(), [siteTags.BOOKMARK])
         })
       })
@@ -279,6 +279,13 @@ describe('siteUtil', function () {
           assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
         })
       })
+      describe('when adding history', function () {
+        it('sets default values for lastAccessedTime and tag when they are missing', function () {
+          const processedSites = siteUtil.addSite(emptySites, bookmarkMinFields)
+          assert.equal(!!processedSites.getIn([0, 'lastAccessedTime']), true)
+          assert.deepEqual(processedSites.getIn([0, 'tags']).toJS(), [])
+        })
+      })
     })
 
     describe('for existing entries (oldSite is an existing siteDetail)', function () {