Skip to content

Commit

Permalink
Tighten Campaign level denormalization and saving
Browse files Browse the repository at this point in the history
* Before saving the campaign, load all missing levels and achievements so rewards are properly set
* Have individually loaded levels save and report changes properly
* Do not propagate campaignIndex from non-course campaigns to levels
* Do not set level properties to 'undefined', making it seem like they have local changes
* Update campaign indexes before saving the campaign
* Have fetching achievements by campaign handle odd level original values
* When a level is edited, propagate those changes to campaigns immediately
  • Loading branch information
sderickson committed Oct 31, 2016
1 parent 9abf9dc commit 43bf457
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 14 deletions.
6 changes: 6 additions & 0 deletions app/collections/Achievements.coffee
Expand Up @@ -9,3 +9,9 @@ module.exports = class AchievementCollection extends CocoCollection
options = _.extend({data: {}}, options)
options.data.related = levelOriginal
@fetch(options)

fetchForCampaign: (campaignHandle, options) ->
options = _.extend({data: {}}, options)
options.url = "/db/campaign/#{campaignHandle}/achievements"
@fetch(options)

4 changes: 2 additions & 2 deletions app/schemas/models/campaign.schema.coffee
Expand Up @@ -84,7 +84,7 @@ _.extend CampaignSchema.properties, {
}}
}

