Move the checks of users existence and activity to hubot-routines #164
Conversation
ya7on
force-pushed
the
user-active-list
branch
2 times, most recently
from
21ac052
to
b800b89
Compare
ya7on
force-pushed
the
user-active-list
branch
from
b800b89
to
dea94db
Compare
ya7on
changed the title
ya7on
force-pushed
the
user-active-list
branch
from
dea94db
to
620fdf6
Compare
src/birthder.js
Outdated
| const username = msg.message.user.name | ||
| const user = Object.values(brain).filter(item => item.name === username).shift() | ||
| const user = (await routines.getAllUsers(robot)).filter(item => item.name === username).shift() |
There was a problem hiding this comment.
Looks like you should use findUserByName instead.
src/birthder.js
Outdated
| userNames = users.map(user => `@${user.name}`) | ||
| message = `${userNames.join(', ')}` | ||
| const userNames = users.map(user => `@${user.name}`) | ||
| const message = `${userNames.join(', ')}` | ||
|
|
There was a problem hiding this comment.
I am not sure about it - first you delete block of variable declaration, then add it again in other place. May be will be less changes and they will be more clear if you change type of variables in block upper?
src/birthder.js
Outdated
| let users = [] | ||
|
|
||
| if (!await routines.isAdmin(robot, msg.message.user.name.toString())) { | ||
| if (!await routines.isAdmin(robot, msg.message.user.name)) { |
There was a problem hiding this comment.
This string looks like refactoring and not relative to the commit message refactoring.
src/birthder.js
Outdated
| } | ||
| } | ||
| const name = msg.match[2].trim() | ||
| const user = await routines.findUserByName(robot, name) |
There was a problem hiding this comment.
And again you combine small changes with moving block of variable declaration.
ya7on
force-pushed
the
user-active-list
branch
2 times, most recently
from
70900aa
to
1127c33
Compare
src/birthder.js
Outdated
| } else { | ||
| return msg.send(`I have never met ${name}.`) | ||
| } | ||
| }) | ||
|
|
||
| // Print the users names whose birthdays match the specified date. | ||
| robot.respond(routes.check, async (msg) => { | ||
| let allUsers | ||
| let date | ||
| let message | ||
| let users = [] |
There was a problem hiding this comment.
Now the variable users doesn't need the initial value.
src/birthder.js
Outdated
| @@ -204,16 +181,9 @@ | |||
| title = `First working days` | |||
| } | |||
|
|
|||
| let message | |||
There was a problem hiding this comment.
Leave message here to not redefine it lower.
src/birthder.js
Outdated
|
|
||
| if (!await routines.isAdmin(robot, msg.message.user.name.toString())) { | ||
| msg.send(utils.MSG_PERMISSION_DENIED) | ||
| return | ||
| } | ||
|
|
||
| name = msg.match[2].trim() | ||
| date = msg.match[3] |
There was a problem hiding this comment.
Let's don't rewrite date and name variables.
src/utils.js
Outdated
| users.push(bdayUser) | ||
| } | ||
| } | ||
| const targetDay = moment().add(-exp.BIRTHDAY_CHANNEL_TTL, 'day') |
There was a problem hiding this comment.
Let's don't refactor targetDay variable.
ya7on
force-pushed
the
user-active-list
branch
3 times, most recently
from
2fbc2fb
to
0d15a39
Compare
ya7on
force-pushed
the
user-active-list
branch
from
0d15a39
to
aea3376
Compare
close #158
close #161
Please make sure all below checkboxes in the Mandatory section have been checked before submitting your PR (also don't forget to pay attention to the Depending on Circumstances section)
Mandatory
Add something,Remove something,Fix something, etc.).Depending on Circumstances
Some of the items in the section become mandatory if you are a first-time contributor and/or your changes are relevant to the items.
README.md.AUTHORS.md.