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

fix applySiteSettingRecord error, remove unneeded setObjectId calls #8242

Merged
merged 1 commit into from
Apr 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 2 additions & 10 deletions js/state/syncUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ const applySiteSettingRecord = (record) => {
}
}
const appActions = require('../actions/appActions')
const objectId = new Immutable.List(record.objectId)
const category = CATEGORY_MAP[record.objectData].categoryName
const hostPattern = record.siteSetting.hostPattern
if (!hostPattern) {
throw new Error('siteSetting.hostPattern is required.')
Expand All @@ -147,16 +145,8 @@ const applySiteSettingRecord = (record) => {
switch (record.action) {
case writeActions.CREATE:
case writeActions.UPDATE:
// Set the objectId if needed so we can access the existing object
let existingObject = module.exports.getObjectById(objectId, category)
if (!existingObject) {
appActions.changeSiteSetting(hostPattern, 'objectId', objectId, false, true)
existingObject = module.exports.getObjectById(objectId, category)
}
const existingObjectData = existingObject[1]
applySetting = (key, value) => {
const applyValue = getValue(key, value)
if (existingObjectData.get(key) === applyValue) { return }
appActions.changeSiteSetting(hostPattern, key, applyValue, false, true)
}
break
Expand All @@ -167,6 +157,8 @@ const applySiteSettingRecord = (record) => {
break
}

// Set the record objectId if it doesn't exist already
appActions.changeSiteSetting(hostPattern, 'objectId', new Immutable.List(record.objectId), false, true)
for (let key in record.siteSetting) {
if (key === 'hostPattern') { continue }
applySetting(key, record.siteSetting[key])
Expand Down
30 changes: 11 additions & 19 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,13 +575,11 @@ const handleAppAction = (action) => {
{
let propertyName = action.temporary ? 'temporarySiteSettings' : 'siteSettings'
let newSiteSettings = siteSettings.mergeSiteSetting(appState.get(propertyName), action.hostPattern, action.key, action.value)
if (!action.temporary) {
let syncObject = siteUtil.setObjectId(newSiteSettings.get(action.hostPattern))
if (!action.skipSync) {
const objectId = syncObject.get('objectId')
const item = new Immutable.Map({objectId, [action.key]: action.value})
syncActions.updateSiteSetting(action.hostPattern, item)
}
if (!action.temporary && !action.skipSync) {
const syncObject = siteUtil.setObjectId(newSiteSettings.get(action.hostPattern))
const objectId = syncObject.get('objectId')
const item = new Immutable.Map({objectId, [action.key]: action.value})
syncActions.updateSiteSetting(action.hostPattern, item)
newSiteSettings = newSiteSettings.set(action.hostPattern, syncObject)
}
appState = appState.set(propertyName, newSiteSettings)
Expand All @@ -592,13 +590,11 @@ const handleAppAction = (action) => {
let propertyName = action.temporary ? 'temporarySiteSettings' : 'siteSettings'
let newSiteSettings = siteSettings.removeSiteSetting(appState.get(propertyName),
action.hostPattern, action.key)
if (!action.temporary) {
let syncObject = siteUtil.setObjectId(newSiteSettings.get(action.hostPattern))
if (!action.skipSync) {
const objectId = syncObject.get('objectId')
const item = new Immutable.Map({objectId, [action.key]: null})
syncActions.removeSiteSetting(action.hostPattern, item)
}
if (!action.temporary && !action.skipSync) {
const syncObject = siteUtil.setObjectId(newSiteSettings.get(action.hostPattern))
const objectId = syncObject.get('objectId')
const item = new Immutable.Map({objectId, [action.key]: null})
syncActions.removeSiteSetting(action.hostPattern, item)
newSiteSettings = newSiteSettings.set(action.hostPattern, syncObject)
}
appState = appState.set(propertyName, newSiteSettings)
Expand All @@ -610,7 +606,7 @@ const handleAppAction = (action) => {
let newSiteSettings = new Immutable.Map()
appState.get(propertyName).map((entry, hostPattern) => {
let newEntry = entry.delete(action.key)
if (!action.skipSync) {
if (!action.temporary && !action.skipSync) {
newEntry = siteUtil.setObjectId(newEntry)
const objectId = newEntry.get('objectId')
const item = new Immutable.Map({objectId, [action.key]: null})
Expand Down Expand Up @@ -918,8 +914,6 @@ const handleAppAction = (action) => {

if (result === undefined) {
let newSiteSettings = siteSettings.mergeSiteSetting(appState.get('siteSettings'), pattern, 'ledgerPayments', true)
const syncObject = siteUtil.setObjectId(newSiteSettings.get(pattern))
newSiteSettings = newSiteSettings.set(pattern, syncObject)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: can newSiteSettings be const?

Copy link
Member

Choose a reason for hiding this comment

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

Pls do audited commit if changing, I'll merge this in the meantime. Thanks.

appState = appState.set('siteSettings', newSiteSettings)
}
})
Expand All @@ -928,8 +922,6 @@ const handleAppAction = (action) => {
Object.keys(action.publishers).map((item) => {
const pattern = `https?://${item}`
let newSiteSettings = siteSettings.mergeSiteSetting(appState.get('siteSettings'), pattern, 'ledgerPinPercentage', action.publishers[item].pinPercentage)
const syncObject = siteUtil.setObjectId(newSiteSettings.get(pattern))
newSiteSettings = newSiteSettings.set(pattern, syncObject)
appState = appState.set('siteSettings', newSiteSettings)
})
break
Expand Down