Removes history and bookmarks when they are removed from suggestions

Resolves brave#9736

Auditors:

Test Plan:

app/browser/reducers/historyReducer.js

@@ -16,9 +16,11 @@ const messages = require('../../../js/constants/messages')
 
 // Utils
 const {makeImmutable} = require('../../common/state/immutableUtil')
+const {remove} = require('../../common/lib/siteSuggestions')
 const syncUtil = require('../../../js/state/syncUtil')
 const filtering = require('../../filtering')
 const {calculateTopSites} = require('../api/topSites')
+const bookmarkLocationCache = require('../../common/cache/bookmarkLocationCache')
 
 /**
  * Helper to pass message to windows to clear closed frames
@@ -43,6 +45,15 @@ const historyReducer = (state, action, immutableAction) => {
  const temp = state.get('tempClearBrowsingData', Immutable.Map())
  const clearData = defaults ? defaults.merge(temp) : temp
  if (clearData.get('browserHistory')) {
+ let historyList = Immutable.List()
+ historyState.getSites(state).forEach((site) => {
+ const bookmarkKey = bookmarkLocationCache.getCacheKey(state, site.get('location'))
+ if (bookmarkKey.size === 0) {
+ historyList = historyList.push(site)
+ }
+ })
+
+ remove(historyList)
  state = historyState.clearSites(state)
  state = aboutHistoryState.clearHistory(state)
  filtering.clearHistory()

app/browser/reducers/urlBarSuggestionsReducer.js

@@ -7,11 +7,13 @@
 const Immutable = require('immutable')
 const appConstants = require('../../../js/constants/appConstants')
 const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion')
-const {init, add} = require('../../common/lib/siteSuggestions')
+const {init, add, remove} = require('../../common/lib/siteSuggestions')
 const {makeImmutable} = require('../../common/state/immutableUtil')
 const tabState = require('../../common/state/tabState')
 const historyState = require('../../common/state/historyState')
 const bookmarksState = require('../../common/state/bookmarksState')
+const historyUtil = require('../../common/lib/historyUtil')
+const bookmarkLocationCache = require('../../common/cache/bookmarkLocationCache')
 
 const urlBarSuggestionsReducer = (state, action) => {
  switch (action.actionType) {
@@ -26,6 +28,67 @@ const urlBarSuggestionsReducer = (state, action) => {
  add(makeImmutable(action.siteDetail))
  }
  break
+ case appConstants.APP_REMOVE_BOOKMARK:
+ {
+ const bookmarkKey = action.bookmarkKey
+ if (bookmarkKey == null) {
+ break
+ }
+
+ let data = null
+ if (Array.isArray(bookmarkKey)) {
+ data = Immutable.List()
+ bookmarkKey.forEach((key) => {
+ const bookmark = bookmarksState.getBookmark(state, key)
+ const historyKey = historyUtil.getKey(bookmark)
+ if (!historyState.hasSite(state, historyKey)) {
+ data = data.push(bookmark)
+ }
+ })
+ } else {
+ const bookmark = bookmarksState.getBookmark(state, bookmarkKey)
+ const historyKey = historyUtil.getKey(bookmark)
+ if (!historyState.hasSite(state, historyKey)) {
+ data = bookmark
+ }
+ }
+
+ if (data != null) {
+ remove(data)
+ }
+ break
+ }
+
+ case appConstants.APP_REMOVE_HISTORY_SITE:
+ {
+ const historyKey = action.historyKey
+ if (historyKey == null) {
+ break
+ }
+
+ let data = null
+ if (Array.isArray(historyKey)) {
+ data = Immutable.List()
+ historyKey.map((key) => {
+ const site = historyState.getSite(state, key)
+ const bookmarkKey = bookmarkLocationCache.getCacheKey(state, site.get('location'))
+ if (bookmarkKey.size === 0) {
+ data = data.push(site)
+ }
+ })
+ } else {
+ const site = historyState.getSite(state, historyKey)
+ const bookmarkKey = bookmarkLocationCache.getCacheKey(state, site.get('location'))
+ if (bookmarkKey.size === 0) {
+ data = site
+ }
+ }
+
+ if (data != null) {
+ remove(data)
+ }
+ break
+ }
  case appConstants.APP_SET_STATE:
  const bookmarks = bookmarksState.getBookmarks(action.appState)
  const history = historyState.getSites(action.appState)

app/common/lib/siteSuggestions.js

@@ -135,6 +135,13 @@ const add = (data) => {
  engine.add(data.toJS ? data : Immutable.fromJS(data))
 }
 
+const remove = (data) => {
+ if (!initialized) {
+ return
+ }
+ engine.remove(data.toJS ? data : Immutable.fromJS(data))
+}
+
 const query = (input, options = {}) => {
  if (!initialized) {
  return Promise.resolve([])
@@ -163,5 +170,6 @@ module.exports = {
  init,
  add,
  tokenizeInput,
- query
+ query,
+ remove
 }

app/common/state/historyState.js

@@ -27,6 +27,11 @@ const historyState = {
  return state.getIn([STATE_SITES.HISTORY_SITES, key], Immutable.Map())
  },
 
+ hasSite: (state, key) => {
+ state = validateState(state)
+ return state.hasIn([STATE_SITES.HISTORY_SITES, key], Immutable.Map())
+ },
+
  addSite: (state, siteDetail) => {
  let sites = historyState.getSites(state)
  let siteKey = historyUtil.getKey(siteDetail)

js/stores/appStore.js

@@ -178,14 +178,14 @@ const handleAppAction = (action) => {
  // until we have a better way to manage dependencies.
  // tabsReducer must be above dragDropReducer.
  require('../../app/browser/reducers/tabsReducer'),
+ require('../../app/browser/reducers/urlBarSuggestionsReducer'),
  require('../../app/browser/reducers/bookmarksReducer'),
  require('../../app/browser/reducers/bookmarkFoldersReducer'),
  require('../../app/browser/reducers/historyReducer'),
  require('../../app/browser/reducers/pinnedSitesReducer'),
  require('../../app/browser/reducers/windowsReducer'),
  require('../../app/browser/reducers/syncReducer'),
  require('../../app/browser/reducers/clipboardReducer'),
- require('../../app/browser/reducers/urlBarSuggestionsReducer'),
  require('../../app/browser/reducers/passwordManagerReducer'),
  require('../../app/browser/reducers/spellCheckerReducer'),
  require('../../app/browser/reducers/tabMessageBoxReducer'),

test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js

@@ -8,8 +8,21 @@ const appConstants = require('../../../../../js/constants/appConstants')
 require('../../../braveUnit')
 const siteTags = require('../../../../../js/constants/siteTags')
 
-describe('urlBarSuggestionsReducer', function () {
+describe('urlBarSuggestionsReducer unit test', function () {
  let urlBarSuggestionsReducer
+
+ const site1 = {
+ location: 'https://brave.com',
+ type: siteTags.BOOKMARK
+ }
+
+ const initState = Immutable.fromJS({
+ bookmarks: {
+ 'key': site1
+ },
+ historySites: {}
+ })
+
  before(function () {
  mockery.enable({
  warnOnReplace: false,
@@ -19,7 +32,8 @@ describe('urlBarSuggestionsReducer', function () {
 
  this.siteSuggestionsStub = {
  init: sinon.stub(),
- add: sinon.stub()
+ add: sinon.stub(),
+ remove: sinon.stub()
  }
  mockery.registerMock('../../common/lib/siteSuggestions', this.siteSuggestionsStub)
 
@@ -39,22 +53,11 @@ describe('urlBarSuggestionsReducer', function () {
  afterEach(function () {
  this.siteSuggestionsStub.init.reset()
  this.siteSuggestionsStub.add.reset()
+ this.siteSuggestionsStub.remove.reset()
  this.suggestionStub.generateNewSuggestionsList.reset()
  this.suggestionStub.generateNewSearchXHRResults.reset()
  })
 
- const site1 = {
- location: 'https://brave.com',
- type: siteTags.BOOKMARK
- }
-
- const initState = Immutable.fromJS({
- bookmarks: {
- 'key': site1
- },
- historySites: {}
- })
-
  describe('APP_SET_STATE', function () {
  it('inits the suggestions lib with sites', function () {
  urlBarSuggestionsReducer(Immutable.Map(), {actionType: appConstants.APP_SET_STATE, appState: initState})
@@ -74,6 +77,95 @@ describe('urlBarSuggestionsReducer', function () {
  assert.deepEqual(newState, initState)
  })
  })
+ describe('APP_REMOVE_BOOKMARK', function () {
+ let customState
+
+ before(function () {
+ customState = initState
+ .setIn(['bookmarks', '1'], Immutable.fromJS({
+ location: 'https://brianbondy.com',
+ type: siteTags.BOOKMARK
+ }))
+ .setIn(['bookmarks', '2'], Immutable.fromJS({
+ location: 'https://clifton.io',
+ type: siteTags.BOOKMARK
+ }))
+ })
+
+ it('null case', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_BOOKMARK
+ })
+ assert.equal(this.siteSuggestionsStub.remove.notCalled, true)
+ })
+
+ it('bookmark key is list (multiple bookmarks)', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_BOOKMARK,
+ bookmarkKey: [
+ '1',
+ 'key'
+ ]
+ })
+ const argument = Immutable.List()
+ .push(customState.getIn(['bookmarks', '1']))
+ .push(customState.getIn(['bookmarks', 'key']))
+ assert.deepEqual(this.siteSuggestionsStub.remove.args[0][0], argument)
+ })
+
+ it('bookmark key is map (single bookmark)', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_BOOKMARK,
+ bookmarkKey: '1'
+ })
+ const argument = customState.getIn(['bookmarks', '1'])
+ assert.deepEqual(this.siteSuggestionsStub.remove.args[0][0], argument)
+ })
+ })
+
+ describe('APP_REMOVE_HISTORY_SITE', function () {
+ let customState
+
+ before(function () {
+ customState = initState
+ .setIn(['historySites', '1'], Immutable.fromJS({
+ location: 'https://brianbondy.com'
+ }))
+ .setIn(['historySites', '2'], Immutable.fromJS({
+ location: 'https://clifton.io'
+ }))
+ })
+
+ it('null case', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_HISTORY_SITE
+ })
+ assert.equal(this.siteSuggestionsStub.remove.notCalled, true)
+ })
+
+ it('history key is list (multiple history sites)', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_HISTORY_SITE,
+ historyKey: [
+ '1',
+ '2'
+ ]
+ })
+ const argument = Immutable.List()
+ .push(customState.getIn(['historySites', '1']))
+ .push(customState.getIn(['historySites', '2']))
+ assert.deepEqual(this.siteSuggestionsStub.remove.args[0][0], argument)
+ })
+
+ it('history key is map (single history site)', function () {
+ urlBarSuggestionsReducer(customState, {
+ actionType: appConstants.APP_REMOVE_HISTORY_SITE,
+ historyKey: '1'
+ })
+ const argument = customState.getIn(['historySites', '1'])
+ assert.deepEqual(this.siteSuggestionsStub.remove.args[0][0], argument)
+ })
+ })
  describe('APP_ADD_HISTORY_SITE', function () {
  it('adds a site in the suggestions lib', function () {
  const newState = urlBarSuggestionsReducer(initState, {actionType: appConstants.APP_ADD_HISTORY_SITE, siteDetail: site1})