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

Commit

Permalink
Fix bookmark folder migration (part of #10595)
Browse files Browse the repository at this point in the history
This is a follow up to:
- 9ad7b4f
- 0dfd283

Auditors: @NejcZdovc
  • Loading branch information
bsclifton committed Dec 21, 2017
1 parent 8ebb80f commit 7caf16a
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 39 deletions.
71 changes: 34 additions & 37 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ module.exports.runPreMigrations = (data) => {

if (data.sites) {
// pinned sites
data.pinnedSites = data.pinnedSites || {}
data.pinnedSites = {}
for (let key of Object.keys(data.sites)) {
const site = data.sites[key]
if (site.tags && site.tags.includes('pinned')) {
Expand Down Expand Up @@ -716,46 +716,43 @@ module.exports.runPreMigrations = (data) => {
let bookmarkOrder = {}

// bookmark folders
if (!data.bookmarkFolders) {
data.bookmarkFolders = {}

for (let key of Object.keys(data.sites)) {
const oldFolder = data.sites[key]
if (oldFolder.tags && oldFolder.tags.includes(siteTags.BOOKMARK_FOLDER)) {
let folder = {}
key = key.toString()

if (oldFolder.customTitle) {
folder.title = oldFolder.customTitle
} else {
folder.title = oldFolder.title
}

if (oldFolder.parentFolderId == null) {
folder.parentFolderId = 0
} else {
folder.parentFolderId = oldFolder.parentFolderId
}
data.bookmarkFolders = {}
for (let key of Object.keys(data.sites)) {
const oldFolder = data.sites[key]
if (oldFolder.tags && oldFolder.tags.includes(siteTags.BOOKMARK_FOLDER)) {
let folder = {}
key = key.toString()

folder.folderId = oldFolder.folderId
folder.partitionNumber = oldFolder.partitionNumber
folder.objectId = oldFolder.objectId
folder.type = siteTags.BOOKMARK_FOLDER
folder.key = key
data.bookmarkFolders[key] = folder
if (oldFolder.customTitle) {
folder.title = oldFolder.customTitle
} else {
folder.title = oldFolder.title
}

// bookmark order
const id = folder.parentFolderId.toString()
if (!bookmarkOrder[id]) {
bookmarkOrder[id] = []
}
if (oldFolder.parentFolderId == null) {
folder.parentFolderId = 0
} else {
folder.parentFolderId = oldFolder.parentFolderId
}

bookmarkOrder[id].push({
key: key,
order: oldFolder.order,
type: siteTags.BOOKMARK_FOLDER
})
folder.folderId = oldFolder.folderId
folder.partitionNumber = oldFolder.partitionNumber
folder.objectId = oldFolder.objectId
folder.type = siteTags.BOOKMARK_FOLDER
folder.key = key
data.bookmarkFolders[key] = folder

// bookmark order
const id = folder.parentFolderId.toString()
if (!bookmarkOrder[id]) {
bookmarkOrder[id] = []
}

bookmarkOrder[id].push({
key: key,
order: oldFolder.order,
type: siteTags.BOOKMARK_FOLDER
})
}
}

Expand Down
92 changes: 90 additions & 2 deletions test/unit/app/sessionStoreTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const Immutable = require('immutable')
const settings = require('../../../js/constants/settings')
const {makeImmutable} = require('../../../app/common/state/immutableUtil')
const downloadStates = require('../../../js/constants/downloadStates')
const siteTags = require('../../../js/constants/siteTags')
const compareVersions = require('compare-versions')

require('../braveUnit')
Expand Down Expand Up @@ -1284,8 +1285,48 @@ describe('sessionStore unit tests', function () {
'pinned'
],
'themeColor': 'rgb(255, 255, 255)'
},
'30': {
originalSeed: [
36, 34, 85, 153, 174, 102, 60, 93, 35, 187, 5, 231, 20, 87, 57, 216, 139, 240, 127, 58, 38, 30, 57, 182, 132, 201, 245, 70, 162, 164, 226, 108
],
lastAccessedTime: 1460058786000,
customTitle: 'interesting',
folderId: 30,
order: 7045,
parentFolderId: 1,
title: 'articles',
tags: [
'bookmark-folder'
],
objectId: [
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16
]
},
'31': {
originalSeed: [
22, 34, 85, 153, 174, 102, 60, 93, 35, 187, 5, 231, 20, 87, 57, 216, 139, 240, 127, 58, 38, 30, 57, 182, 132, 201, 245, 70, 162, 164, 226, 108
],
lastAccessedTime: 1460058786000,
folderId: 31,
order: 7046,
title: 'articles',
tags: [
'bookmark-folder'
],
objectId: [
16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1
]
}
},
// BEGIN - these values should never exist for actual users
pinnedSites: {
'https://should-be-cleared-on-migrate.com|0|0': {}
},
bookmarkFolders: {
'12': {}
},
// END - these values should never exist for actual users
'about': {
'newtab': {
'gridLayoutSize': 'small',
Expand Down Expand Up @@ -1472,6 +1513,9 @@ describe('sessionStore unit tests', function () {
it('copies themeColor', function () {
assert.equal(oldValue.get('themeColor'), newValue.themeColor)
})
it('destroys any existing values in `data.pinnedSites`', function () {
assert.equal(runPreMigrations.pinnedSites['https://should-be-cleared-on-migrate.com|0|0'], undefined)
})
})

describe('default sites', function () {
Expand All @@ -1488,8 +1532,52 @@ describe('sessionStore unit tests', function () {
})
})

describe('bookmark order', function () {
// TODO:
describe('bookmark folders', function () {
let oldValue
let newValue

before(function () {
oldValue = data.getIn(['sites', '30'])
newValue = runPreMigrations.bookmarkFolders['30']
})

describe('with title', function () {
it('copies from customTitle if present', function () {
assert.equal(oldValue.get('customTitle'), newValue.title)
})
it('copies from title when customTitle is not present', function () {
const tempOldValue = data.getIn(['sites', '31'])
const tempNewValue = runPreMigrations.bookmarkFolders['31']
assert.equal(tempOldValue.get('title'), tempNewValue.title)
})
})
describe('with parentFolderId', function () {
it('sets to 0 if null', function () {
const tempNewValue = runPreMigrations.bookmarkFolders['31']
assert.equal(tempNewValue.parentFolderId, 0)
})
it('copies parentFolderId if not null', function () {
assert.equal(oldValue.get('parentFolderId'), newValue.parentFolderId)
})
})
it('copies folderId', function () {
assert.equal(oldValue.get('folderId'), newValue.folderId)
})
it('copies partitionNumber', function () {
assert.equal(oldValue.get('partitionNumber'), newValue.partitionNumber)
})
it('copies objectId', function () {
assert.deepEqual(oldValue.get('objectId').toJS(), newValue.objectId)
})
it('sets type to bookmark folder', function () {
assert.equal(newValue.type, siteTags.BOOKMARK_FOLDER)
})
it('sets key', function () {
assert.equal(newValue.key, 30)
})
it('destroys any existing values in `data.bookmarkFolders`', function () {
assert.equal(runPreMigrations.bookmarkFolders['12'], undefined)
})
})

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

0 comments on commit 7caf16a

Please sign in to comment.