Comments (8)
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.
from groupify.
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)
from groupify.
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'
.
from groupify.
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
.
from groupify.
@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.
from groupify.
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.
from groupify.
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.
from groupify.
from groupify.
Related Issues (20)
- Cannot get single table inheritance work HOT 4
- nesting groups inside groups HOT 7
- Adding users to groups HOT 3
- Cannot properly use groupify when using prefixed table names HOT 1
- Removing Users From a Group [QUESTION] HOT 2
- `in_only_groups` doesn't match on group IDs HOT 4
- failed to add a named group HOT 14
- changed group_membership but it still looking for a table group_membership HOT 3
- Question re Group Names HOT 1
- Named Groups? (similar to GitHub organizations data model) HOT 2
- Relationship between two models HOT 2
- Rails 5.1.5 Support HOT 2
- Two Quick Questions David! (RE: Levels, and Multiple Groups) HOT 2
- Two More Quick Questions! (RE: Creating Groups, & Classes) HOT 2
- Cannot retrieve group members PG
- Cannot retrieve group members in pg HOT 5
- ActiveRecord::RecordInvalid: Validation failed: Group must exist HOT 1
- Use exists? instead of Array#include? method
- How to make subgroups with groupify
- Rails 7 Updates? HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from groupify.