Disable variant options when false or nil present

In response to https://github.com/rails/rails/issues/32917

In the current implementation, ActiveStorage passes all options to the underlying processor,
including when a key has a value of false.

For example, passing:

```
avatar.variant(resize: "100x100", monochrome: false, flip: "-90")
```

will return a monochrome image (or an error, pending on ImageMagick configuration) because
it passes `-monochrome false` to the command (but the command line does not allow disabling
flags this way, as usually a user would omit the flag entirely to disable that feature).

This fix only passes those keys forward to the underlying processor if the value responds to
`present?`. In practice, this means that `false` or `nil` will be filtered out before going
to the processor.

One possible use case would be for a user to be able to apply different filters to an avatar.
The code might look something like:

```
  variant_options = {
    monochrome: params[:monochrome],
    resize:     params[:resize]
  }

  avatar.variant(*variant_options)
```

Obviously some sanitization may be beneficial in a real-world scenario, but this type of
configuration object could be used in many other places as well.

- Add removing falsy values from varaints to changelog

- The entirety of #image_processing_transformation inject block was wrapped in `list.tap`
 to guard against the default `nil` being returned if no conditional was called.

- add test for explicitly true variant options

3 files changed, 81 insertions, 12 deletions

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 7b724b7b81..c8911fe611 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,13 @@
+*   Variant arguments of `false` or `nil` will no longer be passed to the
+    processor. For example, the following will not have the monochrome
+    variation applied:
+
+    ```ruby
+      avatar.variant(monochrome: false)
+    ```
+
+    *Jacob Smith*
+
 *   Generated attachment getter and setter methods are created
     within the model's `GeneratedAssociationMethods` module to
     allow overriding and composition using `super`.
diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb
index 42f00beb82..806af6366d 100644
--- a/activestorage/app/models/active_storage/variation.rb
+++ b/activestorage/app/models/active_storage/variation.rb
@@ -66,11 +66,13 @@ class ActiveStorage::Variation
     # Applies image transformations using the ImageProcessing gem.
     def image_processing_transform(file, format)
       operations = transformations.inject([]) do |list, (name, argument)|
-        if name.to_s == "combine_options"
-          ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.")
-          list.concat argument.to_a
-        else
-          list << [name, argument]
+        list.tap do |list|
+          if name.to_s == "combine_options"
+            ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.")
+            list.concat argument.keep_if { |key, value| value.present? }.to_a
+          elsif argument.present?
+            list << [name, argument]
+          end
         end
       end
 
@@ -116,14 +118,10 @@ class ActiveStorage::Variation
     end
 
     def pass_transform_argument(command, method, argument)
-      if eligible_argument?(argument)
-        command.public_send(method, argument)
-      else
+      if argument == true
         command.public_send(method)
+      elsif argument.present?
+        command.public_send(method, argument)
       end
     end
-
-    def eligible_argument?(argument)
-      argument.present? && argument != true
-    end
 end
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index e74bbc9ab4..6577f1cd9f 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -25,6 +25,67 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     assert_match(/Gray/, image.colorspace)
   end
 
+  test "monochrome with default variant_processor" do
+    begin
+      ActiveStorage.variant_processor = nil
+
+      blob = create_file_blob(filename: "racecar.jpg")
+      variant = blob.variant(monochrome: true).processed
+      image = read_image(variant)
+      assert_match(/Gray/, image.colorspace)
+    ensure
+      ActiveStorage.variant_processor = :mini_magick
+    end
+  end
+
+  test "disabled variation of JPEG blob" do
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = blob.variant(resize: "100x100", monochrome: false).processed
+    assert_match(/racecar\.jpg/, variant.service_url)
+
+    image = read_image(variant)
+    assert_equal 100, image.width
+    assert_equal 67, image.height
+    assert_match(/RGB/, image.colorspace)
+  end
+
+  test "disabled variation of JPEG blob with :combine_options" do
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = ActiveSupport::Deprecation.silence do
+      blob.variant(combine_options: {
+        resize: "100x100",
+        monochrome: false
+      }).processed
+    end
+    assert_match(/racecar\.jpg/, variant.service_url)
+
+    image = read_image(variant)
+    assert_equal 100, image.width
+    assert_equal 67, image.height
+    assert_match(/RGB/, image.colorspace)
+  end
+
+  test "disabled variation using :combine_options" do
+    begin
+      ActiveStorage.variant_processor = nil
+      blob = create_file_blob(filename: "racecar.jpg")
+      variant = ActiveSupport::Deprecation.silence do
+        blob.variant(combine_options: {
+          crop: "100x100+0+0",
+          monochrome: false
+        }).processed
+      end
+      assert_match(/racecar\.jpg/, variant.service_url)
+
+      image = read_image(variant)
+      assert_equal 100, image.width
+      assert_equal 100, image.height
+      assert_match(/RGB/, image.colorspace)
+    ensure
+      ActiveStorage.variant_processor = :mini_magick
+    end
+  end
+
   test "center-weighted crop of JPEG blob using :combine_options" do
     begin
       ActiveStorage.variant_processor = nil