groupify
groupify copied to clipboard
Inverse of has_members missing as a feature?
Taking the active_record_spec example setup with only the relevant parts:
class User < ActiveRecord::Base
groupify :group_member
end
class Manager < User
end
class Group < ActiveRecord::Base
groupify :group
end
class Organization < Group
has_members :managers
end
With this, Organization
class has a has_many :managers
association.
How do I get an inverse_of
association of that as has_many :organizations
relation in the Manager
class?
I would expect there to be a has_groups
method available for group members.
There are a couple important things to understand about the current design of Groupify.
- A member can only belong to one type of group. This must be defined in the
groups
association. - When using inheritance in models, child classes inherit the association of their parent.
In the sample code above, User
and Manager
both share the same groups
association pointing to the Group
class. Since Organization
inherits from Group
, it is valid for a Manager
to belong to either a Group
or an Organization
.
If you want to enforce that a Manager
can only belong to an Organization
rather than a generic Group
, you can try adding this into the Manager
class:
groupify :group_member, group_class_name: 'Organization'
This should work because it will define a new groups
association on the Manager
class which would override the one defined in the User
class.
Let me know if this works for you, and I can add a note to the README about this use case.
I wrote some specs to clarify my intention.
has_groups :organizations association
First off we define a counterpart to the has_many :managers
association generated into Organization
class:
class Manager < User
has_many :organizations,
-> { uniq },
through: :group_memberships_as_member,
source: :group,
source_type: 'Group'
end
then I would expect the following spec to pass(which it does):
it "adds a manager to an organization" do
organization = Organization.create!
manager = Manager.create!
organization.add manager
expect(organization.managers).to include(manager)
expect(manager.organizations).to include(organization)
end
So my wish here would be to have a helper to define the association like with has_members
.
has_members :managers scoping issue
But then I noticed that organization.managers
does not list only managers, but all users:
it "adds only managers to an organizations managers" do
organization = Organization.create!
user = User.create!
manager = Manager.create!
organization.add manager
organization.add user
expect(organization.managers).to include(manager)
expect(organization.managers).to_not include(user) #TODO failing
end
Is this expected behaviour or is this a bug?
(Note that my custom organizations
association above has the same scoping issue)
So the desired behavior is to have a way to define a custom association which returns a subset of possible groups (filtered by type)? That seems reasonable.
The behavior with managers seems like a bug. I would expect the organization.managers
association to filter by users.type = 'Manager'
.
I have sketched out a suggestion for the helper at the commit referenced above by paraphrasing your work on the PR above.
How should the has_groups
method be named to avoid confusion about its usage? aka. it's not meant to restrict membership but to act as a counterpart to has_members
. Also it is only relevant with STI so that could be included somehow.
The commit still needs to be merged with your open PR above to ensure compatibility when a model is both a group and a member and also uses both has_groups
and has_members
.
@juhazi I started doing similar work last week to support allowing a member to belong to multiple groups (of any class, not just of one base class). Here is as far as I got: https://github.com/dwbutler/groupify/compare/feature/multiple-group-associations
It's very similar to the work you did. I ended up calling the method belongs_to_groups
instead of has_groups
, otherwise our work is nearly identical.
I abandoned this approach when I realized how much work it will be. It will essentially require a rewrite of the entire gem. Everywhere there is encoded the assumption that there is a single groups
association that contains all the groups. This would no longer be possible if there are multiple, independent, group associations that have to query different tables. All of the membership predicate methods, (such as shares_any_group?
) would have to be rewritten to query the group_memberships
table directly.
Furthermore, it's not clear to me that this approach will be compatible with how the Mongoid adapter is currently implemented, which might necessitate storing custom data structures instead of using the built in Mongoid relations.
In any case, we can implement a similar interface here, with the documented restriction that the membership classes must be subclasses of Group
in order for the gem to work correctly. And then in the future, perhaps the same interface can be reused with that restriction lifted.
So feature-wise my commit would check all the boxes. I'll change the name to yours or should it be belongs_to_group_subclasses
or something to reflect the restriction?
This feature should probably be merged after #48 because it has the same Mongoid issue and filtering should work correctly both ways.
I would stick to naming it belongs_to_groups
and document its usage and limitations. Reason being that I don't want the public interface to change too much (if at all) when the internal implementation changes.
@juhazi this is in #61