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

3.0.0 Breaking change with enable_processing behavior of versions #2676

Closed
rajyan opened this issue Jul 18, 2023 · 3 comments
Closed

3.0.0 Breaking change with enable_processing behavior of versions #2676

rajyan opened this issue Jul 18, 2023 · 3 comments

Comments

@rajyan
Copy link
Contributor

rajyan commented Jul 18, 2023

Summary

This change 1531a67 led to a breaking change with enable_processing configuration.

Before this change we could change versions' enable_processing by calling the mounted column uploader's config like in the README.

MyUploader.enable_processing = true

Although, 3.0.0 has a complex change with this behavior. The above configuration sometimes works, but sometimes not.

Minimum reproducible example

class TestImageUploader < CarrierWave::Uploader::Base
  version :test do
  end
end

class TestImage < ActiveRecord::Base
  mount_uploader :image, TestImageUploader
end

i=TestImage.new
i.image.class.instance_variables
i.image.enable_processing
i.image.class.instance_variables # instance_variable `@enable_processing` is set

i.image.test.class.instance_variables
i.image.test.enable_processing
i.image.test.class.instance_variables # instance_variable `@enable_processing` is not set before 3.0.0 but set in 3.0.0

Problem

We noticed this change with our test suite.
If we have called the mounted column somewhere in our test, we cannot change the enable_processing configuration of the versions afterwards in other tests.

RSpec.describe TestImageUploader do
  describe 'test1' do
    let(:test_image) { build(:test_image) }

    it 'run something that invokes enable_processing' do
      test_image.image = File.open('foo.jpg', 'rb')
    end
  end

  describe 'test2' do
    let(:test_image) { build(:test_image) }

    before do
      TestImageUploader.enable_processing = true
    end

    after do
      TestImageUploader.enable_processing = false
    end

    it 'can change enable_processing' do
      expect(test_image.image.test.enable_processing).to be true
    end
  end
end

test2 succeeds if you run it individually, but fails if you run after test1.

I don't have a valid use-case in production, but if someone is changing enable_processing dynamically in their application, the same issue might happen.

I believe restoring these lines
1531a67#diff-c3e7243412c35dfefcaf2569736474b74402cc34cb550e472aacef867d91da18L90-L97
in the Builder can fix this issue, but want some opinions before proceeding.

@rajyan
Copy link
Contributor Author

rajyan commented Jul 18, 2023

There is a workaround

test_image.image.test.class.enable_processing = true

@mshibuya
Copy link
Member

My intention was that the inheritance structure change in 1531a67 will make the #enable_processing patch unnecessary, but actually it wasn't as you mentioned. I've restored the #enable_processing patch back and now this is fixed.

Thank you for reporting 👍

@rajyan
Copy link
Contributor Author

rajyan commented Jul 24, 2023

@mshibuya

Thank you for your swift replies!

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

No branches or pull requests

2 participants