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

Unique constraint violation on move_higher, move_lower, destroy #267

Closed mdesantis closed 7 years ago

mdesantis commented 7 years ago

acts_as_list 0.9.5.

I noticed that some issues related to unique constraints have been closed. Should acts_as_list play friendly with unique constraints now? Otherwise I could open some issues about, since move_higher, move_lower, destroy raise errors on my app.

zharikovpro commented 7 years ago

since move_higher, move_lower, destroy raise errors on my app.

Primary target was insert_at. Thus, methods you described can still have issues. They probably could be fixed based on that PR as example: #246 , see #insert_at update.

brendon commented 7 years ago

@mdesantis, If you feel up to it, have a go at doing a PR for this. @zharikovpro is correct in that you should be able to borrow the techniques used in insert_at to fix the problem. Please add tests to make sure the fixes are carried forward correctly.

mdesantis commented 7 years ago

Well, since I use PostgreSQL I actually solved the problem at db level, setting the unique constraint as DEFERRABLE INITIALLY DEFERRED. Here the context, someone might need it:

class Content::Page < ApplicationRecord
  acts_as_list scope: :content, sequential_updates: false
end

class SetContentPagesContentPositionUniqueIndexDeferrable < ActiveRecord::Migration[5.1]
  def up
    connection.execute <<-SQL
      ALTER TABLE content_pages
        ADD CONSTRAINT index_content_pages_on_content_id_and_position
          UNIQUE
          USING INDEX index_content_pages_on_content_id_and_position
          DEFERRABLE INITIALLY DEFERRED
    SQL
  end

  def down
    connection.execute <<-SQL
      ALTER TABLE content_pages
        DROP CONSTRAINT index_content_pages_on_content_id_and_position
    SQL

    add_index :content_pages, [:content_id, :position], unique: true
  end
end

As you can see I can even set sequential_updates: false, since the constraint check is deferred.

brendon commented 7 years ago

Ah ok :) I'm sure that'll help those that need it. Unless someone wants to put in a PR on this one I'll probably close it for now as it's not an overly common situation. Are you ok with that?

mdesantis commented 7 years ago

I'll close it for you ;) at least we have a nice fix for PostgreSQL... poor MySQL users! I'm so sad for them :DDD

brendon commented 7 years ago

lol I wish I could quit MYSQL