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

acts_as_list methods on has_many through #308

Closed
lsarni opened this issue Mar 22, 2018 · 10 comments
Closed

acts_as_list methods on has_many through #308

lsarni opened this issue Mar 22, 2018 · 10 comments

Comments

@lsarni
Copy link

lsarni commented Mar 22, 2018

I'm using a has_many through relationship with a join table, like it was explained in some other issues and Stackoverflow.

This is how my models look like:

class Update < ActiveRecord::Base
  has_many :links, through: 'update_links'
  has_many :update_links
end

class Link < ActiveRecord::Base
  has_many :collections, through: 'update_links', foreign_key: 'update_id'
  has_many :update_links
end

class UpdateLink < ActiveRecord::Base
  default_scope { order :position }
  belongs_to :collection, foreign_key: 'update_id', class_name: 'Update'
  belongs_to :link
  acts_as_list scope: :update
end

But if I try to use any of the acts_as_list methods like this:

update.links.last.move_to_top
  Link Load (1.1ms)  SELECT  "links".* FROM "links" INNER JOIN "update_links" ON "links"."id" = "update_links"."link_id" WHERE "update_links"."update_id" = $1  ORDER BY "update_links"."position" DESC, update_links.position DESC LIMIT 1  [["update_id", 1]]
NoMethodError: undefined method `position' for #<Link:0x007fcd53f93210>

I saw this workaround but since it's quite an old post I was wondering if the library has been extended to be usable on this cases.

I made some changes to the workaround so it works with all the methods and had to add a primary_key to my join table (since many of the list methods call it):

  def method_missing(symbol, *args, &block)
    if acts_as_list_method?(symbol)
      pass_method_to_update_links(symbol, args.shift, *args)
    else
      super
    end
  end

  def pass_method_to_update_links(symbol, link, *args)
    raise "A link is needed" unless link.kind_of? Link
    move_link = self.update_links.find_by(link: link)
    move_link.send(symbol, *args) if move_link
  end

  def acts_as_list_method?(symbol)
    ActiveRecord::Acts::List::InstanceMethods.instance_methods.include?(symbol.to_sym)
  end

That way instead of doing list_item.move_higher I do collection.move_higher(list_item).

But if there is a built in way of doing this I would much prefer that.

@brendon
Copy link
Owner

brendon commented Mar 22, 2018

Hi @lsarni, that's a mind bender. Do you only have a position column on the join table? or also on the links table?

acts_as_list assumes the column exists on the table that the model represents. And you're right, I'm pretty sure it also assumes a primary key column, though that might be more via rails.

@lsarni
Copy link
Author

lsarni commented Mar 22, 2018

I have the position column on the join table, it wouldn't make sense for it to be on the links table since a link can be in a different position depending on the update.

And the primary key is requested in quite a few of the list methods defined by the library, for example higher_items here

With the stated workaround I managed to get al the methods working correctly. Maybe, if there isn't an out of the box solution, adding a flag to state that we are working on a join table and include this behaviour could be a good improvement.

@brendon
Copy link
Owner

brendon commented Mar 22, 2018

I see, what if you just called the methods directly on the join association:

link = update.links.last # Or whatever you want to select
update.update_links.where(link: link).first.move_to_top

It's a bit messy, but should do the trick. What do you think?

@lsarni
Copy link
Author

lsarni commented Mar 23, 2018

I have tried it out, and I get this error:

NoMethodError:   UpdateLink Load (0.7ms)  SELECT "update_links".* FROM "update_links" WHERE "update_links"."update_id" = $1 AND "update_links"."link_id" = 10805  ORDER BY "update_links"."position" ASC  [["update_id", 1]]
undefined method `move_to_top' for #<UpdateLink::ActiveRecord_AssociationRelation:0x007f9bb07902e0>
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/activerecord-4.2.5/lib/active_record/relation/delegation.rb:136:in `method_missing'
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/activerecord-4.2.5/lib/active_record/relation/delegation.rb:99:in `method_missing'
	from (irb):11
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/railties-4.2.5/lib/rails/commands/console.rb:110:in `start'
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/railties-4.2.5/lib/rails/commands/console.rb:9:in `start'
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/railties-4.2.5/lib/rails/commands/commands_tasks.rb:68:in `console'
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/railties-4.2.5/lib/rails/commands/commands_tasks.rb:39:in `run_command!'
	from /Users/lucia/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/railties-4.2.5/lib/rails/commands.rb:17:in `<top (required)>'
	from bin/rails:4:in `require'
	from bin/rails:4:in `<main>'

The same happens for the other methods.

@swanandp
Copy link
Contributor

swanandp commented Mar 23, 2018

acts_as_list scope: :update

Shouldn't this be acts_as_list scope: :collection ?

undefined method `move_to_top' for #UpdateLink::ActiveRecord_AssociationRelation:0x007f9bb07902e0

You need update.update_links.where(link: link).first.move_to_top

.where returns an collection object (it could well be a collection of one item, as in this case), but to get to the actual record, you need to call .first.

@lsarni
Copy link
Author

lsarni commented Mar 23, 2018

@swanandp you're right, I was missing the first.

It works regardless of whether the scope is :update or :collection.

It is pretty messy, but it's is exactly what the workaround does so I'm going to keep that on my project so it looks neater. But feel free to close the issue.

@swanandp
Copy link
Contributor

Glad it's working.

@swanandp swanandp reopened this Mar 23, 2018
@brendon
Copy link
Owner

brendon commented Mar 23, 2018

I think the scope should be :update since that's the foreign key name (update_id). That's for spotting my error @swanandp :D

@lsarni
Copy link
Author

lsarni commented Apr 2, 2018

I just noticed that this won't work for insert_at since in this case update.update_links.where(link: link) returns nil because the link isn't part of the list yet.

@brendon
Copy link
Owner

brendon commented Apr 3, 2018

Instead of using insert_at just specify the position you want to create the record at. Other items will be shuffled around accordingly when the item is created.

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

No branches or pull requests

3 participants