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

Support has_many :through instead of just only has_and_belongs_to_many #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kidlab
Copy link

@kidlab kidlab commented Aug 31, 2013

has_and_belongs_to_many works well for most of use-cases, but it's annoying if we want to:

  • Use a custom join table instead of having to follow the rule "#{self.to_s.tableize.gsub(/\//, "_")}_#{self.role_table_name}"
  • Have more fields in the join table, e.g: created_at and updated_at

For the above reasons, I added one more option called has_many_through to allow developer can use a customized join table by has_many :through association.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.17%) when pulling 5c926e9 on kidlab:master into 45de8cf on EppO:master.

@tmaier
Copy link

tmaier commented Oct 2, 2013

+1 for using has_many :through instead of habtm.

Whats the reason for has_many :through as an option?
As far as I know, habtm is considered simpler but not recommended choice in general.

My use case:
I want to list all licensees and also be able to add new licensees this way.
as resourcify, I get an ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection error

  resourcify
  has_many :licensees,
           class_name: 'User',
           through: :roles,
           source: :users,
           conditions: { roles: { name: 'licensee' } }

has_and_belongs_to_many :roles, rolify_options
# Option to support has_many :through
has_many_through_table = options.delete(:has_many_through)
if has_many_through_table

This comment was marked as resolved.

@EppO
Copy link
Member

EppO commented Jan 28, 2014

Thanks for this PR. I plan to switch from HABTM to has_many :trough in rolify 4.0.
I will eventually add this as a opt-in option in 3.x branch (most likely in 3.5)

@enricostano
Copy link

👍

@enricostano
Copy link

@kidlab this will allow us to add validations on Role, right?

# Option to support has_many :through
has_many_through_table = options.delete(:has_many_through)
if has_many_through_table
self.role_join_table_name = has_many_through_table
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I need to use that variable here :)

@kidlab
Copy link
Author

kidlab commented Mar 10, 2014

@enricostano Yup, but the main purpose of this is to allow us use a custom joining table (e.g, users_roles) to keep track information like create_at, and more in that table.

@enricostano
Copy link

👌

@enricostano
Copy link

is anybody using this branch in a production environment? :shipit:

@wldcordeiro wldcordeiro modified the milestone: rolify 3.5 Jul 3, 2014
@wldcordeiro wldcordeiro modified the milestones: rolify 3.5, rolify 4.0 Jan 8, 2015
@wldcordeiro
Copy link
Member

@kidlab Is this PR all setup and working? I see that the Travis build passes but it's been a while so I was wanting some more info. This would be a good feature to include in Rolify.

@werleo
Copy link

werleo commented Jun 15, 2016

@wldcordeiro I made quick look on blame in changed method.
It looks like last change there was made on 27 Aug 2013.
PR was made 31 Aug 2013 so everything should be fine after merge.

@hut8
Copy link

hut8 commented Jul 4, 2016

Any progress on this? What's holding it up?

@Genkilabs
Copy link

Genkilabs commented Oct 19, 2017

Anyone still interested in this? I would love to create a relation and scopes :through roles.
Perhaps with syntax like:

class User < ApplicationRecord
  has_many :owned_accounts, -> { where(:roles => {:name => :owner}) }, 
           :through => :roles, 
           :source => :resource, 
           :source_type => :account, 
           :class_name => 'Account'

Boo, this was wrong, see next comment

@Genkilabs
Copy link

Scratch my last comment, you can remove class_name, but must be sure source_type is capitalized to match the model name and it seems to create good SQL:

class User < ApplicationRecord
  has_many :owned_accounts, -> { where(:roles => {:name => :owner}) }, 
           :through => :roles, 
           :source => :resource, 
           :source_type =>  :Account

@Durev
Copy link

Durev commented Dec 20, 2020

Hi guys,
Is this PR still relevant?
Is more development or testing needed?
How can we help to get this shipped?

@tmaier
Copy link

tmaier commented Dec 20, 2020

Long time has past, but i think this is still relevant

@yshmarov
Copy link

SUMMARY:

user.rb

has_many :posts, through: :roles, source: :resource, source_type:  :Post
has_many :moderated_posts, -> { where(roles: {name: :moderator}) }, through: :roles, source: :resource, source_type:  :Post

let's you do

@user.posts 
=> all the posts where the @user has a role
@user.moderated_posts
=> all the posts where the @user has a moderator

post.rb

has_many :users, through: :roles, class_name: 'User', source: :users
has_many :moderators, -> { where(:roles => {name: :moderator}) }, through: :roles, class_name: 'User', source: :users

let's you do

@post.users
=> all the users that have a role in this post
@post.moderators
=> all the users that have a moderator role in this post

This is something that should be 100% included in the gem, or in the description

@alec-c4
Copy link

alec-c4 commented Jul 21, 2021

Hey! Any news with this PR?

@thomas-mcdonald
Copy link
Member

I'd like to ship this! I'm not sure of the migration path for folks with existing Rolify instances or if we need to change generators. I think I would rather we default to has_many through if possible.

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.