Avoid index_active_storage_attachments_uniqueness violation by attaching multiple files in a single call to attach

[jpawlyn]

Description

We recently tried to switch to using direct_upload: true on the Files field. However when creating a new record with multiple files, we got the error duplicate key value violates unique constraint "index_active_storage_attachments_uniqueness".

The fix for this is to attach files in a single call to attach instead of calling attach for each file.

Fixes # (issue)

Checklist:

• [ ] I have performed a self-review of my own code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

1. Step 1
2. Step 2

Manual reviewer: please leave a comment with output from the test if that's the case.

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit 17c2addb and detected 0 issues on this pull request.

View more on Code Climate.

[adrianthedev]

@camilodelvasto can we add a test to check for the bad behavior that was happening before? Also to check for nil values (to see that the compact_blank) works.

[jpawlyn]

Looking great! Thank you @jpawlyn for the contribution!

Good to merge after adding some tests

I haven't been able to re-create the issue in the Avo test suite I'm afraid but I have recreated it in the Active Storage test suite.

Looking at https://github.com/rails/rails/blob/ee661852313ff697d5f3a73a68aab99101ea3ead/activestorage/test/models/attached/many_test.rb#L417-L426 the call to attach is taking a single signed id. When I change that to 2 separate calls to attach ie

  test "attaching an existing blob from a signed ID to a new record" do
    User.new(name: "Jason").tap do |user|
      user.highlights.attach create_blob(filename: "funky.jpg").signed_id
      user.highlights.attach create_blob(filename: "town.jpg").signed_id
      assert_predicate user, :new_record?
      assert_equal "funky.jpg", user.highlights.first.filename.to_s

      user.save!
      assert_equal "funky.jpg", user.reload.highlights.first.filename.to_s
    end
  end

the output is:

Error:
ActiveStorage::ManyAttachedTest#test_attaching_an_existing_blob_from_a_signed_ID_to_a_new_record:
ActiveRecord::RecordNotUnique: SQLite3::ConstraintException: UNIQUE constraint failed: active_storage_attachments.record_type, active_storage_attachments.record_id, active_storage_attachments.name, active_storage_attachments.blob_id
    test/models/attached/many_test.rb:425:in `block (2 levels) in <class:ManyAttachedTest>'
    test/models/attached/many_test.rb:419:in `block in <class:ManyAttachedTest>'

When I switch the test to a single call to attach with multiple signed_ids, the test passes. ie

  test "attaching an existing blob from a signed ID to a new record" do
    User.new(name: "Jason").tap do |user|
      user.highlights.attach create_blob(filename: "funky.jpg").signed_id, create_blob(filename: "town.jpg").signed_id
      assert_predicate user, :new_record?
      assert_equal "funky.jpg", user.highlights.first.filename.to_s

      user.save!
      assert_equal "funky.jpg", user.reload.highlights.first.filename.to_s
    end
  end

I don't see anywhere warning against multiple calls to attach but examples do suggest just one call to attach should be used.

Not sure how easy it is to re-create this in the Avo test suite but please let me know if you have any suggestions.

[jpawlyn]

@Paul-Bob I tried writing a test but unfortunately it doesn't fail with the original code. Somehow my app running in production and the activestorage test suite re-creates the issue but not for the Avo test suite.

[Paul-Bob]

@Paul-Bob I tried writing a test but unfortunately it doesn't fail with the original code. Somehow my app running in production and the activestorage test suite re-creates the issue but not for the Avo test suite.

Does it fail on Avo local?

[jpawlyn]

@Paul-Bob I tried writing a test but unfortunately it doesn't fail with the original code. Somehow my app running in production and the activestorage test suite re-creates the issue but not for the Avo test suite.

Does it fail on Avo local?

When running my app locally, yes it does fail

[Paul-Bob]

When running my app locally, yes it does fail

Ok, thank you for your effort on this @jpawlyn. I'll have a look