Testing #2
Testing #2
Conversation
… API to take in room Ids when adding users to rooms
…ed to befoe wrapper, stub created for the Metero.users.findOne() call
…serId instead of passing parameter, moved many updates to the model layer
…iles and created additional stubs
…actored many of the describe statetements, added proper meteor Error messages as well as custom validations to models
…ted several ner erros on methods
| }; | ||
|
|
||
| Chatter.configure = function (opts) { | ||
| _.extend(this.options, opts); |
There was a problem hiding this comment.
Certain that this will be set to the right context, and not to the caller's context? Might need to extend Chatter.options instead
There was a problem hiding this comment.
It does seem to work after testing it!
| } | ||
|
|
||
| return Chatter.UserRoom.upsert({ | ||
| userId: chatterUserId, |
There was a problem hiding this comment.
Which userId is this? If you name chatterUserId the same as Meteor's own userId you will quickly confuse yourself and the users of chatter. You might want to rename all variables that are chatterUserIds to chatterUserId to avoid this confusion
There was a problem hiding this comment.
Good point. This has been on my list for while. I have changed all variables to chatterUserId, and chatterUser. Will that suffice or should I also rename the attributes of the chatter models, so that when creating new instances in becomes this:
return Chatter.User.upsert({
- chatterUserId: chatterUserId,
- roomId: roomId
});
rather than this:
return Chatter.User.upsert({
- userId: chatterUserId,
- roomId: roomId
});
There was a problem hiding this comment.
Why would a chatter user have a chatterUserId? Wouldn't it be just _id in this case? And you also don't need to duplicate the variable name if key and value are the same.
There was a problem hiding this comment.
The goal is to distinguish the two types of userIds that belong to different collections.
There was a problem hiding this comment.
Understood. Just realized it was a stupid question. :)
…astronomy and es6 deconstructs
…catch empty or undefined variables
… checkers for undefined or null variables
…none objects of describe statements
| @@ -1,7 +1,8 @@ | |||
| Chatter = { | |||
| options: { | |||
| messageLimit: 30, | |||
| nickProperty: "username", | |||
| roomLimit: 15, | |||
There was a problem hiding this comment.
Is this the max number of rooms a user can have, or a limit to the number of users in a room, or messages in a room?
There was a problem hiding this comment.
I removed this. Now only a set number of rooms are loaded initially, and all rooms loaded after user clicks on "show more button". This set number of initially loaded rooms should be customizable through chatter options.
There was a problem hiding this comment.
I guess same thinking can be applied to messages, like whatsapp, if you scroll far back and want to see older messages you can load more messages
…ces, user variable changed to chatterUser to avoid confusion, updated all methods names, removed database cleaner, removed need for multiple before statements...
…sts, alos renamed count attribute to unreadMsgCounter
Unit tests for chatter core