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

Membership types #6

Merged
merged 55 commits into from Jun 18, 2014
Merged

Membership types #6

merged 55 commits into from Jun 18, 2014

Conversation

dwbutler
Copy link
Owner

@dwbutler dwbutler commented Apr 7, 2014

Implements membership types to allow members to belong to groups or named groups in a more specific way. This can be used to implement role-based authorization on groups of resources.

Fixes #3 and #4.

@dwbutler
Copy link
Owner Author

dwbutler commented Apr 7, 2014

Next steps are to implement the same functionality for Mongoid and do some refactoring. The API is pretty stable but may be subject to change.

See the following for some documentation of this feature:

https://github.com/dwbutler/groupify/tree/membership_type#membership-types

@auser
Copy link

auser commented Apr 7, 2014

This is looking great!

One feature I want is the ability to select the manager from the group, i.e. group.managers

@dwbutler
Copy link
Owner Author

dwbutler commented Apr 8, 2014

I thought about doing that as well. But there are some problems with it.

  • The membership types are dynamic and are not known in advance. This could be solved with method_missing and dynamic method creation, but it's a little bit hairy.
  • Membership types can be associated with any member model, and it's impossible for Groupify to automatically know which model you want. This could be solved with some configuration gunk, but at that point you might as well define the methods yourself.
  • A method of this sort makes some assumptions about how you're using Groupify that might not match how everyone will use it. For example, someone might add the membership type "626 Project Foobar 325" with the intention that everyone of the same membership type belongs to the same sub-group of the main group. In this case, a method such as group.project_foobar_325s wouldn't be obvious or make a lot of sense.
  • This might interfere if you're already defining a group.managers method when using STI or another gem such as Rolify.

A possibly generic solution would be to use method_missing to support a method such as group.manager_users. This way the membership type and the model class are both specified.

Another solution would be to document how to define the methods yourself or perhaps provide a macro to call such as has_members :managers, class_name: "User", as: :manager

@dwbutler
Copy link
Owner Author

dwbutler commented Apr 8, 2014

Another solution would be to add a custom method onto the member associations which would add a new scope. For example, group.users.as(:manager) would return only the users that have the manager membership type on this group. I feel like this might be the cleanest solution.

@dwbutler
Copy link
Owner Author

dwbutler commented Apr 8, 2014

Added the implementation, so group.users.as(:manager) should now work. Let me know if that's close enough to what you need.

@auser
Copy link

auser commented Apr 8, 2014

Awesome! Think this will get merged soon? Or should we just use your fork?

@auser
Copy link

auser commented Apr 8, 2014

By the way... was just reviewing the source. It's looking great. I think that groups.users.as() syntax works nicely.

@dwbutler
Copy link
Owner Author

dwbutler commented Apr 8, 2014

@auser Thanks! It's going to take a while to re-implement the same functionality for Mongoid. I would recommend for now that you try out the branch, make sure it meets your needs, and look for any subtle bugs. I wanted to do some refactoring as well, but I can hold off on that since you guys are waiting for the functionality.

@dwbutler
Copy link
Owner Author

dwbutler commented Jun 4, 2014

This functionality is just about complete. I made some changes to the design during the process of implementing this for Mongoid.

First, the group_memberships relation should be considered private and not accessed directly. This is because the model is implemented differently in ActiveRecord and Mongoid, so it can't be built or queried in the same way.

Second, I decided it makes the most sense to add a member to the group generically when they are added with a membership type. So:

group.add(user, as: 'employee')
user.in_group?(group) # => true
group.users.delete(user, as: 'employee')
user.in_group?(group) # => true
group.users.delete(user)
user.in_group?(group) # => false

dwbutler added a commit that referenced this pull request Jun 18, 2014
@dwbutler dwbutler merged commit 4e6becb into master Jun 18, 2014
@dwbutler
Copy link
Owner Author

Released Groupify 0.6.0.rc1 to rubygems. Let me know if you have any issues.

@dwbutler dwbutler deleted the membership_type branch August 27, 2014 03:31
@dwbutler
Copy link
Owner Author

The membership type functionality has now been released in version 0.6.0. Enjoy!

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.

Extend to deal with group managers/leaders?
2 participants