brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

acts_as_list methods on has_many through #308

Closed lsarni closed 6 years ago

lsarni commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

acts_as_list scope: :update

Shouldn't this be acts_as_list scope: :collection ?

undefined method `move_to_top' for #

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 commented 6 years ago

@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 commented 6 years ago

Glad it's working.

brendon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.