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

Commit

Permalink
Pulled code from about:history into a new historyUtil module
Browse files Browse the repository at this point in the history
Fixes #5353

This has regressed at least once before; we need a way besides web driver to test the code.
(usually, problem is immutable vs mutable; ex: ".get method not found", etc)

Auditors: @bbondy

Test Plan:
1. Launch Brave and visit about:history
2. Double click any entry
3. Enjoy :)
  • Loading branch information
bsclifton authored and bbondy committed Nov 4, 2016
1 parent 30a9674 commit 8d13095
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 47 deletions.
56 changes: 56 additions & 0 deletions app/common/lib/historyUtil.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/* 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/. */

'use strict'
const Immutable = require('immutable')

const getDayString = (entry, locale) => {
const lastAccessedTime = entry.get('lastAccessedTime')
return lastAccessedTime
? new Date(lastAccessedTime).toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' })
: ''
}

module.exports.groupEntriesByDay = (history, locale) => {
const reduced = history.reduce((previousValue, currentValue, currentIndex, array) => {
const result = currentIndex === 1 ? [] : previousValue
if (currentIndex === 1) {
const firstDate = getDayString(currentValue, locale)
result.push({date: firstDate, entries: [previousValue]})
}
const date = getDayString(currentValue, locale)
const dateIndex = result.findIndex((entryByDate) => entryByDate.date === date)
if (dateIndex !== -1) {
result[dateIndex].entries.push(currentValue)
} else {
result.push({date: date, entries: [currentValue]})
}
return result
})
if (reduced) {
return Immutable.fromJS(
Array.isArray(reduced)
? reduced
: [{date: getDayString(reduced, locale), entries: [reduced]}]
)
}
return Immutable.fromJS([])
}

/**
* Return an array with ALL entries.
* Format is expected to be array containing one array per day.
*/
module.exports.totalEntries = (entriesByDay) => {
let result = new Immutable.List()
entriesByDay.forEach((entry) => {
result = result.push(entry.get('entries'))
})
return result
}

module.exports.sortTimeDescending = (left, right) => {
if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1
if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1
return 0
}
60 changes: 13 additions & 47 deletions js/about/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const getSetting = require('../settings').getSetting
const SortableTable = require('../components/sortableTable')
const Button = require('../components/button')
const siteUtil = require('../state/siteUtil')
const {makeImmutable} = require('../../app/common/state/immutableUtil')
const historyUtil = require('../../app/common/lib/historyUtil')

const ipc = window.chrome.ipc

