Skip to content
This repository has been archived by the owner on Jul 31, 2020. It is now read-only.

Commit

Permalink
Merge pull request #97 from brave/fix/merge-batched-record-actions
Browse files Browse the repository at this point in the history
Same batch Create + Update records should resolve to Create
  • Loading branch information
ayumi authored Jun 8, 2017
2 parents 0188881 + 865c2e5 commit 9325e6d
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
27 changes: 25 additions & 2 deletions client/recordUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ module.exports.createFromUpdate = (record) => {
}
}

const ACTION_NUMBERS_TO_STRINGS = Object.keys(proto.actions)
.reduce((obj, key) => Object.assign({}, obj, { [proto.actions[key]]: key }), {})

/**
* @param {number} action e.g. 0
* @returns {string} action string e.g. CREATE
*/
const humanAction = (action) => {
const string = ACTION_NUMBERS_TO_STRINGS[action]
if (string) { return string }
if (typeof action.toString === 'function') {
return action.toString()
} else {
return undefined
}
}

const pickFields = (object, fields) => {
return fields.reduce((a, x) => {
if (object.hasOwnProperty(x)) { a[x] = object[x] }
Expand Down Expand Up @@ -169,7 +186,7 @@ const resolveSiteSettingsRecordWithObject = (record, existingObject) => {
module.exports.resolve = (record, existingObject) => {
if (!record) { throw new Error('Missing syncRecord JS object.') }
const nullIgnore = () => {
console.log(`Ignoring ${record.action} of object ${record.objectId}.`)
console.log(`Ignoring ${humanAction(record.action)} of object ${record.objectId}.`)
return null
}
switch (record.action) {
Expand Down Expand Up @@ -202,7 +219,13 @@ const mergeRecord = (record1, record2) => {
if (record1.objectData !== record2.objectData) {
throw new Error('Records with same objectId have mismatched objectData!')
}
return merge(record1, record2)
const newRecord = {}
merge(newRecord, record1)
merge(newRecord, record2)
if (record2.action === proto.actions.UPDATE && record1.action === proto.actions.CREATE) {
newRecord.action = proto.actions.CREATE
}
return newRecord
}

/**
Expand Down
21 changes: 20 additions & 1 deletion test/client/recordUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const Record = (props) => {
}
return Object.assign({}, baseProps, props)
}
const CreateRecord = (props) => {
return Record(Object.assign({action: proto.actions.CREATE}, props))
}
const UpdateRecord = (props) => {
return Record(Object.assign({action: proto.actions.UPDATE}, props))
}
Expand Down Expand Up @@ -366,7 +369,7 @@ test('recordUtil.resolve', (t) => {
})

test('recordUtil.resolveRecords()', (t) => {
t.plan(3)
t.plan(4)

t.test(`${t.name} takes [ [{syncRecord}, {existingObject || null}], ... ] and returns resolved records [{syncRecord}, ...]`, (t) => {
t.plan(1)
Expand Down Expand Up @@ -400,6 +403,22 @@ test('recordUtil.resolveRecords()', (t) => {
t.deepEquals(resolved, [], t.name)
})

t.test(`${t.name} Create + Update of a new object should resolve to a single Create`, (t) => {
t.plan(1)
const expectedRecord = CreateRecord({
objectId: recordBookmark.objectId,
objectData: 'bookmark',
bookmark: Object.assign(
{},
props.bookmark,
{ site: Object.assign({}, siteProps, updateSiteProps) }
)
})
const input = [[recordBookmark, null], [updateBookmark, null]]
const resolved = recordUtil.resolveRecords(input)
t.deepEquals(resolved, [expectedRecord], t.name)
})

t.test(`${t.name} resolves bookmark records with same parent folder`, (t) => {
t.plan(1)
const record = {
Expand Down

0 comments on commit 9325e6d

Please sign in to comment.