Follow up for sites splits PR brave#10296

Resolves brave#10296
Resolves brave#10348

Auditors:

Test Plan:

app/common/state/bookmarkFoldersState.js

@@ -36,19 +36,33 @@ const bookmarkFoldersState = {
 
   getFolder: (state, folderKey) => {
     state = validateState(state)
+
+    if (folderKey == null) {
+      return Immutable.Map()
+    }
+
     folderKey = folderKey.toString()
     return state.getIn([STATE_SITES.BOOKMARK_FOLDERS, folderKey], Immutable.Map())
   },
 
   getFoldersByParentId: (state, parentFolderId) => {
     state = validateState(state)
 
+    if (parentFolderId == null) {
+      return Immutable.List()
+    }
+
     const folders = bookmarkOrderCache.getFoldersByParentId(state, parentFolderId)
     return folders.map(folder => bookmarkFoldersState.getFolder(state, folder.get('key')))
   },
 
   addFolder: (state, folderDetails, destinationKey) => {
     state = validateState(state)
+
+    if (folderDetails == null) {
+      return state
+    }
+
     folderDetails = makeImmutable(folderDetails)
     let folders = bookmarkFoldersState.getFolders(state)
     let key = folderDetails.get('folderId')

app/common/state/bookmarksState.js

@@ -266,16 +266,20 @@ const bookmarksState = {
         : destinationItem.get('folderId')
 
       state = bookmarkOrderCache.removeCacheKey(state, bookmark.get('parentFolderId'), bookmarkKey)
+      state = bookmarkLocationCache.removeCacheKey(state, bookmark.get('location'), bookmarkKey)
       bookmark = bookmark.set('parentFolderId', ~~parentFolderId)
       const newKey = bookmarkUtil.getKey(bookmark)
       state = state.deleteIn([STATE_SITES.BOOKMARKS, bookmarkKey])
       state = bookmarkOrderCache.addBookmarkToCache(state, bookmark.get('parentFolderId'), newKey)
+      state = bookmarkLocationCache.addCacheKey(state, bookmark.get('location'), newKey)
       bookmark = bookmark.set('key', newKey)
       return state.setIn([STATE_SITES.BOOKMARKS, newKey], bookmark)
     }
 
     // move bookmark to another place
     state = bookmarkOrderCache.removeCacheKey(state, bookmark.get('parentFolderId'), bookmarkKey)
+    state = bookmarkLocationCache.removeCacheKey(state, bookmark.get('location'), bookmarkKey)
+    state = bookmarkLocationCache.addCacheKey(state, bookmark.get('location'), bookmarkKey)
     state = bookmarkOrderCache.addBookmarkToCache(
       state,
       bookmark.get('parentFolderId'),

test/unit/app/common/state/aboutHistoryStateTest.js

@@ -1,29 +1,39 @@
-/* global describe, it */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* global describe, it, before, after */
 const aboutHistoryState = require('../../../../../app/common/state/aboutHistoryState')
 const Immutable = require('immutable')
 const assert = require('assert')
+const sinon = require('sinon')
 
 const defaultAppState = Immutable.fromJS({
   about: {
     history: {
-      entries: [],
-      updatedStamp: undefined
+      entries: {},
+      updatedStamp: 0
     }
   }
 })
 
 const arbitraryTimeInThePast = 1450000000000
 
+const historyItems = Immutable.fromJS({
+  'https://brave.com/|0': {
+    location: 'https://brave.com'
+  }
+})
+
 const assertTimeUpdated = (state) => {
   const updatedStamp = state.getIn(['about', 'history', 'updatedStamp'])
   assert.equal(typeof updatedStamp === 'number' && updatedStamp > arbitraryTimeInThePast, true)
 }
 
-describe('aboutHistoryState', function () {
+describe('aboutHistoryState unit test', function () {
   describe('getHistory', function () {
     it('reads the history from the state', function () {
-      const fakeHistoryEntries = [1, 2, 3]
-      const state = defaultAppState.setIn(['about', 'history', 'entries'], fakeHistoryEntries)
+      const state = defaultAppState.setIn(['about', 'history', 'entries'], historyItems)
       const history = aboutHistoryState.getHistory(state)
       assert.deepEqual(state.getIn(['about', 'history']).toJS(), history.toJS())
     })
@@ -35,4 +45,21 @@ describe('aboutHistoryState', function () {
       assertTimeUpdated(state)
     })
   })
+
+  describe('clearHistory', function () {
+    before(function () {
+      this.clock = sinon.useFakeTimers()
+      this.clock.tick(0)
+    })
+
+    after(function () {
+      this.clock.restore()
+    })
+
+    it('history is cleared', function () {
+      const state = defaultAppState.setIn(['about', 'history', 'entries'], historyItems)
+      const history = aboutHistoryState.clearHistory(state)
+      assert.deepEqual(history.toJS(), defaultAppState.toJS())
+    })
+  })
 })

test/unit/app/common/state/aboutNewTabStateTest.js

@@ -1,7 +1,12 @@
-/* global describe, it */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* global describe, it, before, after */
 const aboutNewTabState = require('../../../../../app/common/state/aboutNewTabState')
 const Immutable = require('immutable')
 const assert = require('assert')
+const sinon = require('sinon')
 
 const defaultAppState = Immutable.fromJS({
   about: {
@@ -10,7 +15,7 @@ const defaultAppState = Immutable.fromJS({
       sites: [],
       ignoredTopSites: [],
       pinnedTopSites: [],
-      updatedStamp: undefined
+      updatedStamp: 0
     }
   }
 })
@@ -26,10 +31,10 @@ const assertTimeUpdated = (state) => {
 const assertNoChange = (state) => {
   const updatedStamp = state.getIn(['about', 'newtab', 'updatedStamp'])
   assert.deepEqual(state, defaultAppState)
-  assert.equal(updatedStamp, undefined)
+  assert.equal(updatedStamp, 0)
 }
 
-describe('aboutNewTabState', function () {
+describe('aboutNewTabState unit test', function () {
   describe('getSites', function () {
     it('returns the contents of about.newtab.sites', function () {
       const expectedSites = Immutable.List().push(1).push(2).push(3)
@@ -87,4 +92,25 @@ describe('aboutNewTabState', function () {
       assert.equal(updatedValue, 'TEST STRING')
     })
   })
+
+  describe('clearTopSites', function () {
+    before(function () {
+      this.clock = sinon.useFakeTimers()
+      this.clock.tick(0)
+    })
+
+    after(function () {
+      this.clock.restore()
+    })
+
+    it('is cleared', function () {
+      const state = defaultAppState.setIn(['about', 'newtab', 'sites'], Immutable.fromJS([
+        {
+          location: 'https://brave.com'
+        }
+      ]))
+      const topSItes = aboutNewTabState.clearTopSites(state)
+      assert.deepEqual(topSItes.toJS(), defaultAppState.toJS())
+    })
+  })
 })