denormalizedLevelProperties = [
CampaignSchema.denormalizedLevelProperties = [
'name'
'description'
'i18n'
Expand Down Expand Up @@ -125,7 +125,7 @@ denormalizedLevelProperties = [
'scoreTypes'
]
hiddenLevelProperties = ['name', 'description', 'i18n', 'replayable', 'slug', 'original', 'primerLanguage', 'shareable', 'concepts', 'scoreTypes']
for prop in denormalizedLevelProperties
for prop in CampaignSchema.denormalizedLevelProperties
CampaignSchema.properties.levels.additionalProperties.properties[prop] = _.cloneDeep(LevelSchema.properties[prop])
for hiddenProp in hiddenLevelProperties
CampaignSchema.properties.levels.additionalProperties.properties[hiddenProp].format = 'hidden'
Expand Down
47 changes: 36 additions & 11 deletions app/views/editor/campaign/CampaignEditorView.coffee
Expand Up @@ -61,7 +61,6 @@ module.exports = class CampaignEditorView extends RootView
@listenToOnce @achievements, 'sync', @onFundamentalLoaded

onLeaveMessage: ->
@propagateCampaignIndexes()
for model in @toSave.models
diff = model.getDelta()
if _.size(diff)
Expand All @@ -84,20 +83,35 @@ module.exports = class CampaignEditorView extends RootView
onFundamentalLoaded: ->
# Load any levels which haven't been denormalized into our campaign.
return unless @campaign.loaded and @levels.loaded and @achievements.loaded
@loadMissingLevelsAndRelatedModels()

loadMissingLevelsAndRelatedModels: ->
promises = []
for level in _.values(@campaign.get('levels'))
continue if model = @levels.findWhere(original: level.original)
model = new Level({})
model.setProjection Campaign.denormalizedLevelProperties
model.setURL("/db/level/#{level.original}/version")
@levels.add @supermodel.loadModel(model).model
levelResource = @supermodel.loadModel(model)
@levels.add levelResource.model
# Handle SuperModel's caching, and make sure loaded levels save and notice changes properly
if levelResource.jqxhr
levelResource.model.once('sync', ->
@setURL("/db/level/#{@id}")
@markToRevert()
)
promises.push(levelResource.jqxhr)
achievements = new RelatedAchievementsCollection level.original
achievements.setProjection achievementProject
@supermodel.loadCollection achievements, 'achievements'
@listenToOnce achievements, 'sync', ->
@achievements.add(achievements.models)
achievementsResource = @supermodel.loadCollection(achievements)
promises.push(achievementsResource.jqxhr)
@listenToOnce achievements, 'sync', (achievementsLoaded) ->
@achievements.add(achievementsLoaded.models)
return Promise.resolve($.when(promises...))

onLoaded: ->
@updateCampaignLevels()
@campaignView.render()
super()

updateCampaignLevels: ->
Expand All @@ -107,7 +121,7 @@ module.exports = class CampaignEditorView extends RootView
levelOriginal = level.get('original')
campaignLevel = campaignLevels[levelOriginal]
continue if not campaignLevel
$.extend campaignLevel, _.omit(level.attributes, '_id')
$.extend campaignLevel, _.pick(level.attributes, Campaign.denormalizedLevelProperties)
# TODO: better way for it to remember when we intend to not specifically require/restrict gear any more
delete campaignLevel.requiredGear if not level.attributes.requiredGear
delete campaignLevel.restrictedGear if not level.attributes.restrictedGear
Expand All @@ -124,7 +138,12 @@ module.exports = class CampaignEditorView extends RootView
for level in _.values campaignLevels
continue if /test/.test @campaign.get('slug') # Don't overwrite level stuff for testing Campaigns
model = @levels.findWhere {original: level.original}
model.set key, level[key] for key in Campaign.denormalizedLevelProperties
# do not propagate campaignIndex for non-course campaigns
propsToPropagate = Campaign.denormalizedLevelProperties
if @campaign.get('type') isnt 'course'
propsToPropagate = _.without(propsToPropagate, 'campaignIndex')
for key in propsToPropagate
model.set key, level[key] if model.get(key) isnt level[key]
@toSave.add model if model.hasLocalChanges()

formatRewards: (level) ->
Expand Down Expand Up @@ -184,10 +203,16 @@ module.exports = class CampaignEditorView extends RootView
@openCampaignLevelView @supermodel.getModelByOriginal Level, original
break

onClickSaveButton: ->
@propagateCampaignIndexes()
@toSave.set @toSave.filter (m) -> m.hasLocalChanges()
@openModalView new SaveCampaignModal({}, @toSave)
onClickSaveButton: (e) ->
return if @openingModal
@openingModal = true
@loadMissingLevelsAndRelatedModels().then(=>
@openingModal = false
@propagateCampaignIndexes()
@updateCampaignLevels()
@toSave.set @toSave.filter (m) -> m.hasLocalChanges()
@openModalView new SaveCampaignModal({}, @toSave)
)

afterRender: ->
super()
Expand Down
4 changes: 3 additions & 1 deletion server/middleware/campaigns.coffee
Expand Up @@ -50,7 +50,9 @@ module.exports =
project = parse.getProjectFromReq(req)
fetches = []
for level in _.values(levels)
fetches.push Achievement.find({ related: level.original }).select(project)
# Sometimes, level.original is some sort of object, but not an object id. Add '' to get this to work.
# This strange behavior shows itself when making changes in the CampaignEditorView.
fetches.push Achievement.find({ related: level.original + '' }).select(project)
achievementses = yield fetches
achievements = _.flatten(achievementses)
res.status(200).send((achievement.toObject({req: req}) for achievement in achievements))
Expand Down
21 changes: 21 additions & 0 deletions server/models/Level.coffee
Expand Up @@ -45,6 +45,27 @@ LevelSchema.plugin(plugins.TranslationCoveragePlugin)
LevelSchema.post 'init', (doc) ->
if _.isString(doc.get('nextLevel'))
doc.set('nextLevel', undefined)

LevelSchema.post 'save', (doc) ->
return unless doc.get('version').isLatestMajor
Campaign = require('./Campaign')
# Leave setting campaign and campaignIndex to CampaignEditorView. In particular, for non-course levels
# we want campaignIndex on the campaign level object, but not on the level document.
denormalizedLevelProperties = _.without(Campaign.jsonSchema.denormalizedLevelProperties, 'campaignIndex', 'campaign')
update = { $set: {}, $unset: {} }
for property in denormalizedLevelProperties
path = "levels.#{doc.get('original')}.#{property}"
value = doc.get(property)
if value?
update.$set[path] = value
else
update.$unset[path] = ''
delete update.$unset if _.isEmpty(update.$unset)
delete update.$set if _.isEmpty(update.$set)
return if _.isEmpty(update)
query = {}
query["levels.#{doc.get('original')}"] = {$exists: true}
Campaign.update(query, update, {multi:true}).exec()

LevelSchema.statics.postEditableProperties = ['name']
LevelSchema.statics.jsonSchema = jsonSchema
Expand Down
25 changes: 25 additions & 0 deletions spec/server/functional/level.spec.coffee
Expand Up @@ -87,6 +87,31 @@ describe 'POST /db/level/:handle', ->
level = yield Level.findById(level.id)
expect(level.get('description')).toBe('Original desc')
done()

it 'updates campaigns that contain that level', utils.wrap (done) ->
admin = yield utils.initAdmin()
yield utils.loginUser(admin)

level = yield utils.makeLevel({name: 'First name'})
campaign = yield utils.makeCampaign({}, {levels: [level]})

otherLevel = yield utils.makeLevel()
unrelatedCampaign = yield utils.makeCampaign({}, {levels: [otherLevel]})

url = getURL("/db/level/#{level.id}")
levelJSON = level.toObject()
levelJSON.name = 'New name'
spyOn(Campaign, 'update').and.callThrough()
[res, body] = yield request.postAsync({url: url, json: levelJSON})
expect(Campaign.update.calls.count()).toBe(1)
yield Campaign.update.calls.mostRecent().returnValue # wait until update is finished
campaign = yield Campaign.findById(campaign.id)
expect(_.size(campaign.get('levels'))).toBe(1)
expect(campaign.get('levels')[level.get('original')].name).toBe('New name')
unrelatedCampaign = yield Campaign.findById(unrelatedCampaign.id)
expect(_.size(unrelatedCampaign.get('levels'))).toBe(1)
expect(unrelatedCampaign.get('levels')[level.get('original')]).not.toBe('New name')
done()

describe 'GET /db/level/:handle/session', ->

Expand Down

0 comments on commit 43bf457

Please sign in to comment.