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

Commit

Permalink
fix applySiteSettingRecord error, remove unneeded setObjectId calls
Browse files Browse the repository at this point in the history
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
  • Loading branch information
diracdeltas authored and bsclifton committed Apr 12, 2017
1 parent 0fb7fb1 commit 707fcf2
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 29 deletions.
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)
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

0 comments on commit 707fcf2

Please sign in to comment.