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

Commit

Permalink
fix sync bookmarks hierarchy
Browse files Browse the repository at this point in the history
fix #7971

Test Plan:
1. open pyramid 0, enable sync
2. bookmark bing.com
3. create a folder
4. move the bing bookmark into the folder
5. open pyramid 1, sync it to pyramid 0
6. a folder should appear in pyramid 1 with bing inside the folder
  • Loading branch information
diracdeltas committed Apr 5, 2017
1 parent 7157e82 commit df0080e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
27 changes: 21 additions & 6 deletions js/state/syncUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ let folderIdMap = new Immutable.Map()
* Converts sync records into a form that can be consumed by AppStore.
* @param {Object} record
* @param {Immutable.Map} appState
* @param {Immutable.List=} records - batch of records possibly not yet applied
*/
module.exports.getSiteDataFromRecord = (record, appState) => {
module.exports.getSiteDataFromRecord = (record, appState, records) => {
const objectId = new Immutable.List(record.objectId)
const category = CATEGORY_MAP[record.objectData].categoryName
let existingObjectData
Expand Down Expand Up @@ -96,7 +97,7 @@ module.exports.getSiteDataFromRecord = (record, appState) => {
const parentFolderObjectId = siteProps.parentFolderObjectId
if (parentFolderObjectId && parentFolderObjectId.length > 0) {
siteProps.parentFolderId =
getFolderIdByObjectId(new Immutable.List(parentFolderObjectId), appState)
getFolderIdByObjectId(new Immutable.List(parentFolderObjectId), appState, records)
}
}
const siteDetail = new Immutable.Map(pickFields(siteProps, SITE_FIELDS))
Expand Down Expand Up @@ -341,16 +342,30 @@ module.exports.getObjectById = (objectId, category, appState) => {
* Given an bookmark folder objectId, find the folder and return its folderId.
* @param {Immutable.List} objectId
* @param {Immutable.Map=} appState
* @param {Immutable.List=} records
* @returns {number|undefined}
*/
const getFolderIdByObjectId = (objectId, appState) => {
const getFolderIdByObjectId = (objectId, appState, records) => {
if (folderIdMap.has(objectId)) {
return folderIdMap.get(objectId)
}
let folderId
const entry = module.exports.getObjectById(objectId, 'BOOKMARKS', appState)
if (!entry) { return undefined }
const folderId = entry[1].get('folderId')
folderIdMap = folderIdMap.set(objectId, folderId)
if (entry) {
folderId = entry[1].get('folderId')
} else if (records) {
// Look for a folder record with a matching object ID in this record batch
const matchingFolder = records.find((record) => {
record = Immutable.fromJS(record)
return record && objectId.equals(record.get('objectId')) && typeof record.getIn(['bookmark', 'site', 'folderId']) === 'number'
})
if (matchingFolder) {
folderId = matchingFolder.bookmark.site.folderId
}
}
if (folderId) {
folderIdMap = folderIdMap.set(objectId, folderId)
}
return folderId
}

Expand Down
20 changes: 13 additions & 7 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const ExtensionConstants = require('../../app/common/constants/extensionConstant
const AppDispatcher = require('../dispatcher/appDispatcher')
const appConfig = require('../constants/appConfig')
const settings = require('../constants/settings')
const siteTags = require('../constants/siteTags')
const writeActions = require('../constants/sync/proto').actions
const siteUtil = require('../state/siteUtil')
const syncUtil = require('../state/syncUtil')
Expand Down Expand Up @@ -438,16 +437,23 @@ const handleAppAction = (action) => {
nativeImage.copyDataURL(action.dataURL, action.html, action.text)
break
case appConstants.APP_APPLY_SITE_RECORDS:
let nextFolderId = siteUtil.getNextFolderId(appState.get('sites'))
// Ensure that all folders are assigned folderIds
action.records.forEach((record, i) => {
if (record.action !== writeActions.DELETE &&
record.bookmark && record.bookmark.isFolder &&
record.bookmark.site &&
typeof record.bookmark.site.folderId !== 'number') {
record.bookmark.site.folderId = nextFolderId
action.records.set(i, record)
nextFolderId = nextFolderId + 1
}
})
action.records.forEach((record) => {
const siteData = syncUtil.getSiteDataFromRecord(record, appState)
const siteData = syncUtil.getSiteDataFromRecord(record, appState, action.records)
const tag = siteData.tag
let siteDetail = siteData.siteDetail
const sites = appState.get('sites')
if (record.action !== writeActions.DELETE &&
tag === siteTags.BOOKMARK_FOLDER &&
!siteDetail.get('folderId')) {
siteDetail = siteDetail.set('folderId', siteUtil.getNextFolderId(sites))
}
switch (record.action) {
case writeActions.CREATE:
appState = appState.set('sites',
Expand Down

0 comments on commit df0080e

Please sign in to comment.