Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional 'process convert: :format' always changes file extension #2723

Open
langalex opened this issue Jan 30, 2024 · 1 comment
Open
Labels

Comments

@langalex
Copy link

Given an uploader:

class MyUploader < CarrierWave::Uploader::Base
  process convert: :webp, if: :should_convert_to_webp?

  def should_convert_to_webp?(filename)
    ..
  end
end

The file extension is always changed to webp, even if should_convert_to_webp? returns false.

The error was introduced in 3.0.5:

module CarrierWave
  module Uploader
    module Processing
        def process(*args)
          new_processors = args.inject({}) do |hash, arg|
            arg = { arg => [] } unless arg.is_a?(Hash)
            hash.merge!(arg)
          end

          condition_type = new_processors.keys.detect { |key| [:if, :unless].include?(key) }
          condition = new_processors.delete(:if) || new_processors.delete(:unless)
          new_processors.each do |processor, processor_args|
            self.processors += [[processor, processor_args, condition, condition_type]]

            if processor == :convert
              # Treat :convert specially, since it should trigger the file extension change
              force_extension processor_args
            end
          end
        end
      end # ClassMethods
    end
  end
end

The if processor == :convert clause runs on the class level, ignoring the condition which is evaluated on the instance level, when an actual file is uploaded.

@mshibuya mshibuya added the bug label Feb 4, 2024
@mshibuya mshibuya added this to the Release v4.0.0 milestone Dec 7, 2024
@mshibuya
Copy link
Member

mshibuya commented Dec 7, 2024

Sorry but it turned out that this issue is hard to resolve in a non-breaking way. I've added a warning message in fb287f3, so users can be aware of this.

For the time being I would suggest working around this by using conditional versions, like:

class CarrierwaveUploader < CarrierWave::Uploader::Base
  ...

  version :webp, if: :should_convert_to_webp? do
    process convert: :webp
  end

  def should_convert_to_webp?(file)
    file.content_type == 'image/jpeg'
  end

  def url(*args)
    if version_active?(:webp)
      webp.url(*args)
    else
      super
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants