brendon / acts_as_list

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

move_higher and move_lower need to be ran twice #340

Closed 7vs closed 5 years ago

7vs commented 5 years ago

Hi, I have an issue with acts_as_list in a simple has_many relationship. When I try to update the position of items using the provided method I get an error: 'position taken', but if I run it again, I've got what was expected. Any thoughts?

@messages={:position=>["translation missing: eu.activerecord.errors.models.option.attributes.position.taken"]}, @details={:position=>[{:error=>:taken, :value=>1}]}>

here is the update action:

irb(main):057:0> tw.move_lower
  Option Load (3.3ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 1) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
   (0.4ms)  BEGIN
  Option Load (2.9ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 1) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
  Option Load (3.0ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 1) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
  Menu Load (3.4ms)  SELECT  "menus".* FROM "menus" WHERE "menus"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  Option Exists (7.3ms)  SELECT  1 AS one FROM "options" WHERE "options"."position" = $1 AND ("options"."id" != $2) AND "options"."menu_id" = $3 LIMIT $4  [["position", 1], ["id", 1], ["menu_id", 1], ["LIMIT", 1]]
  Option Exists (2.6ms)  SELECT  1 AS one FROM "options" WHERE "options"."position" = $1 AND ("options"."id" != $2) AND "options"."menu_id" = $3 LIMIT $4  [["position", 2], ["id", 2], ["menu_id", 1], ["LIMIT", 1]]
   (0.2ms)  COMMIT
=> false
irb(main):058:0> tw.move_lower
  Option Load (6.3ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 2) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
   (0.5ms)  BEGIN
  Option Load (2.4ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 2) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
  Option Load (2.7ms)  SELECT  "options".* FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" >= 2) AND ("options".id != 2) ORDER BY "options"."position" ASC LIMIT $2  [["menu_id", 1], ["LIMIT", 1]]
  Menu Load (3.3ms)  SELECT  "menus".* FROM "menus" WHERE "menus"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  Option Exists (2.6ms)  SELECT  1 AS one FROM "options" WHERE "options"."position" = $1 AND ("options"."id" != $2) AND "options"."menu_id" = $3 LIMIT $4  [["position", 1], ["id", 1], ["menu_id", 1], ["LIMIT", 1]]
  Option Exists (2.7ms)  SELECT  1 AS one FROM "options" WHERE "options"."position" = $1 AND ("options"."id" != $2) AND "options"."menu_id" = $3 LIMIT $4  [["position", 3], ["id", 2], ["menu_id", 1], ["LIMIT", 1]]
  SQL (4.9ms)  UPDATE "options" SET "position" = $1, "updated_at" = $2 WHERE "options"."id" = $3  [["position", 3], ["updated_at", "2019-02-21 07:30:27.674229"], ["id", 2]]
   (2.2ms)  SELECT COUNT(*) FROM "options" WHERE "options"."menu_id" = $1 AND ("options"."position" = 3)  [["menu_id", 1]]
   (0.8ms)  COMMIT
=> true
brendon commented 5 years ago

Hi @7vs, looks like you have a uniqueness constraint? Take a look at the sequential_updates feature that will shuffle things around atomically so that you don't end up setting two positions the same while the shuffle happens.

7vs commented 5 years ago

Hi @brendon, thanks for your reply, I've seen thesequential _updates features now and I've removed the uniqueness constraint, delegating that work to the gem: validates_uniqueness_of :position, scope: :menu_id Thank you.

brendon commented 5 years ago

Hi @7vs, you should be able to keep the uniqueness constraint (on the database if you want) and it should all still work. That's what it was designed for :)

swanandp commented 5 years ago

I've removed the uniqueness constraint

@7vs Oops, this is a bad idea. Constraints should be enforced on the database level. sequential_updates will work constraints.

7vs commented 5 years ago

@7vs Oops, this is a bad idea.

Yes, @swanandp I figured it out after testing it properly. I managed to return back my constraint so everything is working smoothly now. Thank you both :)