diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 5c5da551ae4fbc714c591ea67df4e211a8e94dc9..75eb63f0b404452ab31d836bd8c6de81ef512b8b 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -43,18 +43,26 @@ module ActiveStorage mattr_accessor :logger mattr_accessor :verifier + mattr_accessor :variant_processor, default: :mini_magick + mattr_accessor :queues, default: {} + mattr_accessor :previewers, default: [] - mattr_accessor :analyzers, default: [] - mattr_accessor :variant_processor, default: :mini_magick + mattr_accessor :analyzers, default: [] + mattr_accessor :paths, default: {} - mattr_accessor :variable_content_types, default: [] + + mattr_accessor :variable_content_types, default: [] + mattr_accessor :binary_content_type, default: "application/octet-stream" mattr_accessor :content_types_to_serve_as_binary, default: [] - mattr_accessor :content_types_allowed_inline, default: [] - mattr_accessor :binary_content_type, default: "application/octet-stream" + mattr_accessor :content_types_allowed_inline, default: [] + mattr_accessor :service_urls_expire_in, default: 5.minutes + mattr_accessor :routes_prefix, default: "/rails/active_storage" + mattr_accessor :replace_on_assign_to_many, default: false + module Transformers extend ActiveSupport::Autoload diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb index ae7f0685f23135578b0485355ffb2325db6644ac..06864a846fb71a9e90d4b5cff3a60a3778e835fd 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -93,12 +93,19 @@ def #{name} end def #{name}=(attachables) - attachment_changes["#{name}"] = - if attachables.nil? || Array(attachables).none? - ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self) - else - ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables) + if ActiveStorage.replace_on_assign_to_many + attachment_changes["#{name}"] = + if attachables.nil? || Array(attachables).none? + ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self) + else + ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables) + end + else + if !attachables.nil? || Array(attachables).any? + attachment_changes["#{name}"] = + ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, #{name}.blobs + attachables) end + end end CODE diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index f70d0a512acaf0dcae94617b11d2aec4925a4220..e88e7fa14e9fc3d9bbcf404d318c53250c58d4a3 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -79,6 +79,8 @@ class Engine < Rails::Engine # :nodoc: ActiveStorage.service_urls_expire_in = app.config.active_storage.service_urls_expire_in || 5.minutes ActiveStorage.content_types_allowed_inline = app.config.active_storage.content_types_allowed_inline || [] ActiveStorage.binary_content_type = app.config.active_storage.binary_content_type || "application/octet-stream" + + ActiveStorage.replace_on_assign_to_many = app.config.active_storage.replace_on_assign_to_many || false end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 878e284049f5e5c3ffc133fc3ae54aee3950c4e8..39ddecb0411302063058401d33abe92d27121d07 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -269,6 +269,24 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "updating an existing record with attachments when appending on assign" do + append_on_assign do + @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") + + assert_difference -> { @user.reload.highlights.count }, +2 do + @user.update! highlights: [ create_blob(filename: "whenever.jpg"), create_blob(filename: "wherever.jpg") ] + end + + assert_no_difference -> { @user.reload.highlights.count } do + @user.update! highlights: [ ] + end + + assert_no_difference -> { @user.reload.highlights.count } do + @user.update! highlights: nil + end + end + end + test "attaching existing blobs to a new record" do User.new(name: "Jason").tap do |user| user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") @@ -538,4 +556,12 @@ def highlights User.remove_method :highlights end end + + private + def append_on_assign + ActiveStorage.replace_on_assign_to_many, previous = false, ActiveStorage.replace_on_assign_to_many + yield + ensure + ActiveStorage.replace_on_assign_to_many = previous + end end diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index c1520766283c923ec91e51cec74683c53e6ce050..7c5478a03d4a7dd67dd515fa8ad679c98f2f6ca9 100644 --- a/guides/source/6_0_release_notes.md +++ b/guides/source/6_0_release_notes.md @@ -672,6 +672,12 @@ Please refer to the [Changelog][active-storage] for detailed changes. is saved instead of immediately. ([Pull Request](https://github.com/rails/rails/pull/33303)) +* Optionally replace existing files instead of adding to them when assigning to + a collection of attachments (as in `@user.update!(images: [ … ])`). Use + `config.active_storage.replace_on_assign_to_many` to control this behavior. + ([Pull Request](https://github.com/rails/rails/pull/33303), + [Pull Request](https://github.com/rails/rails/pull/36716)) + * Add the ability to reflect on defined attachments using the existing Active Record reflection mechanism. ([Pull Request](https://github.com/rails/rails/pull/33018)) @@ -688,10 +694,6 @@ Please refer to the [Changelog][active-storage] for detailed changes. `mini_magick` directly. ([Pull Request](https://github.com/rails/rails/pull/32471)) -* Replace existing images instead of adding to them when updating an - attached model via `update` or `update!` with, say, `@user.update!(images: [ … ])`. - ([Pull Request](https://github.com/rails/rails/pull/33303)) - Active Model ------------ diff --git a/guides/source/configuring.md b/guides/source/configuring.md index e53e8b4e926722bc6eac4e6142dd0d58bf5e78bb..4e0224b61e2100d8145057800133d3cd7207d072 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -881,7 +881,9 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla config.active_storage.routes_prefix = '/files' ``` - The default is `/rails/active_storage` + The default is `/rails/active_storage`. + +* `config.active_storage.replace_on_assign_to_many` determines whether assigning to a collection of attachments declared with `has_many_attached` replaces any existing attachments or appends to them. The default is `true`. ### Results of `load_defaults` @@ -917,6 +919,7 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla - `config.active_job.return_false_on_aborted_enqueue`: `true` - `config.active_storage.queues.analysis`: `:active_storage_analysis` - `config.active_storage.queues.purge`: `:active_storage_purge` +- `config.active_storage.replace_on_assign_to_many`: `true` - `config.active_record.collection_cache_versioning`: `true` ### Configuring a Database diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index f17955e02210b27d138a5eced54aa8da3756336b..05980d1614ea9fd4e4e6e68567e5f2a96889f356 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -398,6 +398,54 @@ config.load_defaults "6.0" config.autoloader = :classic ``` +### Active Storage assignment behavior change + +In Rails 5.2, assigning to a collection of attachments declared with `has_many_attached` appended new files: + +```ruby +class User < ApplicationRecord + has_many_attached :highlights +end + +user.highlights.attach(filename: "funky.jpg", ...) +user.higlights.count # => 1 + +blob = ActiveStorage::Blob.create_after_upload!(filename: "town.jpg", ...) +user.update!(highlights: [ blob ]) + +user.highlights.count # => 2 +user.highlights.first.filename # => "funky.jpg" +user.highlights.second.filename # => "town.jpg" +``` + +With the default configuration for Rails 6.0, assigning to a collection of attachments replaces existing files +instead of appending to them. This matches Active Record behavior when assigning to a collection association: + +```ruby +user.highlights.attach(filename: "funky.jpg", ...) +user.highlights.count # => 1 + +blob = ActiveStorage::Blob.create_after_upload!(filename: "town.jpg", ...) +user.update!(highlights: [ blob ]) + +user.highlights.count # => 1 +user.highlights.first.filename # => "town.jpg" +``` + +`#attach` can be used to add new attachments without removing the existing ones: + +```ruby +blob = ActiveStorage::Blob.create_after_upload!(filename: "town.jpg", ...) +user.highlights.attach(blob) + +user.highlights.count # => 2 +user.highlights.first.filename # => "funky.jpg" +user.highlights.second.filename # => "town.jpg" +``` + +Opt in to the new default behavior by setting `config.active_storage.replace_on_assign_to_many` to `true`. +The old behavior will be deprecated in Rails 6.1 and removed in a subsequent release. + Upgrading from Rails 5.1 to Rails 5.2 ------------------------------------- diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 50d43ff69ed91e994b590b6243ebd968239a673e..934578e9f12c0a81afd62cc5c158ce4bff48af95 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -145,6 +145,8 @@ def load_defaults(target_version) if respond_to?(:active_storage) active_storage.queues.analysis = :active_storage_analysis active_storage.queues.purge = :active_storage_purge + + active_storage.replace_on_assign_to_many = true end if respond_to?(:active_record) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt index abb03e761bbe4632e12ceaee114ba0288657a6d7..ffe53497bf434b341e8e2cbad605cb709212c92a 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt @@ -26,6 +26,10 @@ # Rails.application.config.active_storage.queues.analysis = :active_storage_analysis # Rails.application.config.active_storage.queues.purge = :active_storage_purge +# When assigning to a collection of attachments declared via `has_many_attached`, replace existing +# attachments instead of appending. Use #attach to add new attachments without replacing existing ones. +# Rails.application.config.active_storage.replace_on_assign_to_many = true + # Use ActionMailer::MailDeliveryJob for sending parameterized and normal mail. # # The default delivery jobs (ActionMailer::Parameterized::DeliveryJob, ActionMailer::DeliveryJob),