From 183fe9c49675f81dd557696649d17c2867d36fca Mon Sep 17 00:00:00 2001 From: Nick Winter Date: Fri, 25 Apr 2014 10:46:43 -0700 Subject: [PATCH 1/3] Code review of 97d3b6. --- app/lib/LinkedInHandler.coffee | 13 ++--- app/schemas/models/user.coffee | 9 +-- .../modal/employer_signup_modal.jade | 2 +- app/views/employers_view.coffee | 2 +- app/views/modal/employer_signup_modal.coffee | 56 ++++++------------- server/users/user_handler.coffee | 8 ++- 6 files changed, 36 insertions(+), 54 deletions(-) diff --git a/app/lib/LinkedInHandler.coffee b/app/lib/LinkedInHandler.coffee index 415e142ac..69f9cb473 100644 --- a/app/lib/LinkedInHandler.coffee +++ b/app/lib/LinkedInHandler.coffee @@ -7,22 +7,21 @@ storage = require 'lib/storage' module.exports = LinkedInHandler = class LinkedInHandler extends CocoClass constructor: -> super() - + subscriptions: - 'linkedin-loaded':'onLinkedInLoaded' - - onLinkedInLoaded: (e) => + 'linkedin-loaded': 'onLinkedInLoaded' + + onLinkedInLoaded: (e) -> IN.Event.on IN, "auth", @onLinkedInAuth onLinkedInAuth: (e) => console.log "Authorized with LinkedIn" - + constructEmployerAgreementObject: (cb) => IN.API.Profile("me") .fields(["positions","public-profile-url","id","first-name","last-name","email-address"]) .error(cb) .result (profiles) => cb null, profiles.values[0] - - + destroy: -> super() diff --git a/app/schemas/models/user.coffee b/app/schemas/models/user.coffee index b19535d1d..2ea6ab67f 100644 --- a/app/schemas/models/user.coffee +++ b/app/schemas/models/user.coffee @@ -30,10 +30,11 @@ UserSchema = c.object {}, artisanNews: { $ref: '#/definitions/emailSubscription' } diplomatNews: { $ref: '#/definitions/emailSubscription' } scribeNews: { $ref: '#/definitions/emailSubscription' } - + # notifications anyNotes: { $ref: '#/definitions/emailSubscription' } # overrides any other notifications settings recruitNotes: { $ref: '#/definitions/emailSubscription' } + employerNotes: { $ref: '#/definitions/emailSubscription' } # server controlled permissions: c.array {'default': []}, c.shortString() @@ -53,7 +54,7 @@ UserSchema = c.object {}, #Internationalization stuff preferredLanguage: {type: 'string', default: 'en', 'enum': c.getLanguageCodeArray()} - + signedCLA: c.date({title: 'Date Signed the CLA'}) wizard: c.object {}, colorConfig: c.object {additionalProperties: c.colorConfig()} @@ -112,8 +113,8 @@ UserSchema = c.object {}, signedEmployerAgreement: c.object {}, linkedinID: c.shortString {title:"LinkedInID", description: "The user's LinkedIn ID when they signed the contract."} date: c.date {title: "Date signed employer agreement"} - data: c.object - + data: c.object {description: "Cached LinkedIn data slurped from profile."} + c.extendBasicProperties UserSchema, 'user' diff --git a/app/templates/modal/employer_signup_modal.jade b/app/templates/modal/employer_signup_modal.jade index 7bee4aabb..1e3605d73 100644 --- a/app/templates/modal/employer_signup_modal.jade +++ b/app/templates/modal/employer_signup_modal.jade @@ -63,6 +63,6 @@ block modal-footer button.btn.btn-primary(id="contract-agreement-button") I agree else .modal-footer.linkedin - | Thanks #{firstName}! You've already agreed to the contract. + | Thanks! You've already agreed to the contract. \ No newline at end of file diff --git a/app/views/employers_view.coffee b/app/views/employers_view.coffee index 59593d121..1fd874890 100644 --- a/app/views/employers_view.coffee +++ b/app/views/employers_view.coffee @@ -28,7 +28,7 @@ module.exports = class EmployersView extends View getRenderData: -> c = super() c.candidates = @candidates.models - userPermissions = me.get('permissions') || [] + userPermissions = me.get('permissions') ? [] c.isEmployer = _.contains userPermissions, "employer" c.moment = moment diff --git a/app/views/modal/employer_signup_modal.coffee b/app/views/modal/employer_signup_modal.coffee index ec89603cf..9e03f2c95 100644 --- a/app/views/modal/employer_signup_modal.coffee +++ b/app/views/modal/employer_signup_modal.coffee @@ -10,27 +10,27 @@ module.exports = class EmployerSignupView extends View template: template closeButton: true - + subscriptions: "server-error": "onServerError" "created-user-without-reload": "linkedInAuth" - + events: "click #contract-agreement-button": "agreeToContract" - - + + constructor: (options) -> super(options) @authorizedWithLinkedIn = IN?.User?.isAuthorized() window.tracker?.trackEvent 'Started Employer Signup' @reloadWhenClosed = false - window.contractCallback = => + window.contractCallback = => @authorizedWithLinkedIn = IN?.User?.isAuthorized() @render() - - onServerError: (e) -> + + onServerError: (e) -> @disableModalInProgress(@$el) - + afterInsert: -> super() linkedInButtonParentElement = document.getElementById("linkedInAuthButton")?.parentNode @@ -38,16 +38,14 @@ module.exports = class EmployerSignupView extends View IN.parse() if me.get('anonymous') $(".IN-widget").get(0).addEventListener('click', @createAccount, true) - console.log "Parsed linkedin button element!" - console.log linkedInButtonParentElement - + getRenderData: -> context = super() context.userIsAuthorized = @authorizedWithLinkedIn context.userHasSignedContract = "employer" in me.get("permissions") context.userIsAnonymous = context.me.get('anonymous') context - + agreeToContract: -> application.linkedinHandler.constructEmployerAgreementObject (err, profileData) => if err? then return handleAgreementFailure err @@ -57,54 +55,36 @@ module.exports = class EmployerSignupView extends View type: "POST" success: @handleAgreementSuccess error: @handleAgreementFailure - + handleAgreementSuccess: (result) -> window.tracker?.trackEvent 'Employer Agreed to Contract' me.fetch() window.location.reload() - + handleAgreementFailure: (error) -> alert "There was an error signing the contract. Please contact team@codecombat.com with this error: #{error.responseText}" - + createAccount: (e) => window.tracker?.trackEvent 'Finished Employer Signup' - console.log "Tried to create account!" e.stopPropagation() forms.clearFormAlerts(@$el) userObject = forms.formToObject @$el delete userObject.subscribe for key, val of me.attributes when key in ["preferredLanguage", "testGroupNumber", "dateCreated", "wizardColor1", "name", "music", "volume", "emails"] userObject[key] ?= val - subscribe = true - #TODO: Enable all email subscriptions - - userObject.emails ?= {} - userObject.emails.generalNews ?= {} - userObject.emails.generalNews.enabled = subscribe + userObject.emails.employerNotes = {enabled: true} res = tv4.validateMultiple userObject, User.schema return forms.applyErrorsToForm(@$el, res.errors) unless res.valid - window.tracker?.trackEvent 'Finished Signup' @enableModalInProgress(@$el) auth.createUserWithoutReload userObject, null - console.log "Authorizing with linkedin" - IN.User.authorize(@recordUserDetails, @) - - linkedInAuth: (e) => + IN.User.authorize @render, @ + + linkedInAuth: (e) -> me.fetch() @reloadWhenClosed = true - - - recordUserDetails: (e) => - #TODO: refactor this out - @render() - + destroy: -> reloadWhenClosed = @reloadWhenClosed super() if reloadWhenClosed window.location.reload() - - - - - \ No newline at end of file diff --git a/server/users/user_handler.coffee b/server/users/user_handler.coffee index 30e49ac6f..b720f1e5c 100644 --- a/server/users/user_handler.coffee +++ b/server/users/user_handler.coffee @@ -232,6 +232,7 @@ UserHandler = class UserHandler extends Handler return @sendDatabaseError(res, err) if err documents = (LevelSessionHandler.formatEntity(req, doc) for doc in documents) @sendSuccess(res, documents) + agreeToEmployerAgreement: (req, res) -> userIsAnonymous = req.user?.get('anonymous') if userIsAnonymous then return errors.unauthorized(res, "You need to be logged in to agree to the employer agreeement.") @@ -244,19 +245,20 @@ UserHandler = class UserHandler extends Handler 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" - signedEmployerAgreement = + signedEmployerAgreement = linkedinID: profileData.id date: new Date() data: profileData - updateObject = + updateObject = "employerAt": employerAt "signedEmployerAgreement": signedEmployerAgreement $push: "permissions":'employer' - + User.update {"_id": req.user.id}, updateObject, (err, result) => if err? then return errors.serverError(res, "There was an issue updating the user object to reflect employer status: #{err}") res.send({"message": "The agreement was successful."}) res.end() + getCandidates: (req, res) -> authorized = req.user.isAdmin() or ('employer' in req.user.get('permissions')) since = (new Date((new Date()) - 2 * 30.4 * 86400 * 1000)).toISOString() From 2ff97c3bcdfea96a3455173a7305714c26b0e53b Mon Sep 17 00:00:00 2001 From: Nick Winter Date: Fri, 25 Apr 2014 10:55:37 -0700 Subject: [PATCH 2/3] I guess emails might not be there. --- app/views/modal/employer_signup_modal.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/modal/employer_signup_modal.coffee b/app/views/modal/employer_signup_modal.coffee index 9e03f2c95..61154d23b 100644 --- a/app/views/modal/employer_signup_modal.coffee +++ b/app/views/modal/employer_signup_modal.coffee @@ -72,6 +72,7 @@ module.exports = class EmployerSignupView extends View delete userObject.subscribe for key, val of me.attributes when key in ["preferredLanguage", "testGroupNumber", "dateCreated", "wizardColor1", "name", "music", "volume", "emails"] userObject[key] ?= val + userObject.emails ?= {} userObject.emails.employerNotes = {enabled: true} res = tv4.validateMultiple userObject, User.schema return forms.applyErrorsToForm(@$el, res.errors) unless res.valid From ef30498985cfcc83a8599bf8a03d1902e50760f7 Mon Sep 17 00:00:00 2001 From: Nick Winter Date: Fri, 25 Apr 2014 11:12:52 -0700 Subject: [PATCH 3/3] Improved bad sendwithus error logging. --- server/users/User.coffee | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/server/users/User.coffee b/server/users/User.coffee index 1b591a304..45c6e6b9a 100644 --- a/server/users/User.coffee +++ b/server/users/User.coffee @@ -29,7 +29,7 @@ UserSchema.post('init', -> UserSchema.methods.isAdmin = -> p = @get('permissions') return p and 'admin' in p - + emailNameMap = generalNews: 'announcement' adventurerNews: 'tester' @@ -39,20 +39,20 @@ emailNameMap = diplomatNews: 'translator' ambassadorNews: 'support' anyNotes: 'notification' - + UserSchema.methods.setEmailSubscription = (newName, enabled) -> oldSubs = _.clone @get('emailSubscriptions') if oldSubs and oldName = emailNameMap[newName] oldSubs = (s for s in oldSubs when s isnt oldName) oldSubs.push(oldName) if enabled @set('emailSubscriptions', oldSubs) - + newSubs = _.clone(@get('emails') or _.cloneDeep(jsonschema.properties.emails.default)) newSubs[newName] ?= {} newSubs[newName].enabled = enabled @set('emails', newSubs) @newsSubsChanged = true if newName in mail.NEWS_GROUPS - + UserSchema.methods.isEmailSubscriptionEnabled = (newName) -> emails = @get 'emails' if not emails @@ -74,10 +74,10 @@ UserSchema.statics.updateMailChimp = (doc, callback) -> newGroups = [] for [mailchimpEmailGroup, emailGroup] in _.zip(mail.MAILCHIMP_GROUPS, mail.NEWS_GROUPS) newGroups.push(mailchimpEmailGroup) if doc.isEmailSubscriptionEnabled(emailGroup) - + if (not existingProps) and newGroups.length is 0 return callback?() # don't add totally unsubscribed people to the list - + params = {} params.id = mail.MAILCHIMP_LIST_ID params.email = if existingProps then {leid:existingProps.leid} else {email:doc.get('email')} @@ -113,7 +113,7 @@ UserSchema.pre('save', (next) -> recipient: address: @get 'email' sendwithus.api.send data, (err, result) -> - log.error 'error', err, 'result', result if err + log.error "sendwithus post-save error: #{err}, result: #{result}" if err next() )