Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Allow incrementing default sites count
Browse files Browse the repository at this point in the history
Fix #11043

The problem is that top sites caching keeps the smallest count, so that it doesn't need to consider items that is smaller than that count. But for top sites it is 1, and when added to history it is 0

This change is only for 0.19.x and 0.20.x

Auditors: @cezaraugusto
  • Loading branch information
bbondy committed Sep 23, 2017
1 parent 8e370de commit 12f516d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
4 changes: 1 addition & 3 deletions js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,9 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId, order) =>
if (newSiteDetail.get('themeColor') || (oldSiteDetail && oldSiteDetail.get('themeColor'))) {
site = site.set('themeColor', newSiteDetail.get('themeColor') || oldSiteDetail.get('themeColor'))
}
if (site.get('tags').size === 0) {
// Increment the visit count for history items
if (site.get('tags').size === 0 || (site.get('tags').size === 1 && site.getIn(['tags', 0]) === siteTags.DEFAULT)) {
site = site.set('count', ((oldSiteDetail || site).get('count') || 0) + 1)
}

return site
}

Expand Down
10 changes: 10 additions & 0 deletions test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,16 @@ describe('siteUtil', function () {
assert.equal(state.getIn(['sites', oldSiteKey, 'skipSync']), true)
})
})
it('increments count for default default sites', function () {
const site = Immutable.fromJS({
location: 'https://brave.com',
tags: [siteTags.BOOKMARK_FOLDER]
})
let newState = siteUtil.addSite(Immutable.fromJS({sites: {}}), site, siteTags.DEFAULT)
assert.equal(newState.getIn(['sites', 'https://brave.com|0|0', 'count']), 1)
newState = siteUtil.addSite(newState, site, siteTags.DEFAULT)
assert.equal(newState.getIn(['sites', 'https://brave.com|0|0', 'count']), 2)
})
})

describe('removeSite', function () {
Expand Down

0 comments on commit 12f516d

Please sign in to comment.