Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add groups function to user object #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msabramo
Copy link

Useful so that hubot-auth can be used with hubot-approval

Cc: @michaelansel

@msabramo
Copy link
Author

Tests I believe were previously broken.

@michaelansel
Copy link

Looks good to me. Do we have an owner for hubot-auth?

@msabramo
Copy link
Author

There's a commit 19 days ago from @patcon, so maybe him?

@patcon
Copy link
Member

patcon commented Oct 25, 2015

Cool, thanks! Yep, I still have merge perms.

Have you been actively running this code? Also, will it need updated version constraints in package.json, ie for the earliest hubot version it will work with?

@msabramo
Copy link
Author

Yeah, I've been running this code with our hubot instance at work for a few days.

It uses robot.listenerMiddleware so it sounds like from https://github.com/github/hubot/blob/master/CHANGELOG.md that this will need at least hubot 2.14.0. I'll go ahead and update the package.json to reflect that...

@msabramo msabramo force-pushed the add_groups_function_to_user_object branch from 4bf5e17 to 1a5cfaa Compare October 25, 2015 13:45
@patcon
Copy link
Member

patcon commented Oct 25, 2015

Thanks! Rats... mock-adaptor doesn't seem to work against that version of hubot. Haven't been using hubot lately -- any suggestions on the current best-practice for hubot testing?

robot.listenerMiddleware (context, next, done) ->
context.response.message.user.groups = (cb) ->
cb(robot.auth.userRoles(context.response.message.user))
next()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires at least v2.15.0.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Updated package.json to require "^2.15.0".

Useful so that hubot-auth can be used with
[hubot-approval](https://github.com/michaelansel/hubot-approval)

Updated `package.json` so that required hubot version goes from ^2.7.5
to ^2.15.0
@msabramo msabramo force-pushed the add_groups_function_to_user_object branch from 1a5cfaa to 97871ff Compare October 30, 2015 22:05
@msabramo
Copy link
Author

@michaelansel: Do you have recommendations for how @patcon can fix the tests?

It looks like https://github.com/michaelansel/hubot-approval/blob/master/test/approval-test.coffee is using a combination of chai, sinon, and sinon-chai. I don't know what those things are because I'm new to NodeJS, but I saw hubot-approval using them, so maybe they're the current state of the art?

@msabramo msabramo mentioned this pull request Oct 30, 2015
@michaelansel
Copy link

Yup, it's an open issue on the core. Up to @patcon to decide how to handle failing tests for his project.

For more conversation on testing patterns, take a look at the open issue on the core: hubotio/hubot#985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants