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 · 362a8bb · 362a8bb
1 parent 5de9547
commit 362a8bb
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 () {