Giter VIP home page Giter VIP logo

Comments (8)

dwbutler avatar dwbutler commented on June 2, 2024

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.

juhazi avatar juhazi commented on June 2, 2024

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.

dwbutler avatar dwbutler commented on June 2, 2024

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.

juhazi avatar juhazi commented on June 2, 2024

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.

dwbutler avatar dwbutler commented on June 2, 2024

@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.

juhazi avatar juhazi commented on June 2, 2024

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.

dwbutler avatar dwbutler commented on June 2, 2024

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.

joelvh avatar joelvh commented on June 2, 2024

@juhazi this is in #61

from groupify.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.