From 3e0518cd7033200306a06569edd54a619c4d4a6b Mon Sep 17 00:00:00 2001 From: Nick Winter Date: Mon, 1 Sep 2014 09:11:10 -0700 Subject: [PATCH] Fixed a few bugs and added a bunch of defensive handling for permissions not existing without new defaults. Fixed longstanding mistake with signup ignoring email newsletter setting. --- app/lib/forms.coffee | 4 ++-- app/models/CocoModel.coffee | 27 ++++++++++------------ app/models/LevelSession.coffee | 2 +- app/models/User.coffee | 5 +--- app/templates/account/settings.jade | 2 +- app/templates/base.jade | 2 +- app/views/EmployersView.coffee | 6 ++--- app/views/admin/CandidatesView.coffee | 8 +++---- app/views/editor/ForkModal.coffee | 2 +- app/views/modal/AuthModal.coffee | 2 +- app/views/modal/EmployerSignupModal.coffee | 2 +- app/views/play/level/tome/Spell.coffee | 2 +- app/views/user/JobProfileView.coffee | 8 +++---- server/users/user_handler.coffee | 8 +++---- 14 files changed, 35 insertions(+), 45 deletions(-) diff --git a/app/lib/forms.coffee b/app/lib/forms.coffee index 86fe1530d..f319be0f2 100644 --- a/app/lib/forms.coffee +++ b/app/lib/forms.coffee @@ -31,7 +31,7 @@ module.exports.applyErrorsToForm = (el, errors, warning=false) -> module.exports.setErrorToField = setErrorToField = (el, message, warning=false) -> formGroup = el.closest('.form-group') unless formGroup.length - return console.error "#{el} did not contain a form group" + return console.error el, " did not contain a form group, so couldn't show message:", message kind = if warning then 'warning' else 'error' formGroup.addClass "has-#{kind}" @@ -40,7 +40,7 @@ module.exports.setErrorToField = setErrorToField = (el, message, warning=false) module.exports.setErrorToProperty = setErrorToProperty = (el, property, message, warning=false) -> input = $("[name='#{property}']", el) unless input.length - return console.error "#{property} not found in #{el}" + return console.error "#{property} not found in", el, "so couldn't show message:", message setErrorToField input, message, warning diff --git a/app/models/CocoModel.coffee b/app/models/CocoModel.coffee index c6518340e..44a99bee3 100644 --- a/app/models/CocoModel.coffee +++ b/app/models/CocoModel.coffee @@ -51,16 +51,16 @@ class CocoModel extends Backbone.Model @loadFromBackup() getNormalizedURL: -> "#{@urlRoot}/#{@id}" - + attributesWithDefaults: undefined - + get: (attribute, withDefault=false) -> if withDefault if @attributesWithDefaults is undefined then @buildAttributesWithDefaults() return @attributesWithDefaults[attribute] else super(attribute) - + set: -> delete @attributesWithDefaults inFlux = @loading or not @loaded @@ -180,13 +180,13 @@ class CocoModel extends Backbone.Model clone isPublished: -> - for permission in @get('permissions') or [] + for permission in (@get('permissions', true) ? []) return true if permission.target is 'public' and permission.access is 'read' false publish: -> if @isPublished() then throw new Error('Can\'t publish what\'s already-published. Can\'t kill what\'s already dead.') - @set 'permissions', (@get('permissions') or []).concat({access: 'read', target: 'public'}) + @set 'permissions', @get('permissions', true).concat({access: 'read', target: 'public'}) @isObjectID: (s) -> s.length is 24 and s.match(/[a-f0-9]/gi)?.length is 24 @@ -195,10 +195,9 @@ class CocoModel extends Backbone.Model # actor is a User object actor ?= me return true if actor.isAdmin() - if @get('permissions')? - for permission in @get('permissions') - if permission.target is 'public' or actor.get('_id') is permission.target - return true if permission.access in ['owner', 'read'] + for permission in (@get('permissions', true) ? []) + if permission.target is 'public' or actor.get('_id') is permission.target + return true if permission.access in ['owner', 'read'] return false @@ -206,16 +205,14 @@ class CocoModel extends Backbone.Model # actor is a User object actor ?= me return true if actor.isAdmin() - if @get('permissions')? - for permission in @get('permissions') - if permission.target is 'public' or actor.get('_id') is permission.target - return true if permission.access in ['owner', 'write'] + for permission in (@get('permissions', true) ? []) + if permission.target is 'public' or actor.get('_id') is permission.target + return true if permission.access in ['owner', 'write'] return false getOwner: -> - return null unless permissions = @get 'permissions' - ownerPermission = _.find permissions, access: 'owner' + ownerPermission = _.find @get('permissions', true), access: 'owner' ownerPermission?.target getDelta: -> diff --git a/app/models/LevelSession.coffee b/app/models/LevelSession.coffee index 34b03be6d..ba87aef23 100644 --- a/app/models/LevelSession.coffee +++ b/app/models/LevelSession.coffee @@ -13,7 +13,7 @@ module.exports = class LevelSession extends CocoModel @set('state', state) updatePermissions: -> - permissions = @get 'permissions' + permissions = @get 'permissions', true permissions = (p for p in permissions when p.target isnt 'public') if @get('multiplayer') permissions.push {target: 'public', access: 'write'} diff --git a/app/models/User.coffee b/app/models/User.coffee index d9af11f11..25071ac26 100644 --- a/app/models/User.coffee +++ b/app/models/User.coffee @@ -13,10 +13,7 @@ module.exports = class User extends CocoModel CocoModel.pollAchievements() # Check for achievements on login super arguments... - isAdmin: -> - permissions = @attributes['permissions'] or [] - return 'admin' in permissions - + isAdmin: -> 'admin' in @get('permissions', true) isAnonymous: -> @get('anonymous', true) displayName: -> @get('name', true) diff --git a/app/templates/account/settings.jade b/app/templates/account/settings.jade index cbef47a7c..d07b97d16 100644 --- a/app/templates/account/settings.jade +++ b/app/templates/account/settings.jade @@ -32,7 +32,7 @@ block content .form - var name = me.get('name') || ''; - var email = me.get('email'); - - var admin = me.get('permissions').indexOf('admin') != -1; + - var admin = me.get('permissions', true).indexOf('admin') != -1; .form-group label.control-label(for="name", data-i18n="general.name") Name input#name.form-control(name="name", type="text", value="#{name}") diff --git a/app/templates/base.jade b/app/templates/base.jade index 2d36b5d0e..a80d84c89 100644 --- a/app/templates/base.jade +++ b/app/templates/base.jade @@ -67,7 +67,7 @@ body .footer.clearfix .content p.footer-link-text - if pathname == "/" || (me.get('permissions') || []).indexOf('employer') != -1 + if pathname == "/" || (me.get('permissions', true)).indexOf('employer') != -1 a(href='/employers', title='Home', tabindex=-1, data-i18n="nav.employers") Employers else a(href='/', title='Home', tabindex=-1, data-i18n="nav.home") Home diff --git a/app/views/EmployersView.coffee b/app/views/EmployersView.coffee index bb1292c93..4e9c45e5f 100644 --- a/app/views/EmployersView.coffee +++ b/app/views/EmployersView.coffee @@ -197,9 +197,7 @@ module.exports = class EmployersView extends RootView ctx.numberOfCandidates = ctx.featuredCandidates.length ctx - isEmployer: -> - userPermissions = me.get('permissions') ? [] - _.contains userPermissions, 'employer' + isEmployer: -> 'employer' in me.get('permissions', true) setUpScrolling: => $('.nano').nanoScroller() @@ -209,7 +207,7 @@ module.exports = class EmployersView extends RootView # $('.nano').nanoScroller({scrollTo: $(window.location.hash)}) checkForEmployerSignupHash: => - if window.location.hash is '#employerSignupLoggingIn' and not ('employer' in me.get('permissions')) and not me.isAdmin() + if window.location.hash is '#employerSignupLoggingIn' and not ('employer' in me.get('permissions', true)) and not me.isAdmin() @openModalView new EmployerSignupModal window.location.hash = '' diff --git a/app/views/admin/CandidatesView.coffee b/app/views/admin/CandidatesView.coffee index 3216be2dc..39ddca817 100644 --- a/app/views/admin/CandidatesView.coffee +++ b/app/views/admin/CandidatesView.coffee @@ -52,9 +52,7 @@ module.exports = class CandidatesView extends RootView ctx._ = _ ctx - isEmployer: -> - userPermissions = me.get('permissions') ? [] - _.contains userPermissions, "employer" + isEmployer: -> 'employer' in me.get('permissions', true) setUpScrolling: -> $(".nano").nanoScroller() @@ -64,9 +62,9 @@ module.exports = class CandidatesView extends RootView $(".nano").nanoScroller({scrollTo:$(window.location.hash)}) checkForEmployerSignupHash: => - if window.location.hash is "#employerSignupLoggingIn" and not ("employer" in me.get("permissions")) + if window.location.hash is "#employerSignupLoggingIn" and not ("employer" in me.get('permissions', true)) @openModalView new EmployerSignupModal - window.location.hash = "" + window.location.hash = '' sortTable: -> # http://mottie.github.io/tablesorter/docs/example-widget-bootstrap-theme.html diff --git a/app/views/editor/ForkModal.coffee b/app/views/editor/ForkModal.coffee index 99e4170ea..63399693f 100644 --- a/app/views/editor/ForkModal.coffee +++ b/app/views/editor/ForkModal.coffee @@ -31,7 +31,7 @@ module.exports = class ForkModal extends ModalView newModel.unset 'parent' newModel.set 'commitMessage', "Forked from #{@model.get('name')}" newModel.set 'name', @$el.find('#fork-model-name').val() - if @model.get 'permissions' + if @model.schema.properties.permissions newModel.set 'permissions', [access: 'owner', target: me.id] newPathPrefix = "editor/#{@editorPath}/" res = newModel.save() diff --git a/app/views/modal/AuthModal.coffee b/app/views/modal/AuthModal.coffee index 67abf2537..251a45a8a 100644 --- a/app/views/modal/AuthModal.coffee +++ b/app/views/modal/AuthModal.coffee @@ -75,7 +75,7 @@ module.exports = class AuthModal extends ModalView userObject.name = @suggestedName if @suggestedName for key, val of me.attributes when key in ['preferredLanguage', 'testGroupNumber', 'dateCreated', 'wizardColor1', 'name', 'music', 'volume', 'emails'] userObject[key] ?= val - subscribe = @$el.find('#signup-subscribe').prop('checked') + subscribe = @$el.find('#subscribe').prop('checked') userObject.emails ?= {} userObject.emails.generalNews ?= {} userObject.emails.generalNews.enabled = subscribe diff --git a/app/views/modal/EmployerSignupModal.coffee b/app/views/modal/EmployerSignupModal.coffee index 2c5306356..ab13e309b 100644 --- a/app/views/modal/EmployerSignupModal.coffee +++ b/app/views/modal/EmployerSignupModal.coffee @@ -57,7 +57,7 @@ module.exports = class EmployerSignupModal extends ModalView getRenderData: -> context = super() context.userIsAuthorized = @authorizedWithLinkedIn - context.userHasSignedContract = 'employer' in me.get('permissions') + context.userHasSignedContract = 'employer' in me.get('permissions', true) context.userIsAnonymous = context.me.get('anonymous') context.sentMoreInfoEmail = @sentMoreInfoEmail context diff --git a/app/views/play/level/tome/Spell.coffee b/app/views/play/level/tome/Spell.coffee index 5d56e131d..cfde25fe7 100644 --- a/app/views/play/level/tome/Spell.coffee +++ b/app/views/play/level/tome/Spell.coffee @@ -159,5 +159,5 @@ module.exports = class Spell return true if @spectateView # Use transpiled code for both teams if we're just spectating. return true if @isEnemySpell() # Use transpiled for enemy spells. # Players without permissions can't view the raw code. - return true if @session.get('creator') isnt me.id and not (me.isAdmin() or 'employer' in me.get('permissions')) + return true if @session.get('creator') isnt me.id and not (me.isAdmin() or 'employer' in me.get('permissions', true)) false diff --git a/app/views/user/JobProfileView.coffee b/app/views/user/JobProfileView.coffee index 8a176782e..9edc938fd 100644 --- a/app/views/user/JobProfileView.coffee +++ b/app/views/user/JobProfileView.coffee @@ -72,7 +72,7 @@ module.exports = class JobProfileView extends UserView finishInit: -> return unless @userID @uploadFilePath = "db/user/#{@userID}" - + if @user?.get('firstName') jobProfile = @user.get('jobProfile') jobProfile ?= {} @@ -81,7 +81,7 @@ module.exports = class JobProfileView extends UserView @user.set('jobProfile', jobProfile) @highlightedContainers = [] - if me.isAdmin() or 'employer' in me.get('permissions') + if me.isAdmin() or 'employer' in me.get('permissions', true) $.post "/db/user/#{me.id}/track/view_candidate" $.post "/db/user/#{@userID}/track/viewed_by_employer" unless me.isAdmin() @sessions = @supermodel.loadCollection(new LevelSessionsCollection(@userID), 'candidate_sessions').model @@ -235,7 +235,7 @@ module.exports = class JobProfileView extends UserView context.rawProfile = @user.get('jobProfile') or {} context.user = @user context.myProfile = @isMe() - context.allowedToViewJobProfile = @user and (me.isAdmin() or 'employer' in me.get('permissions') or (context.myProfile && !me.get('anonymous'))) + context.allowedToViewJobProfile = @user and (me.isAdmin() or 'employer' in me.get('permissions', true) or (context.myProfile && !me.get('anonymous'))) context.allowedToEditJobProfile = @user and (me.isAdmin() or (context.myProfile && !me.get('anonymous'))) context.profileApproved = @user?.get 'jobProfileApproved' context.progress = @progress ? @updateProgress() @@ -589,4 +589,4 @@ module.exports = class JobProfileView extends UserView destroy: -> @remarkTreema?.destroy() - super() \ No newline at end of file + super() diff --git a/server/users/user_handler.coffee b/server/users/user_handler.coffee index f3d40b649..40eed11ea 100644 --- a/server/users/user_handler.coffee +++ b/server/users/user_handler.coffee @@ -224,7 +224,7 @@ UserHandler = class UserHandler extends Handler res.end() getLevelSessionsForEmployer: (req, res, userID) -> - return @sendUnauthorizedError(res) unless req.user._id+'' is userID or req.user.isAdmin() or ('employer' in req.user.get('permissions')) + return @sendUnauthorizedError(res) unless req.user._id+'' is userID or req.user.isAdmin() or ('employer' in (req.user.get('permissions') ? [])) query = creator: userID, levelID: {$in: ['gridmancer', 'greed', 'dungeon-arena', 'brawlwood', 'gold-rush']} projection = 'levelName levelID team playtime codeLanguage submitted code totalScore teamSpells level' LevelSession.find(query).select(projection).exec (err, documents) => @@ -280,7 +280,7 @@ UserHandler = class UserHandler extends Handler return @sendMethodNotAllowed res unless req.method is 'POST' isMe = userID is req.user._id + '' isAuthorized = isMe or req.user.isAdmin() - isAuthorized ||= ('employer' in req.user.get('permissions')) and (activityName in ['viewed_by_employer', 'contacted_by_employer']) + isAuthorized ||= ('employer' in (req.user.get('permissions') ? [])) and (activityName in ['viewed_by_employer', 'contacted_by_employer']) return @sendUnauthorizedError res unless isAuthorized updateUser = (user) => activity = user.trackActivity activityName, increment @@ -303,7 +303,7 @@ UserHandler = class UserHandler extends Handler if not profileData.id or not profileData.positions or not profileData.emailAddress or not profileData.firstName or not profileData.lastName return errors.badInput(res, 'You need to have a more complete profile to sign up for this service.') @modelClass.findById(req.user.id).exec (err, user) => - if user.get('employerAt') or user.get('signedEmployerAgreement') or 'employer' in user.get('permissions') + if user.get('employerAt') or user.get('signedEmployerAgreement') or 'employer' in (user.get('permissions') ? []) return errors.conflict(res, 'You already have signed the agreement!') #TODO: Search for the current position employerAt = _.filter(profileData.positions.values, 'isCurrent')[0]?.company.name ? 'Not available' @@ -322,7 +322,7 @@ UserHandler = class UserHandler extends Handler res.end() getCandidates: (req, res) -> - authorized = req.user.isAdmin() or ('employer' in req.user.get('permissions')) + authorized = req.user.isAdmin() or ('employer' in (req.user.get('permissions') ? [])) months = if req.user.isAdmin() then 12 else 2 since = (new Date((new Date()) - months * 30.4 * 86400 * 1000)).toISOString() query = {'jobProfile.updated': {$gt: since}}