From 707fcf25caef4a7bba4791d53c53e2ed53e379ca Mon Sep 17 00:00:00 2001 From: yan Date: Wed, 12 Apr 2017 00:42:30 +0000 Subject: [PATCH] fix applySiteSettingRecord error, remove unneeded setObjectId calls fix #8241 applySiteSettingRecord should not need to check if there is a diff between the existing record and the new record; this is the job of reducers. Auditors: @ayumi Test Plan: 1. 'Syncing' tests should pass 2. Set isProduction to true in js/constants/appConfig 3. Sync to the profile starting with "duality" (sent via slack DM) 4. You should not see any console errors 5. Go http://expectnothing.com/ and verify that shields are down --- js/state/syncUtil.js | 12 ++---------- js/stores/appStore.js | 30 +++++++++++------------------- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/js/state/syncUtil.js b/js/state/syncUtil.js index 38ea4d8d511..9bbbb5b3f67 100644 --- a/js/state/syncUtil.js +++ b/js/state/syncUtil.js @@ -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.') @@ -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 @@ -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]) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index f3141f2e6cc..95cbf9b4d61 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -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) @@ -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) @@ -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}) @@ -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) appState = appState.set('siteSettings', newSiteSettings) } }) @@ -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