diff options
23 files changed, 383 insertions, 42 deletions
diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 9ef4f50df1..879745a895 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -53,7 +53,7 @@ module ActionController #:nodoc: # # Show a 404 page in the browser: # - # send_file '/path/to/404.html', type: 'text/html; charset=utf-8', status: 404 + # send_file '/path/to/404.html', type: 'text/html; charset=utf-8', disposition: 'inline', status: 404 # # Read about the other Content-* HTTP headers if you'd like to # provide the user with more information (such as Content-Description) in diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 7dedecef34..9c430b57e3 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -137,7 +137,11 @@ module ActionDispatch #:nodoc: object_src: "object-src", prefetch_src: "prefetch-src", script_src: "script-src", + script_src_attr: "script-src-attr", + script_src_elem: "script-src-elem", style_src: "style-src", + style_src_attr: "style-src-attr", + style_src_elem: "style-src-elem", worker_src: "worker-src" }.freeze diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index a4634626bb..3d60dc1661 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -128,12 +128,36 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase @policy.script_src false assert_no_match %r{script-src}, @policy.build + @policy.script_src_attr :self + assert_match %r{script-src-attr 'self'}, @policy.build + + @policy.script_src_attr false + assert_no_match %r{script-src-attr}, @policy.build + + @policy.script_src_elem :self + assert_match %r{script-src-elem 'self'}, @policy.build + + @policy.script_src_elem false + assert_no_match %r{script-src-elem}, @policy.build + @policy.style_src :self assert_match %r{style-src 'self'}, @policy.build @policy.style_src false assert_no_match %r{style-src}, @policy.build + @policy.style_src_attr :self + assert_match %r{style-src-attr 'self'}, @policy.build + + @policy.style_src_attr false + assert_no_match %r{style-src-attr}, @policy.build + + @policy.style_src_elem :self + assert_match %r{style-src-elem 'self'}, @policy.build + + @policy.style_src_elem false + assert_no_match %r{style-src-elem}, @policy.build + @policy.worker_src :self assert_match %r{worker-src 'self'}, @policy.build diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 4b3a258287..3df2eaf079 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -571,6 +571,54 @@ module ActionView end end + # Creates an SMS anchor link tag to the specified +phone_number+, which is + # also used as the name of the link unless +name+ is specified. Additional + # HTML attributes for the link can be passed in +html_options+. + # + # When clicked, an SMS message is prepopulated with the passed phone number + # and optional +body+ value. + # + # +sms_to+ has a +body+ option for customizing the SMS message itself by + # passing special keys to +html_options+. + # + # ==== Options + # * <tt>:body</tt> - Preset the body of the message. + # + # ==== Examples + # sms_to "5155555785" + # # => <a href="sms:5155555785;">5155555785</a> + # + # sms_to "5155555785", "Text me" + # # => <a href="sms:5155555785;">Text me</a> + # + # sms_to "5155555785", "Text me", + # body: "Hello Jim I have a question about your product." + # # => <a href="sms:5155555785;?body=Hello%20Jim%20I%20have%20a%20question%20about%20your%20product">Text me</a> + # + # You can use a block as well if your link target is hard to fit into the name parameter. ERB example: + # + # <%= sms_to "5155555785" do %> + # <strong>Text me:</strong> + # <% end %> + # # => <a href="sms:5155555785;"> + # <strong>Text me:</strong> + # </a> + def sms_to(phone_number, name = nil, html_options = {}, &block) + html_options, name = name, nil if block_given? + html_options = (html_options || {}).stringify_keys + + extras = %w{ body }.map! { |item| + option = html_options.delete(item).presence || next + "#{item.dasherize}=#{ERB::Util.url_encode(option)}" + }.compact + extras = extras.empty? ? "" : "?&" + extras.join("&") + + encoded_phone_number = ERB::Util.url_encode(phone_number) + html_options["href"] = "sms:#{encoded_phone_number};#{extras}" + + content_tag("a", name || phone_number, html_options, &block) + end + private def convert_options_to_data_attributes(options, html_options) if html_options diff --git a/actionview/lib/action_view/unbound_template.rb b/actionview/lib/action_view/unbound_template.rb index d28bab91b6..3d4434b2e9 100644 --- a/actionview/lib/action_view/unbound_template.rb +++ b/actionview/lib/action_view/unbound_template.rb @@ -4,9 +4,9 @@ require "concurrent/map" module ActionView class UnboundTemplate - def initialize(source, identifer, handler, options) + def initialize(source, identifier, handler, options) @source = source - @identifer = identifer + @identifier = identifier @handler = handler @options = options @@ -22,7 +22,7 @@ module ActionView options = @options.merge(locals: locals) Template.new( @source, - @identifer, + @identifier, @handler, options ) diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb index 632b32f09f..bce6e7f370 100644 --- a/actionview/test/template/url_helper_test.rb +++ b/actionview/test/template/url_helper_test.rb @@ -708,6 +708,68 @@ class UrlHelperTest < ActiveSupport::TestCase assert_equal({ class: "special" }, options) end + def test_sms_to + assert_dom_equal %{<a href="sms:15155555785;">15155555785</a>}, sms_to("15155555785") + assert_dom_equal %{<a href="sms:15155555785;">Jim Jones</a>}, sms_to("15155555785", "Jim Jones") + assert_dom_equal( + %{<a class="admin" href="sms:15155555785;">Jim Jones</a>}, + sms_to("15155555785", "Jim Jones", "class" => "admin") + ) + assert_equal sms_to("15155555785", "Jim Jones", "class" => "admin"), + sms_to("15155555785", "Jim Jones", class: "admin") + end + + def test_sms_to_with_options + assert_dom_equal( + %{<a class="simple-class" href="sms:15155555785;?&body=Hello%20from%20Jim">Text me</a>}, + sms_to("15155555785", "Text me", class: "simple-class", body: "Hello from Jim") + ) + + assert_dom_equal( + %{<a href="sms:15155555785;?&body=This%20is%20the%20body%20of%20the%20message.">Text me</a>}, + sms_to("15155555785", "Text me", body: "This is the body of the message.") + ) + end + + def test_sms_with_img + assert_dom_equal %{<a href="sms:15155555785;"><img src="/feedback.png" /></a>}, + sms_to("15155555785", raw('<img src="/feedback.png" />')) + end + + def test_sms_with_html_safe_string + assert_dom_equal( + %{<a href="sms:1%2B5155555785;">1+5155555785</a>}, + sms_to(raw("1+5155555785")) + ) + end + + def test_sms_with_nil + assert_dom_equal( + %{<a href="sms:;"></a>}, + sms_to(nil) + ) + end + + def test_sms_returns_html_safe_string + assert_predicate sms_to("15155555785"), :html_safe? + end + + def test_sms_with_block + assert_dom_equal %{<a href="sms:15155555785;"><span>Text me</span></a>}, + sms_to("15155555785") { content_tag(:span, "Text me") } + end + + def test_sms_with_block_and_options + assert_dom_equal %{<a class="special" href="sms:15155555785;?&body=Hello%20from%20Jim"><span>Text me</span></a>}, + sms_to("15155555785", body: "Hello from Jim", class: "special") { content_tag(:span, "Text me") } + end + + def test_sms_does_not_modify_html_options_hash + options = { class: "special" } + sms_to "15155555785", "ME!", options + assert_equal({ class: "special" }, options) + end + def protect_against_forgery? request_forgery end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 22839bd9fb..5628a59845 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -220,7 +220,7 @@ module ActiveModel # # then yield :name and "must be specified" # end def each(&block) - if block.arity == 1 + if block.arity <= 1 @errors.each(&block) else ActiveSupport::Deprecation.warn(<<~MSG) diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index d66d6ceff0..60b20ab59f 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -44,6 +44,14 @@ class ErrorsTest < ActiveModel::TestCase assert_includes errors, "foo", "errors should include 'foo' as :foo" end + def test_each_when_arity_is_negative + errors = ActiveModel::Errors.new(Person.new) + errors.add(:name, :blank) + errors.add(:gender, :blank) + + assert_equal([:name, :gender], errors.map(&:attribute)) + end + def test_any? errors = ActiveModel::Errors.new(Person.new) errors.add(:name) diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 648fdd0dc4..98f57549a5 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -112,7 +112,7 @@ db_namespace = namespace :db do end end - # desc 'Rollbacks the database one migration and re migrate up (options: STEP=x, VERSION=x).' + desc "Rolls back the database one migration and re-migrates up (options: STEP=x, VERSION=x)." task redo: :load_config do raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty? @@ -128,7 +128,7 @@ db_namespace = namespace :db do # desc 'Resets your database using your migrations for the current environment' task reset: ["db:drop", "db:create", "db:migrate"] - # desc 'Runs the "up" for a given migration VERSION.' + desc 'Runs the "up" for a given migration VERSION.' task up: :load_config do ActiveRecord::Tasks::DatabaseTasks.raise_for_multi_db(command: "db:migrate:up") @@ -162,7 +162,7 @@ db_namespace = namespace :db do end end - # desc 'Runs the "down" for a given migration VERSION.' + desc 'Runs the "down" for a given migration VERSION.' task down: :load_config do ActiveRecord::Tasks::DatabaseTasks.raise_for_multi_db(command: "db:migrate:down") @@ -230,7 +230,7 @@ db_namespace = namespace :db do db_namespace["_dump"].invoke end - # desc 'Drops and recreates the database from db/schema.rb for the current environment and loads the seeds.' + desc "Drops and recreates the database from db/schema.rb for the current environment and loads the seeds." task reset: [ "db:drop", "db:setup" ] # desc "Retrieves the charset for the current environment's database" diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 87a53966e4..6a181882ae 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -952,7 +952,7 @@ module ActiveRecord def optimizer_hints!(*args) # :nodoc: args.flatten! - self.optimizer_hints_values += args + self.optimizer_hints_values |= args self end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index e0743de94b..e74fb1a098 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -363,6 +363,13 @@ module ActiveRecord assert_match %r{/\*\+ BADHINT \*/}, post_with_hint.to_sql end + def test_does_not_duplicate_optimizer_hints_on_merge + escaped_table = Post.connection.quote_table_name("posts") + expected = "SELECT /*+ OMGHINT */ #{escaped_table}.* FROM #{escaped_table}" + query = Post.optimizer_hints("OMGHINT").merge(Post.optimizer_hints("OMGHINT")).to_sql + assert_equal expected, query + end + class EnsureRoundTripTypeCasting < ActiveRecord::Type::Value def type :string diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 5c5da551ae..75eb63f0b4 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 ae7f0685f2..06864a846f 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -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 diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index f70d0a512a..e88e7fa14e 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -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 diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 57e094908a..172a2f0aae 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -13,7 +13,7 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase assert_equal 640, metadata[:width] assert_equal 480, metadata[:height] assert_equal [4, 3], metadata[:display_aspect_ratio] - assert_equal true, metadata[:duration].between?(4, 6) + assert_equal 5.166648, metadata[:duration] assert_not_includes metadata, :angle end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 878e284049..39ddecb041 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 @@ 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 diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index c152076628..7c5478a03d 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 e53e8b4e92..4e0224b61e 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 f17955e022..05980d1614 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 50d43ff69e..934578e9f1 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -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) diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 406a5b8fc7..b6225cd8c0 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -40,8 +40,7 @@ module Rails in_root do str = "gem #{parts.join(", ")}" str = indentation + str - str = "\n" + str - append_file "Gemfile", str, verbose: false + append_file_with_newline "Gemfile", str, verbose: false end end @@ -58,9 +57,9 @@ module Rails log :gemfile, "group #{str}" in_root do - append_file "Gemfile", "\ngroup #{str} do", force: true + append_file_with_newline "Gemfile", "\ngroup #{str} do", force: true with_indentation(&block) - append_file "Gemfile", "\nend\n", force: true + append_file_with_newline "Gemfile", "end", force: true end end @@ -71,9 +70,13 @@ module Rails log :github, "github #{str}" in_root do - append_file "Gemfile", "\n#{indentation}github #{str} do", force: true + if @indentation.zero? + append_file_with_newline "Gemfile", "\ngithub #{str} do", force: true + else + append_file_with_newline "Gemfile", "#{indentation}github #{str} do", force: true + end with_indentation(&block) - append_file "Gemfile", "\n#{indentation}end", force: true + append_file_with_newline "Gemfile", "#{indentation}end", force: true end end @@ -91,9 +94,9 @@ module Rails in_root do if block - append_file "Gemfile", "\nsource #{quote(source)} do", force: true + append_file_with_newline "Gemfile", "\nsource #{quote(source)} do", force: true with_indentation(&block) - append_file "Gemfile", "\nend\n", force: true + append_file_with_newline "Gemfile", "end", force: true else prepend_file "Gemfile", "source #{quote(source)}\n", verbose: false end @@ -344,6 +347,13 @@ module Rails ensure @indentation -= 1 end + + # Append string to a file with a newline if necessary + def append_file_with_newline(path, str, options = {}) + gsub_file path, /\n?\z/, options do |match| + match.end_with?("\n") ? "" : "\n#{str}\n" + end + end end end end 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 abb03e761b..ffe53497bf 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), diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 150836d4ce..5d6d7f1595 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -43,7 +43,7 @@ class ActionsTest < Rails::Generators::TestCase def test_add_source_adds_source_to_gemfile run_generator action :add_source, "http://gems.github.com" - assert_file "Gemfile", /source 'http:\/\/gems\.github\.com'/ + assert_file "Gemfile", /source 'http:\/\/gems\.github\.com'\n/ end def test_add_source_with_block_adds_source_to_gemfile_with_gem @@ -51,7 +51,7 @@ class ActionsTest < Rails::Generators::TestCase action :add_source, "http://gems.github.com" do gem "rspec-rails" end - assert_file "Gemfile", /source 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend/ + assert_file "Gemfile", /\n\nsource 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend\n\z/ end def test_add_source_with_block_adds_source_to_gemfile_after_gem @@ -60,13 +60,25 @@ class ActionsTest < Rails::Generators::TestCase action :add_source, "http://gems.github.com" do gem "rspec-rails" end - assert_file "Gemfile", /gem 'will-paginate'\nsource 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend/ + assert_file "Gemfile", /\ngem 'will-paginate'\n\nsource 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend\n\z/ + end + + def test_add_source_should_create_newline_between_blocks + run_generator + action :add_source, "http://gems.github.com" do + gem "rspec-rails" + end + + action :add_source, "http://gems2.github.com" do + gem "fakeweb" + end + assert_file "Gemfile", /\n\nsource 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend\n\nsource 'http:\/\/gems2\.github\.com' do\n gem 'fakeweb'\nend\n\z/ end def test_gem_should_put_gem_dependency_in_gemfile run_generator action :gem, "will-paginate" - assert_file "Gemfile", /gem 'will\-paginate'/ + assert_file "Gemfile", /gem 'will\-paginate'\n\z/ end def test_gem_with_version_should_include_version_in_gemfile @@ -141,7 +153,7 @@ class ActionsTest < Rails::Generators::TestCase gem "fakeweb" end - assert_file "Gemfile", /\ngroup :development, :test do\n gem 'rspec-rails'\nend\n\ngroup :test do\n gem 'fakeweb'\nend/ + assert_file "Gemfile", /\n\ngroup :development, :test do\n gem 'rspec-rails'\nend\n\ngroup :test do\n gem 'fakeweb'\nend\n\z/ end def test_github_should_create_an_indented_block @@ -153,7 +165,7 @@ class ActionsTest < Rails::Generators::TestCase gem "baz" end - assert_file "Gemfile", /\ngithub 'user\/repo' do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend/ + assert_file "Gemfile", /\n\ngithub 'user\/repo' do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend\n\z/ end def test_github_should_create_an_indented_block_with_options @@ -165,7 +177,7 @@ class ActionsTest < Rails::Generators::TestCase gem "baz" end - assert_file "Gemfile", /\ngithub 'user\/repo', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend/ + assert_file "Gemfile", /\n\ngithub 'user\/repo', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend\n\z/ end def test_github_should_create_an_indented_block_within_a_group @@ -177,9 +189,73 @@ class ActionsTest < Rails::Generators::TestCase gem "bar" gem "baz" end + github "user/repo2", a: "correct", other: true do + gem "foo" + gem "bar" + gem "baz" + end + end + + assert_file "Gemfile", /\n\ngroup :magic do\n github 'user\/repo', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\n end\n github 'user\/repo2', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\n end\nend\n\z/ + end + + def test_github_should_create_newline_between_blocks + run_generator + + action :github, "user/repo", a: "correct", other: true do + gem "foo" + gem "bar" + gem "baz" + end + + action :github, "user/repo2", a: "correct", other: true do + gem "foo" + gem "bar" + gem "baz" + end + + assert_file "Gemfile", /\n\ngithub 'user\/repo', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend\n\ngithub 'user\/repo2', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\nend\n\z/ + end + + def test_gem_with_gemfile_without_newline_at_the_end + run_generator + File.open("Gemfile", "a") { |f| f.write("gem 'rspec-rails'") } + + action :gem, "will-paginate" + assert_file "Gemfile", /gem 'rspec-rails'\ngem 'will-paginate'\n\z/ + end + + def test_gem_group_with_gemfile_without_newline_at_the_end + run_generator + File.open("Gemfile", "a") { |f| f.write("gem 'rspec-rails'") } + + action :gem_group, :test do + gem "fakeweb" + end + + assert_file "Gemfile", /gem 'rspec-rails'\n\ngroup :test do\n gem 'fakeweb'\nend\n\z/ + end + + def test_add_source_with_gemfile_without_newline_at_the_end + run_generator + File.open("Gemfile", "a") { |f| f.write("gem 'rspec-rails'") } + + action :add_source, "http://gems.github.com" do + gem "fakeweb" + end + + assert_file "Gemfile", /gem 'rspec-rails'\n\nsource 'http:\/\/gems\.github\.com' do\n gem 'fakeweb'\nend\n\z/ + end + + def test_github_with_gemfile_without_newline_at_the_end + run_generator + File.open("Gemfile", "a") { |f| f.write("gem 'rspec-rails'") } + + action :github, "user/repo" do + gem "fakeweb" end - assert_file "Gemfile", /\ngroup :magic do\n github 'user\/repo', a: 'correct', other: true do\n gem 'foo'\n gem 'bar'\n gem 'baz'\n end\nend\n/ + assert_file "Gemfile", /gem 'rspec-rails'\n\ngithub 'user\/repo' do\n gem 'fakeweb'\nend\n\z/ end def test_environment_should_include_data_in_environment_initializer_block |