1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Preserve existing attachment assignment behavior for upgraded apps

Assigning to a collection of attachments appends rather than replacing, as in 5.2. Existing 5.2 apps that rely on this behavior will no longer break when they're upgraded to 6.0.

For apps generated on 6.0 or newer, assigning replaces the existing attachments in the collection. #attach should be used to add new attachments to the collection without removing existing ones.

I expect that we'll deprecate the old behavior in 6.1.

Closes #36374.
This commit is contained in:
George Claghorn 2019-07-20 06:33:11 -04:00 committed by GitHub
parent 876548a7e7
commit 400b210354
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 118 additions and 16 deletions

View file

@ -43,18 +43,26 @@ module ActiveStorage
mattr_accessor :logger
mattr_accessor :verifier
mattr_accessor :queues, default: {}
mattr_accessor :previewers, default: []
mattr_accessor :analyzers, default: []
mattr_accessor :variant_processor, default: :mini_magick
mattr_accessor :queues, default: {}
mattr_accessor :previewers, default: []
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

View file

@ -93,12 +93,19 @@ module ActiveStorage
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

View file

@ -79,6 +79,8 @@ module ActiveStorage
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

View file

@ -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 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
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

View file

@ -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
------------

View file

@ -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

View file

@ -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
-------------------------------------

View file

@ -145,6 +145,8 @@ module Rails
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)

View file

@ -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),