acts_as_list icon indicating copy to clipboard operation
acts_as_list copied to clipboard

Add byebug gem and failing specs for bulk update with nested attributes

Open singhdurgesh opened this issue 1 year ago • 8 comments

Adding the failing specs for bulk update with nested attributes

singhdurgesh avatar Aug 27 '22 18:08 singhdurgesh

Hi @singhdurgesh, can you give me a quick rundown on the issue? :)

brendon avatar Aug 28 '22 23:08 brendon

Hi,

When we use acts as list with the scope of a foreign key and we have added the uniqueness constraint on position and scoped class id.

Envoirnment: Database: Postgres

Let's say, we have a Page model. and it has many items. And to create/update/destroy, nested attributes are being used.

class Page < ActiveRecord::Base
  attr_accessor :label
  has_many :items, dependent: destroy
   
  accepts_nested_attributes_for :items
end

class Item < ActiveRecord::Base
  attr_accessor :detail, :page_id, :position
  belongs_to :page

  acts_as_list scope: :page
end

We have unique constraint on the items table -> [page_id, position], to avoid the duplication issue.

We create a page with few items.

page = Page.create(label: "ABC Poster", items_attributes: [{detail: 'Sec 1', position: 1}, {'detail: 'Sec 2', position:  2}, {detail: 'Sec 3', position: 3}]
item_position1 = page.items.find_by(position: 1)
item_position2 = page.items.find_by(position: 2)
item_position3 = page.items.find_by(position: 3)

Here, we try to shuffle the position of items, error is being raised.

page.update(items_attributes: [{id: item_position1.id, position: 2}, {id: item_position2.id, position: 3}, {id: item_position3.id, position: 1}]

Exception:
ScopedWithUserDefinedUniqueConstraint#test_scope_with_user_defined_unique_constraint:
ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint "scoped_parent_id_position_unique"
DETAIL:  Key (scoped_parent_id, "position")=(1, 2) already exists.

    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:768:in `exec_params'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:768:in `block (2 levels) in exec_no_cache'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/concurrency/share_lock.rb:187:in `yield_shares'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/dependencies/interlock.rb:41:in `permit_concurrent_loads'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:767:in `block in exec_no_cache'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.3.1/lib/active_record/connection_adapters/abstract_adapter.rb:765:in `block in log'
    /Users/bluesapling05/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.3.1/lib/active_support/notifications/instrumenter.rb:24:in `instrument'

singhdurgesh avatar Aug 29 '22 01:08 singhdurgesh

Thanks @singhdurgesh, check out sequential_updates: https://github.com/brendon/acts_as_list#more-options

Does this help? :)

brendon avatar Aug 29 '22 02:08 brendon

No, sequential_updates doesn't help in this case.

singhdurgesh avatar Aug 29 '22 02:08 singhdurgesh

Hi @singhdurgesh, does it work if you update only one item record at a time with nested attributes? The gem isn't really built to cope with multiple repositioning all at once. When I do this (say just rewriting all positions in a scope for a list) I just use update_all which avoids any interaction with acts_as_list. acts_as_list is supposed to shuffle things for you when you move something but it looks like you're just trying to set all positions again which means you probably don't need acts_as_list in that sense. What do you think?

brendon avatar Aug 29 '22 21:08 brendon

Hi @brendon, with nested attributes, by default Rails saves the nested objects one by one. That is why there is duplication in the database and raising exception. Also, in the following case exception is being raised


class ListObject < ActiveRecord
  attr_accessor :position
end

# position has uniquness constraint on the database.

list_object1 = ListObject.create(position: 1)
list_object2 = ListObject.create(position: 2)

list_object1.update(position: 2)
raises ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR

I looked up into this issue and I think the solution would be around a before_save callback where we will be repositioning the other objects if position is already taken with the current scope.

singhdurgesh avatar Aug 30 '22 15:08 singhdurgesh

I've actually implemented a simple concern (well it started out that way) that implements a lot of what acts_as_list does. It's not a gem yet but the gist is here: https://gist.github.com/brendon/d6cfd60cb5e70dc77a15a2476f04d279

I wanted to have a uniqueness constraint too and also have complex scopes. I've not tried nested attributes with it. Would be interested to see how it works for you?

brendon avatar Aug 30 '22 21:08 brendon

Thanks a lot @brendon!. I'll check it out. :)

singhdurgesh avatar Aug 31 '22 10:08 singhdurgesh