github/hubot — pull request #670 by templaedhel (2014-03-18 19:07:57+00:00)
Title: Userforid reconciliation

We were seeing strange failures with our hubot. I was able to trace it down to the `brain.userForId` function, which I then updated to solve the issue. I'm going to outline the failure and the changes made to fix it below. If I'm missing anything as far as why this function was originally coded like it was then I can look at fixing the issue in other ways, but in the context I can see this fix is fairly straightforward.

### The Issue
Ever since we started using the brain to store info on users we noticed a strange failure where new users weren't having @mention_names assigned.

What appears to be the problem is initially when users are created (in hipchat) they don't have @mention_names. Hubot adds them to the brain as soon as they exist. Then when @mention_name is set the `userForId` function is called again it doesn't update the brain because of the conditions on [line 109](https://github.com/templaedhel/hubot/compare/github:master...templaedhel:userforid-reconciliation?expand=1#diff-3f212f83b49bd9f3b418259c1f06c0cfL109).

### The Fix
I removed all the logic around if and when to update users in the brain. Now `userForId` will always return the user for the given ID, as well as updating it with any additional attributes passed in.

This fixes our issue.

### Several Questions Remain
- Is there a reason behind the complicated logic around only updating if `options.room` isn't set on the existing user? Tracing back through git didn't dig up anything interesting. I also have trouble seeing how these changes will break any logical uses of this function
- Is there a more preferable extend function than using the coffeescript extend function? Usually I'd use something like lodash or underscore, or just write a helper function. Unclear what the convention hubot follows would be.