Expand Down Expand Up @@ -57,6 +59,7 @@ class HistoryTimeCell extends ImmutableComponent {

class HistoryDay extends ImmutableComponent {
navigate (entry) {
entry = makeImmutable(entry)
aboutActions.newFrame({
location: entry.get('location'),
partitionNumber: entry.get('partitionNumber')
Expand All @@ -79,7 +82,7 @@ class HistoryDay extends ImmutableComponent {
urlutils.getHostname(entry.get('location'), true)
])}
rowObjects={this.props.entries}
totalRowObjects={this.props.totalEntries}
totalRowObjects={this.props.totalEntries.toJS()}
tableID={this.props.tableID}
columnClassNames={['time', 'title', 'domain']}
addHoverClass
Expand All @@ -93,53 +96,19 @@ class HistoryDay extends ImmutableComponent {
}

class GroupedHistoryList extends ImmutableComponent {
getDayString (locale, item) {
const lastAccessedTime = item.get('lastAccessedTime')
return lastAccessedTime
? new Date(lastAccessedTime).toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' })
: ''
}
groupEntriesByDay (locale) {
const reduced = this.props.history.reduce((previousValue, currentValue, currentIndex, array) => {
const result = currentIndex === 1 ? [] : previousValue
if (currentIndex === 1) {
const firstDate = this.getDayString(locale, currentValue)
result.push({date: firstDate, entries: [previousValue]})
}
const date = this.getDayString(locale, currentValue)
const dateIndex = result.findIndex((entryByDate) => entryByDate.date === date)
if (dateIndex !== -1) {
result[dateIndex].entries.push(currentValue)
} else {
result.push({date: date, entries: [currentValue]})
}
return result
})
if (reduced) {
return Array.isArray(reduced)
? reduced
: [{date: this.getDayString(locale, reduced), entries: [reduced]}]
}
return []
}
totalEntries (entriesByDay) {
const result = []
entriesByDay.forEach((entry) => {
result.push(entry.entries)
})
return result
}
render () {
const defaultLanguage = this.props.languageCodes.find((lang) => lang.includes(navigator.language)) || 'en-US'
const userLanguage = getSetting(settings.LANGUAGE, this.props.settings)
const entriesByDay = this.groupEntriesByDay(userLanguage || defaultLanguage)
const userLanguage = getSetting(settings.LANGUAGE, this.props.settings) || defaultLanguage
const entriesByDay = historyUtil.groupEntriesByDay(this.props.history, userLanguage)
const totalEntries = historyUtil.totalEntries(entriesByDay)
let index = 0
return <list className='historyList'>
{
entriesByDay.map((groupedEntry) =>
<HistoryDay date={groupedEntry.date}
entries={groupedEntry.entries}
totalEntries={this.totalEntries(entriesByDay)}
<HistoryDay
date={groupedEntry.get('date')}
entries={groupedEntry.get('entries')}
totalEntries={totalEntries}
tableID={index++}
stateOwner={this.props.stateOwner}
/>)
Expand Down Expand Up @@ -189,11 +158,8 @@ class AboutHistory extends React.Component {
}
get historyDescendingOrder () {
return this.state.history.filter((site) => siteUtil.isHistoryEntry(site))
.sort((left, right) => {
if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1
if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1
return 0
}).slice(-500)
.sort(historyUtil.sortTimeDescending)
.slice(-500)
}
clearBrowsingDataNow () {
aboutActions.clearBrowsingDataNow({browserHistory: true})
Expand Down
92 changes: 92 additions & 0 deletions test/unit/lib/historyUtilTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/* global describe, it */
const assert = require('assert')
const Immutable = require('immutable')
const historyUtil = require('../../../app/common/lib/historyUtil')
require('../braveUnit')

describe('historyUtil', function () {
const historyDayOne = Immutable.fromJS({
lastAccessedTime: 1477944718876,
location: 'https://brave.com/page1',
title: 'sample 1',
tags: []
})
const historyDayTwo = Immutable.fromJS({
lastAccessedTime: 1478079042097,
location: 'https://brave.com/page2',
title: 'sample 2',
tags: []
})
const historyDayThree = Immutable.fromJS([{
lastAccessedTime: 1478157051910,
location: 'https://brave.com/page3',
title: 'sample 3',
tags: []
}, {
lastAccessedTime: 1478157051921,
location: 'https://brave.com/page4',
title: 'sample 4',
tags: []
}, {
lastAccessedTime: 1478157051932,
location: 'https://brave.com/page5',
title: 'sample 5',
tags: []
}])
const historyMultipleDays = historyDayThree.push(historyDayTwo, historyDayOne)

describe('groupEntriesByDay', function () {
it('returns the result as an Immutable.List', function () {
const result = historyUtil.groupEntriesByDay(historyDayThree)
assert.equal(Immutable.List.isList(result), true)
})
it('has one object for each day', function () {
const result = historyUtil.groupEntriesByDay(historyDayThree)
assert.equal(result.size, 1)
})
it('can handle multiple days', function () {
const result = historyUtil.groupEntriesByDay(historyMultipleDays)
assert.equal(result.size, 3)
})
describe('with the object representing a day', function () {
it('formats a readable `date` field', function () {
const result = historyUtil.groupEntriesByDay(historyDayThree, 'en-US')
assert.equal(result.getIn([0, 'date']), 'Thursday, November 3, 2016')
})
it('has an entry for each history item', function () {
const result = historyUtil.groupEntriesByDay(historyDayThree, 'en-US')
const entries = result.getIn([0, 'entries'])
assert.equal(entries && entries.size, historyDayThree.size)
})
})
})

describe('totalEntries', function () {
it('returns the result as an Immutable.List', function () {
const result1 = historyUtil.groupEntriesByDay(historyMultipleDays)
const result2 = historyUtil.totalEntries(result1)
assert.equal(Immutable.List.isList(result2), true)
})
it('combines entries for multiple days into one response', function () {
const result1 = historyUtil.groupEntriesByDay(historyMultipleDays)
const result2 = historyUtil.totalEntries(result1)
const expectedResult = [
historyDayThree.toJS(),
[historyDayTwo.toJS()],
[historyDayOne.toJS()]
]
assert.deepEqual(result2.toJS(), expectedResult)
})
})

describe('sortTimeDescending', function () {
it('sorts the items by date/time DESC', function () {
const result = historyMultipleDays.sort(historyUtil.sortTimeDescending)
const expectedResult = historyDayThree.toJS().reverse()
expectedResult.push(historyDayTwo.toJS())
expectedResult.push(historyDayOne.toJS())

assert.deepEqual(result.toJS(), expectedResult)
})
})
})

0 comments on commit 8d13095

Please sign in to comment.