From b33620779da30924a87eaaad66a2484eb727b01d Mon Sep 17 00:00:00 2001 From: Scott Erickson Date: Mon, 11 Apr 2016 11:59:51 -0700 Subject: [PATCH] Refactor achievement handler for #3469 --- server/commons/mapping.coffee | 1 - server/handlers/achievement_handler.coffee | 62 ------------------- server/middleware/achievements.coffee | 25 ++++++++ server/middleware/index.coffee | 1 + server/middleware/rest.coffee | 7 +++ server/models/Achievement.coffee | 20 ++++++ server/routes/index.coffee | 11 ++++ .../server/functional/achievement.spec.coffee | 45 +++++++++++--- 8 files changed, 101 insertions(+), 71 deletions(-) delete mode 100644 server/handlers/achievement_handler.coffee create mode 100644 server/middleware/achievements.coffee diff --git a/server/commons/mapping.coffee b/server/commons/mapping.coffee index 9b1c22bd2..d72b9ddce 100644 --- a/server/commons/mapping.coffee +++ b/server/commons/mapping.coffee @@ -24,7 +24,6 @@ module.exports.handlers = 'user_code_problem': 'handlers/user_code_problem_handler' 'user_remark': 'handlers/user_remark_handler' 'mail_sent': 'handlers/mail_sent_handler' - 'achievement': 'handlers/achievement_handler' 'earned_achievement': 'handlers/earned_achievement_handler' 'poll': 'handlers/poll_handler' 'prepaid': 'handlers/prepaid_handler' diff --git a/server/handlers/achievement_handler.coffee b/server/handlers/achievement_handler.coffee deleted file mode 100644 index f12539d9a..000000000 --- a/server/handlers/achievement_handler.coffee +++ /dev/null @@ -1,62 +0,0 @@ -Achievement = require './../models/Achievement' -Handler = require '../commons/Handler' - -class AchievementHandler extends Handler - modelClass: Achievement - - # Used to determine which properties requests may edit - editableProperties: [ - 'name' - 'query' - 'worth' - 'collection' - 'description' - 'userField' - 'proportionalTo' - 'icon' - 'function' - 'related' - 'difficulty' - 'category' - 'rewards' - 'i18n' - 'i18nCoverage' - ] - - allowedMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'] - jsonSchema = require '../../app/schemas/models/achievement.coffee' - - - hasAccess: (req) -> - req.method in ['GET', 'PUT'] or req.user?.isAdmin() or req.user?.isArtisan() - - hasAccessToDocument: (req, document, method=null) -> - method = (method or req.method).toLowerCase() - return true if method is 'get' - return true if req.user?.isAdmin() or req.user?.isArtisan() - return true if method is 'put' and @isJustFillingTranslations(req, document) - return - - get: (req, res) -> - # /db/achievement?related= - if req.query.related - return @sendForbiddenError(res) if not @hasAccess(req) - Achievement.find {related: req.query.related}, (err, docs) => - return @sendDatabaseError(res, err) if err - docs = (@formatEntity(req, doc) for doc in docs) - @sendSuccess res, docs - else - super req, res - - delete: (req, res, slugOrID) -> - return @sendForbiddenError res unless req.user?.isAdmin() or req.user?.isArtisan() - @getDocumentForIdOrSlug slugOrID, (err, document) => # Check first - return @sendDatabaseError(res, err) if err - return @sendNotFoundError(res) unless document? - document.remove (err, document) => - return @sendDatabaseError(res, err) if err - @sendNoContent res - - getNamesByIDs: (req, res) -> @getNamesByOriginals req, res, true - -module.exports = new AchievementHandler() diff --git a/server/middleware/achievements.coffee b/server/middleware/achievements.coffee new file mode 100644 index 000000000..739adafae --- /dev/null +++ b/server/middleware/achievements.coffee @@ -0,0 +1,25 @@ +errors = require '../commons/errors' +wrap = require 'co-express' +database = require '../commons/database' +Achievement = require '../models/Achievement' + +module.exports = + fetchByRelated: wrap (req, res, next) -> + related = req.query.related + return next() unless related + achievements = yield Achievement.find {related: related} + achievements = (achievement.toObject({req: req}) for achievement in achievements) + res.status(200).send(achievements) + + put: wrap (req, res, next) -> + achievement = yield database.getDocFromHandle(req, Achievement) + if not achievement + throw new errors.NotFound('Document not found.') + hasPermission = req.user.isAdmin() or req.user.isArtisan() + unless hasPermission or database.isJustFillingTranslations(req, achievement) + throw new errors.Forbidden('Must be an admin, artisan or submitting translations to edit an achievement') + + database.assignBody(req, achievement) + database.validateDoc(achievement) + achievement = yield achievement.save() + res.status(200).send(achievement.toObject({req: req})) diff --git a/server/middleware/index.coffee b/server/middleware/index.coffee index 15cd848ef..8bdb48609 100644 --- a/server/middleware/index.coffee +++ b/server/middleware/index.coffee @@ -1,4 +1,5 @@ module.exports = + achievements: require './achievements' auth: require './auth' classrooms: require './classrooms' campaigns: require './campaigns' diff --git a/server/middleware/rest.coffee b/server/middleware/rest.coffee index 738d0c565..b89362462 100644 --- a/server/middleware/rest.coffee +++ b/server/middleware/rest.coffee @@ -40,3 +40,10 @@ module.exports = database.validateDoc(doc) doc = yield doc.save() res.status(200).send(doc.toObject()) + + delete: (Model, options={}) -> wrap (req, res) -> + doc = yield database.getDocFromHandle(req, Model) + if not doc + throw new errors.NotFound('Document not found.') + yield doc.remove() + res.status(204).end() \ No newline at end of file diff --git a/server/models/Achievement.coffee b/server/models/Achievement.coffee index 3884b0fcc..70e8318a6 100644 --- a/server/models/Achievement.coffee +++ b/server/models/Achievement.coffee @@ -74,6 +74,26 @@ AchievementSchema.statics.getLoadedAchievements = -> AchievementSchema.statics.resetAchievements = -> delete AchievementSchema.statics.achievementCollections[collection] for collection of AchievementSchema.statics.achievementCollections + +AchievementSchema.statics.editableProperties = [ + 'name' + 'query' + 'worth' + 'collection' + 'description' + 'userField' + 'proportionalTo' + 'icon' + 'function' + 'related' + 'difficulty' + 'category' + 'rewards' + 'i18n' + 'i18nCoverage' +] + +AchievementSchema.statics.jsonSchema = require '../../app/schemas/models/achievement' # Queries are stored as JSON strings, objectify them upon loading AchievementSchema.post 'init', (doc) -> doc.objectifyQuery() diff --git a/server/routes/index.coffee b/server/routes/index.coffee index f7bb1d68e..1e7945e10 100644 --- a/server/routes/index.coffee +++ b/server/routes/index.coffee @@ -7,6 +7,17 @@ module.exports.setup = (app) -> app.post('/auth/spy', mw.auth.spy) app.post('/auth/stop-spying', mw.auth.stopSpying) + Achievement = require '../models/Achievement' + app.get('/db/achievement', mw.achievements.fetchByRelated, mw.rest.get(Achievement)) + app.post('/db/achievement', mw.auth.checkHasPermission(['admin', 'artisan']), mw.rest.post(Achievement)) + app.get('/db/achievement/:handle', mw.rest.getByHandle(Achievement)) + app.put('/db/achievement/:handle', mw.auth.checkLoggedIn(), mw.achievements.put) + app.delete('/db/achievement/:handle', mw.auth.checkHasPermission(['admin', 'artisan']), mw.rest.delete(Achievement)) + app.get('/db/achievement/names', mw.named.names(Achievement)) + app.get('/db/achievement/:handle/patches', mw.patchable.patches(Achievement)) + app.post('/db/achievement/:handle/watchers', mw.patchable.joinWatchers(Achievement)) + app.delete('/db/achievement/:handle/watchers', mw.patchable.leaveWatchers(Achievement)) + Article = require '../models/Article' app.get('/db/article', mw.rest.get(Article)) app.post('/db/article', mw.auth.checkHasPermission(['admin', 'artisan']), mw.rest.post(Article)) diff --git a/spec/server/functional/achievement.spec.coffee b/spec/server/functional/achievement.spec.coffee index 066c6a093..a0a97e68e 100644 --- a/spec/server/functional/achievement.spec.coffee +++ b/spec/server/functional/achievement.spec.coffee @@ -18,9 +18,10 @@ unlockable = description: 'Started playing Dungeon Arena.' worth: 3 collection: 'level.sessions' - query: "{\"level.original\":\"dungeon-arena\"}" + query: {'level.original':'dungeon-arena'} userField: 'creator' recalculable: true + related: 'a' unlockable2 = _.clone unlockable unlockable2.name = 'This one is obsolete' @@ -30,37 +31,39 @@ repeatable = description: 'Simulated Games.' worth: 1 collection: 'users' - query: "{\"simulatedBy\":{\"$gt\":0}}" + query: {'simulatedBy':{'$gt':0}} userField: '_id' proportionalTo: 'simulatedBy' recalculable: true rewards: gems: 1 + related: 'b' diminishing = name: 'Simulated2' worth: 1.5 collection: 'users' - query: "{\"simulatedBy\":{\"$gt\":0}}" + query: {'simulatedBy':{'$gt':0}} userField: '_id' proportionalTo: 'simulatedBy' function: kind: 'logarithmic' parameters: {a: 1, b: .5, c: .5, d: 1} recalculable: true + related: 'b' addAllAchievements = utils.wrap (done) -> yield utils.clearModels [Achievement, EarnedAchievement, LevelSession, User] @admin = yield utils.initAdmin() yield utils.loginUser(@admin) [res, body] = yield request.postAsync {uri: url, json: unlockable} - expect(res.statusCode).toBe(200) + expect(res.statusCode).toBe(201) @unlockable = yield Achievement.findById(body._id) [res, body] = yield request.postAsync {uri: url, json: repeatable} - expect(res.statusCode).toBe(200) + expect(res.statusCode).toBe(201) @repeatable = yield Achievement.findById(body._id) [res, body] = yield request.postAsync {uri: url, json: diminishing} - expect(res.statusCode).toBe(200) + expect(res.statusCode).toBe(201) @diminishing = yield Achievement.findById(body._id) done() @@ -81,7 +84,7 @@ describe 'POST /db/achievement', -> admin = yield utils.initAdmin() yield utils.loginUser(admin) [res, body] = yield request.postAsync {uri: url, json: unlockable} - expect(res.statusCode).toBe(200) + expect(res.statusCode).toBe(201) done() describe 'PUT /db/achievement', -> @@ -94,6 +97,12 @@ describe 'PUT /db/achievement', -> expect(res.statusCode).toBe(403) done() + it 'works for admins', utils.wrap (done) -> + [res, body] = yield request.putAsync {uri: url + '/'+@unlockable.id, json: {name: 'whatev'}} + expect(res.statusCode).toBe(200) + expect(res.body.name).toBe('whatev') + done() + describe 'GET /db/achievement', -> beforeEach addAllAchievements @@ -104,6 +113,16 @@ describe 'GET /db/achievement', -> expect(res.statusCode).toBe(200) expect(body.length).toBe 3 done() + +describe 'GET /db/achievement?related=:id', -> + beforeEach addAllAchievements + + it 'returns all achievements related to a given doc', utils.wrap (done) -> + [res, body] = yield request.getAsync {uri: url+'?related=b', json: true} + expect(res.statusCode).toBe(200) + expect(res.body.length).toBe(2) + expect(_.difference([@repeatable.id, @diminishing.id], (doc._id for doc in res.body)).length).toBe(0) + done() describe 'GET /db/achievement/:handle', -> beforeEach addAllAchievements @@ -129,6 +148,17 @@ describe 'DELETE /db/achievement/:handle', -> [res, body] = yield request.delAsync {uri: url + '/' + @unlockable.id} expect(res.statusCode).toBe(404) done() + + it 'returns 403 unless you are an admin or artisan', utils.wrap (done) -> + user = yield utils.initUser() + yield utils.loginUser(user) + [res, body] = yield request.delAsync {uri: url + '/' + @unlockable.id} + expect(res.statusCode).toBe(403) + artisan = yield utils.initArtisan() + yield utils.loginUser(artisan) + [res, body] = yield request.delAsync {uri: url + '/' + @unlockable.id} + expect(res.statusCode).toBe(204) + done() describe 'POST /db/earned_achievement', -> @@ -264,7 +294,6 @@ describe 'POST /admin/earned_achievement/recalculate', -> [res, body] = yield request.postAsync { uri:getURL '/admin/earned_achievement/recalculate' } expect(res.statusCode).toBe 202 yield new Promise((resolve) -> setTimeout(resolve, 500)) - console.log 'stop waiting' earnedAchievements = yield EarnedAchievement.find({}) expect(earnedAchievements.length).toBe 3