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 pundit for user roles #76

Merged
merged 5 commits into from
Oct 20, 2017
Merged

Add pundit for user roles #76

merged 5 commits into from
Oct 20, 2017

Conversation

jacobherrington
Copy link
Contributor

Refs #65 ( and #66 )

User roles still need to be assigned manually, but the models exist to assign them. Any role can be added in seeds.rb and then assigned in the console.

@mobjohnson
Copy link
Collaborator

mobjohnson commented Oct 17, 2017

Thanks for contributing!!!

Can you add tests that show the functionality, using existing User and Organization instances? This is not just to test it, but to show other coders how this should be used. Something like an Org Admin can do this. A non-Org admin can not do that. Same for the Charter Admin. Not every use case, but more than what you presently have.

Make sure you use existing User and Organization instances, or the Google Maps API will be mean to you!!!

@jacobherrington
Copy link
Contributor Author

jacobherrington commented Oct 20, 2017

@mobjohnson I added some tests for the models. This PR doesn't actually implement any Pundit policies (except those generated in ApplicationPolicy.rb but those aren't being used atm), it just sets up the infrastructure to do so. Specifically fulfilling #65 and #66, but not the issues depending on them. I'm happy to work those too, but I think they should come after this PR so that anyone familiar with Pundit can also contribute.

@mobjohnson
Copy link
Collaborator

@jacobherrington Thanks for adding some more tests, and agree further testing would be in another story.

@mobjohnson mobjohnson merged commit 2b8790b into rubyforgood:master Oct 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants