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

Groupify v1.0.0: Better STI/polymorphism and various fixes #61

Open
wants to merge 206 commits into
base: master
Choose a base branch
from

Conversation

joelvh
Copy link
Collaborator

@joelvh joelvh commented May 10, 2017

This has turned into an extensive refactor to improve STI/polymorphism. It also attempts to DRY up some aspects of the code.

  • Drops Rails 4.0 and 4.1 support
  • Adds has_group and has_member class methods to configure associations (for STI)
  • Adds ability for polymorphic groups and members with polymorphic_groups and polymorphic_members
  • Adds ability to customize default groups and members association names
  • Adds ability to customize default member class
  • Adds membership_types_for_* method on groups and group members to get list of membership types
  • Adds support for multiple membership types when querying using as(membership_types) or helper methods like user.in_any_group?(group1, group2, as: membership_types) (using SQL IN(...)/OR query)
  • Updated logic when appending a group or member to the related association.
  • Utilize dynamic modules to add common logic to groups and group members (ActiveRecord)
  • Fixes an issue of duplicate membership records if no membership type is specified.
  • Fixes properly clear association caches after merge! on groups and delete or destroy on associations
  • Fixes additional bugs for cases that were not covered (see new tests in test suite)

Please provide feedback.

Thanks!

@joelvh joelvh changed the title Handle polymorphic groups a little better Handle group subclasses and STI/polymorphism better, along with some consistency fixes May 10, 2017
@joelvh joelvh force-pushed the feature/polymorphic_adding branch 2 times, most recently from 59626c5 to 1a598c9 Compare May 10, 2017 21:52
@dwbutler
Copy link
Owner

This looks promising, but I shall want to see tests in order to have confidence that the solution here works.

Also, it seems that validate! wasn't added until Rails 4.2, but this gem supports 4.0+.

@joelvh
Copy link
Collaborator Author

joelvh commented May 19, 2017

@dwbutler changed out the validate! method for some plain Ruby. Also ended up refactoring the association extension modules to share the repeated logic, so that consistency can be more closely maintained.

What are some specific tests you would want to see?

@joelvh
Copy link
Collaborator Author

joelvh commented May 19, 2017

@dwbutler added some tests. Some direction on what tests to add is welcome. Thanks.

@joelvh
Copy link
Collaborator Author

joelvh commented Jun 3, 2017

@dwbutler Wanted to see if you had a chance to look at this? Thanks.

@dwbutler
Copy link
Owner

dwbutler commented Jun 4, 2017

@joelvh Sorry been a crazy few weeks. Definitely planning to get to this soon.

@joelvh
Copy link
Collaborator Author

joelvh commented Jun 5, 2017

@dwbutler got it. Will stand by.

@joelvh
Copy link
Collaborator Author

joelvh commented Jun 30, 2017

hi @dwbutler saw you were able to make some updates recently. Did you get a chance to look at these updates?

@joelvh joelvh force-pushed the feature/polymorphic_adding branch from d180018 to c375da3 Compare June 30, 2017 21:40
@joelvh
Copy link
Collaborator Author

joelvh commented Sep 25, 2017

@dwbutler wanted to see if you had any more time to review this?

@joelvh
Copy link
Collaborator Author

joelvh commented Nov 3, 2017

@dwbutler checking in on your review of this. Anything I can do to help make the review easier for you?

@joelvh joelvh changed the title Refactor for v1.0: Better STI/polymorphism and some consistency fixes Groupify v1.0.0 RC: Better STI/polymorphism and various fixes Nov 3, 2017
@joelvh joelvh changed the title Groupify v1.0.0 RC: Better STI/polymorphism and various fixes Groupify v1.0.0: Better STI/polymorphism and various fixes Nov 3, 2017
@dwbutler
Copy link
Owner

dwbutler commented Nov 7, 2017

Hey @joelvh thanks for following up. Going through some personal stuff right now so I haven't had time to spend on open source like I usually do.

I pretty much have full trust in you at this point. So when I have time, I'll figure out how to give you full access to the project so you can merge and release it.

@joelvh
Copy link
Collaborator Author

joelvh commented Nov 10, 2017

@dwbutler sorry to hear that. I'm happy to help maintain. I'll try to get a list of devs who might be interested in testing a Release Candidate, unless you have a list of people that might be interested.

@dwbutler
Copy link
Owner

dwbutler commented Jan 9, 2018

@joelvh Sorry for the long long delay. You should have full access now to this repository and to push the gem on rubygems. Welcome aboard!

@rposborne
Copy link
Contributor

Would love to see this merged. Anything I can do to help?

@dwbutler
Copy link
Owner

dwbutler commented Mar 8, 2018

@rposborne Would you be interested in helping maintain Groupify as well?

@rposborne
Copy link
Contributor

@dwbutler I don't have much time to offer, I do offer "gem gardening" though, If you just need someone else with the keys to make sure those who are eager have a point of contact. But I cannot take on any real responsibility :/

@tomekr
Copy link

tomekr commented Apr 15, 2020

would be great to see this merged! Curious if you ended up getting access @joelvh ?

@monroemann
Copy link

Is this gem still being monitored and updated?

@dwbutler
Copy link
Owner

I am no longer maintaining this gem. I don't write Ruby any more and don't use this gem, so it's difficult for me to stay involved in its maintenance. If anyone is interested in maintaining it, I would recommend forking it to a new organization. I can facilitate this if anyone is willing to take on the mantle.

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

Successfully merging this pull request may close these issues.

5 participants