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

Clearing history using modal now clears bookmark timestamps #3501

Merged
merged 1 commit into from
Aug 28, 2016
Merged

Clearing history using modal now clears bookmark timestamps #3501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion js/state/siteUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,13 @@ module.exports.filterSitesRelativeTo = function (sites, relSite) {
* @param sites The application state's Immutable sites list.
*/
module.exports.clearSitesWithoutTags = function (sites) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls update the name of this function and comment because it's a bit misleading. I'll merge this but just commit something directly with an auditor: @bbondy message.
It filters out sites without tags, but it also clears last accessed time on the sites that do have tags.
I'd maybe call it clearHistory and then describe both actions in the comment.

return sites.filter((site) => site.get('tags') && site.get('tags').size > 0)
let clearedSites = sites.filter((site) => site.get('tags') && site.get('tags').size > 0)
clearedSites.forEach((site, index) => {
if (site.get('lastAccessedTime')) {
clearedSites = clearedSites.setIn([index, 'lastAccessedTime'], null)
}
})
return clearedSites
}

/**
Expand Down
34 changes: 26 additions & 8 deletions test/unit/state/siteUtilTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,33 @@ describe('siteUtil', function () {

describe('clearSitesWithoutTags', function () {
it('does not remove sites which have a valid `tags` property', function () {
const sites = [
Immutable.fromJS({
tags: [siteTags.BOOKMARK_FOLDER]
}),
Immutable.fromJS({
tags: [siteTags.BOOKMARK]
})]
const sites = Immutable.fromJS([
{ tags: [siteTags.BOOKMARK_FOLDER] },
{ tags: [siteTags.BOOKMARK] }
])
const processedSites = siteUtil.clearSitesWithoutTags(sites)
assert.deepEqual(sites, processedSites)
assert.deepEqual(processedSites.toJS(), sites.toJS())
})
it('sets the lastAccessedTime for all entries to null', function () {
const sites = Immutable.fromJS([
{
location: 'location1',
tags: [],
lastAccessedTime: 123
},
{
location: 'location2',
tags: [siteTags.BOOKMARK],
lastAccessedTime: 123
}
])
const expectedSites = Immutable.fromJS([{
location: 'location2',
tags: [siteTags.BOOKMARK],
lastAccessedTime: null
}])
const processedSites = siteUtil.clearSitesWithoutTags(sites)
assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})
})

Expand Down