Merge branch 'nameable_users' of https://github.com/rubenvereecken/codecombat into rubenvereecken-nameable_users

app/lib/utils.coffee

@@ -76,6 +76,8 @@ module.exports.getByPath = (target, path) ->
     obj = obj[piece]
   obj
 
+module.exports.isID = (id) -> _.isString(id) and id.length is 24 and id.match(/[a-f0-9]/gi)?.length is 24
+
 module.exports.round = _.curry (digits, n) ->
   n = +n.toFixed(digits)
 

app/models/User.coffee

@@ -1,6 +1,7 @@
 GRAVATAR_URL = 'https://www.gravatar.com/'
 cache = {}
 CocoModel = require './CocoModel'
+util = require 'lib/utils'
 
 module.exports = class User extends CocoModel
   @className: 'User'
@@ -45,6 +46,28 @@ module.exports = class User extends CocoModel
     cache[id] = user
     user
 
+  # callbacks can be either success or error
+  @getByIDOrSlug: (idOrSlug, force, callbacks={}) ->
+    {me} = require 'lib/auth'
+    isID = util.isID idOrSlug
+    if me.id is idOrSlug or me.slug is idOrSlug
+      callbacks.success me if callbacks.success?
+      return me
+    cached = cache[idOrSlug]
+    user = cached or new @ _id: idOrSlug
+    if force or not cached
+      user.loading = true
+      user.fetch
+        success: ->
+          user.loading = false
+          Backbone.Mediator.publish 'user:fetched'
+          callbacks.success user if callbacks.success?
+        error: ->
+          user.loading = false
+          callbacks.error user if callbacks.error?
+    cache[idOrSlug] = user
+    user
+
   getEnabledEmails: ->
     @migrateEmails()
     emails = _.clone(@get('emails')) or {}

app/schemas/models/user.coffee

@@ -1,8 +1,12 @@
 c = require './../schemas'
 emailSubscriptions = ['announcement', 'tester', 'level_creator', 'developer', 'article_editor', 'translator', 'support', 'notification']
 
-UserSchema = c.object {},
-  name: c.shortString({title: 'Display Name', default: ''})
+UserSchema = c.object
+  title: 'User'
+
+c.extendNamedProperties UserSchema  # let's have the name be the first property
+
+_.extend UserSchema.properties,
   email: c.shortString({title: 'Email', format: 'email'})
   firstName: c.shortString({title: 'First Name'})
   lastName: c.shortString({title: 'Last Name'})

server/plugins/plugins.coffee

@@ -41,9 +41,12 @@ module.exports.NamedPlugin = (schema) ->
       err.response = {message: ' is a reserved name', property: 'name'}
       err.code = 422
       return next(err)
-    if newSlug isnt @get('slug')
+    if newSlug not in [@get('slug'), ''] and not @get 'anonymous'
       @set('slug', newSlug)
       @checkSlugConflicts(next)
+    else if newSlug is '' and @get 'slug'
+      @set 'slug', undefined
+      next()
     else
       next()
   )

server/routes/auth.coffee

@@ -159,15 +159,11 @@ module.exports.setup = (app) ->
 
 module.exports.loginUser = loginUser = (req, res, user, send=true, next=null) ->
   user.save((err) ->
-    if err
-      return @sendDatabaseError(res, err)
+    return errors.serverError res, err if err?
 
     req.logIn(user, (err) ->
-      if err
-        return @sendDatabaseError(res, err)
-
-      if send
-        return @sendSuccess(res, user)
+      return errors.serverError res, err if err?
+      return res.send user if send
       next() if next
     )
   )

server/users/User.coffee

@@ -4,6 +4,7 @@ crypto = require 'crypto'
 {salt, isProduction} = require '../../server_config'
 mail = require '../commons/mail'
 log = require 'winston'
+plugins = require '../plugins/plugins'
 
 sendwithus = require '../sendwithus'
 
@@ -29,6 +30,9 @@ UserSchema.methods.isAdmin = ->
   p = @get('permissions')
   return p and 'admin' in p
 
+UserSchema.methods.isAnonymous = ->
+  @get 'anonymous'
+
 UserSchema.methods.trackActivity = (activityName, increment) ->
   now = new Date()
   increment ?= parseInt increment or 1
@@ -108,6 +112,30 @@ UserSchema.statics.updateMailChimp = (doc, callback) ->
 
   mc?.lists.subscribe params, onSuccess, onFailure
 
