search

hubot-archive / hubot-auth

Assign roles to users and restrict command access in other scripts
149 stars 54 forks source link

Add `groups` function to user object #20

Open msabramo opened 8 years ago

msabramo commented 8 years ago

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

Cc: @michaelansel

msabramo commented 8 years ago

Tests I believe were previously broken.

michaelansel commented 8 years ago

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

msabramo commented 8 years ago

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

patcon commented 8 years ago

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 commented 8 years ago

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...

patcon commented 8 years ago

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?

msabramo commented 8 years ago

@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?

michaelansel commented 8 years ago

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: github/hubot#985