+UserSchema.statics.unconflictName = unconflictName = (name, done) ->
+  User.findOne {slug: _.str.slugify(name)}, (err, otherUser) ->
+    return done err if err?
+    return done null, name unless otherUser
+    suffix = _.random(0, 9) + ''
+    unconflictName name + suffix, done
+
+UserSchema.methods.register = (done) ->
+  @set('anonymous', false)
+  @set('permissions', ['admin']) if not isProduction
+  if (name = @get 'name')? and name isnt ''
+    unconflictName name, (err, uniqueName) =>
+      return done err if err
+      @set 'name', uniqueName
+      done()
+  else done()
+  data =
+    email_id: sendwithus.templates.welcome_email
+    recipient:
+      address: @get 'email'
+  sendwithus.api.send data, (err, result) ->
+    log.error "sendwithus post-save error: #{err}, result: #{result}" if err
+
+
 UserSchema.pre('save', (next) ->
   @set('emailLower', @get('email')?.toLowerCase())
   @set('nameLower', @get('name')?.toLowerCase())
@@ -115,16 +143,10 @@ UserSchema.pre('save', (next) ->
   if @get('password')
     @set('passwordHash', User.hashPassword(pwd))
     @set('password', undefined)
-  if @get('email') and @get('anonymous')
-    @set('anonymous', false)
-    @set('permissions', ['admin']) if not isProduction
-    data =
-      email_id: sendwithus.templates.welcome_email
-      recipient:
-        address: @get 'email'
-    sendwithus.api.send data, (err, result) ->
-      log.error "sendwithus post-save error: #{err}, result: #{result}" if err
-  next()
+  if @get('email') and @get('anonymous') # a user registers
+    @register next
+  else
+    next()
 )
 
 UserSchema.post 'save', (doc) ->
@@ -136,6 +158,8 @@ UserSchema.statics.hashPassword = (password) ->
   shasum.update(salt + password)
   shasum.digest('hex')
 
+UserSchema.plugin plugins.NamedPlugin
+
 module.exports = User = mongoose.model('User', UserSchema)
 
 AchievablePlugin = require '../plugins/achievements'

server/users/user_handler.coffee

@@ -104,7 +104,8 @@ UserHandler = class UserHandler extends Handler
       return callback(null, req, user) unless req.body.name
       nameLower = req.body.name?.toLowerCase()
       return callback(null, req, user) unless nameLower
-      return callback(null, req, user) if nameLower is user.get('nameLower') and not user.get('anonymous')
+      return callback(null, req, user) if user.get 'anonymous' # anonymous users can have any name
+      return callback(null, req, user) if nameLower is user.get('nameLower')
       User.findOne({nameLower: nameLower, anonymous: false}).exec (err, otherUser) ->
         log.error "Database error setting user name: #{err}" if err
         return callback(res: 'Database error.', code: 500) if err
@@ -116,7 +117,7 @@ UserHandler = class UserHandler extends Handler
   ]
 
   getById: (req, res, id) ->
-    if req.user?._id.equals(id)
+    if Handler.isID(id) and req.user?._id.equals(id)
       return @sendSuccess(res, @formatEntity(req, req.user, 256))
     super(req, res, id)
 

test/server/functional/user.spec.coffee

@@ -44,7 +44,7 @@ describe 'User.updateMailChimp', ->
 
 describe 'POST /db/user', ->
 
-  createAnonNameUser = (done)->
+  createAnonNameUser = (name, done)->
     request.post getURL('/auth/logout'), ->
       request.get getURL('/auth/whoami'), ->
         req = request.post(getURL('/db/user'), (err, response) ->
@@ -52,11 +52,11 @@ describe 'POST /db/user', ->
           request.get getURL('/auth/whoami'), (request, response, body) ->
             res = JSON.parse(response.body)
             expect(res.anonymous).toBeTruthy()
-            expect(res.name).toEqual('Jim')
+            expect(res.name).toEqual(name)
             done()
         )
         form = req.form()
-        form.append('name', 'Jim')
+        form.append('name', name)
 
   it 'preparing test : clears the db first', (done) ->
     clearModels [User], (err) ->
@@ -105,30 +105,18 @@ describe 'POST /db/user', ->
           done()
 
   it 'should allow setting anonymous user name', (done) ->
-    createAnonNameUser(done)
+    createAnonNameUser('Jim', done)
 
   it 'should allow multiple anonymous users with same name', (done) ->
-    createAnonNameUser(done)
-
-
-  it 'should not allow setting existing user name to anonymous user', (done) ->
-
-    createAnonUser = ->
-      request.post getURL('/auth/logout'), ->
-        request.get getURL('/auth/whoami'), ->
-          req = request.post(getURL('/db/user'), (err, response) ->
-            expect(response.statusCode).toBe(409)
-            done()
-          )
-          form = req.form()
-          form.append('name', 'Jim')
+    createAnonNameUser('Jim', done)
 
+  it 'should allow setting existing user name to anonymous user', (done) ->
     req = request.post(getURL('/db/user'), (err, response, body) ->
       expect(response.statusCode).toBe(200)
       request.get getURL('/auth/whoami'), (request, response, body) ->
         res = JSON.parse(response.body)
         expect(res.anonymous).toBeFalsy()
-        createAnonUser()
+        createAnonNameUser 'Jim', done
     )
     form = req.form()
     form.append('email', 'new@user.com')
@@ -213,6 +201,55 @@ ghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghlfarghlarghl
       form.append('email', 'New@email.com')
       form.append('name', 'Wilhelm')
 
+  it 'should not allow two users with the same name slug', (done) ->
+    loginSam (sam) ->
+      samsName = sam.get 'name'
+      sam.set 'name', 'admin'
+      request.put {uri:getURL(urlUser + '/' + sam.id), json: sam.toObject()}, (err, response) ->
+        expect(err).toBeNull()
+        expect(response.statusCode).toBe 409
+
+        # Restore Sam
+        sam.set 'name', samsName
+        done()
+
+  it 'should silently rename an anonymous user if their name conflicts upon signup', (done) ->
+    request.post getURL('/auth/logout'), ->
+      request.get getURL('/auth/whoami'), ->
+        req = request.post getURL('/db/user'), (err, response) ->
+          expect(response.statusCode).toBe(200)
+          request.get getURL('/auth/whoami'), (err, response) ->
+            expect(err).toBeNull()
+            guy = JSON.parse(response.body)
+            expect(guy.anonymous).toBeTruthy()
+            expect(guy.name).toEqual 'admin'
+
+            guy.email = 'blub@blub' # Email means registration
+            req = request.post {url: getURL('/db/user'), json: guy}, (err, response) ->
+              expect(err).toBeNull()
+              finalGuy = response.body
+              expect(finalGuy.anonymous).toBeFalsy()
+              expect(finalGuy.name).not.toEqual guy.name
+              expect(finalGuy.name.length).toBe guy.name.length + 1
+              done()
+        form = req.form()
+        form.append('name', 'admin')
+
+  it 'should be able to unset a slug by setting an empty name', (done) ->
+    loginSam (sam) ->
+      samsName = sam.get 'name'
+      sam.set 'name', ''
+      request.put {uri:getURL(urlUser + '/' + sam.id), json: sam.toObject()}, (err, response) ->
+        expect(err).toBeNull()
+        expect(response.statusCode).toBe 200
+        newSam = response.body
+
+        # Restore Sam
+        sam.set 'name', samsName
+        request.put {uri:getURL(urlUser + '/' + sam.id), json: sam.toObject()}, (err, response) ->
+          expect(err).toBeNull()
+          done()
+
 describe 'GET /db/user', ->
 
   it 'logs in as admin', (done) ->
@@ -267,3 +304,28 @@ describe 'GET /db/user', ->
       expect(response.statusCode).toBe(422)
       done()
     )
+
+  it 'can fetch myself by id completely', (done) ->
+    loginSam (sam) ->
+      request.get {url: getURL(urlUser + '/' + sam.id)}, (err, response) ->
+        expect(err).toBeNull()
+        expect(response.statusCode).toBe(200)
+        done()
+
+  it 'can fetch myself by slug completely', (done) ->
+    loginSam (sam) ->
+      request.get {url: getURL(urlUser + '/sam')}, (err, response) ->
+        expect(err).toBeNull()
+        expect(response.statusCode).toBe(200)
+        guy = JSON.parse response.body
+        expect(guy._id).toBe sam.get('_id').toHexString()
+        expect(guy.name).toBe sam.get 'name'
+        done()
+
+  # TODO Ruben should be able to fetch other users but probably with restricted data access
+  # Add to the test case above an extra data check
+
+  xit 'can fetch another user with restricted fields'
+
+
+