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

Versions on a subclassed uploader save to the location of the parent class #1064

Closed
Gazler opened this Issue Apr 12, 2013 · 8 comments

Comments

Projects
None yet
9 participants
@Gazler

Gazler commented Apr 12, 2013

I have made a repository that displays this issue at https://github.com/Gazler/carrierwave-upload-versions

I am not sure if this behaviour is intended. I have the following Uploader that sets the versions to be used by other Uploaders:

class BaseUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick

  storage :file

  version :square do
    process :resize_to_fill => [380, 380]
    end

  version :thumb, :from_version => :square do
    process :resize_to_fill => [119, 119]
  end
end

I then want to override the store location when an uploader subclasses this, so I do the following:

class AvatarUploader < BaseUploader

  def store_dir
    "uploads/people/pictures/#{model.id}"
  end
end

When I upload an image, I get the following urls:

User.first.avatar.url # "/uploads/people/pictures/4/avatar.png"
User.first.avatar.thumb.url #"/uploads/thumb_avatar.png"

I would expect the url of the thumbnail to be:

User.first.avatar.thumb.url #"/uploads/people/pictures/thumb_avatar.png"

Apologies if this is intentional or has been raised before.

@BBonifield

This comment has been minimized.

Show comment
Hide comment
@BBonifield

BBonifield Jul 9, 2013

👍 I ran into this same issue today. My issue was not around storage locations but rather version process calls.

class BaseUploader < CarrierWave::Uploader::Base

  include CarrierWave::MiniMagick
  version :thumbnail do
    process :resize_to_fill => [200, 200]
  end

end

class IconImageUploader < BaseUploader

  version :processed do
    process :resize_to_fill => [175, 175]
  end

end

I can access .thumbnail of IconImageUploader instances, but the process operation is not performed.

The test below passes if I define the thumbnail version on IconImageUploader, but it fails when thumbnail is defined on BaseUploader

require 'spec_helper'
require 'carrierwave/test/matchers'

describe IconImageUploader do
  include CarrierWave::Test::Matchers

  before do
    bundle = build_stubbed(:bundle)
    test_image_path = fixture_file_path 'test-image.jpg'
    IconImageUploader.enable_processing = true
    @uploader = IconImageUploader.new(bundle, :bundle)
    @uploader.store!(File.open(test_image_path))
  end

  after do
    IconImageUploader.enable_processing = false
    @uploader.remove!
  end

  context 'the thumbnail version' do
    it "should scale down an image to be within 200x200" do
      @uploader.thumbnail.should be_no_larger_than(200, 200)
    end
  end
end

BBonifield commented Jul 9, 2013

👍 I ran into this same issue today. My issue was not around storage locations but rather version process calls.

class BaseUploader < CarrierWave::Uploader::Base

  include CarrierWave::MiniMagick
  version :thumbnail do
    process :resize_to_fill => [200, 200]
  end

end

class IconImageUploader < BaseUploader

  version :processed do
    process :resize_to_fill => [175, 175]
  end

end

I can access .thumbnail of IconImageUploader instances, but the process operation is not performed.

The test below passes if I define the thumbnail version on IconImageUploader, but it fails when thumbnail is defined on BaseUploader

require 'spec_helper'
require 'carrierwave/test/matchers'

describe IconImageUploader do
  include CarrierWave::Test::Matchers

  before do
    bundle = build_stubbed(:bundle)
    test_image_path = fixture_file_path 'test-image.jpg'
    IconImageUploader.enable_processing = true
    @uploader = IconImageUploader.new(bundle, :bundle)
    @uploader.store!(File.open(test_image_path))
  end

  after do
    IconImageUploader.enable_processing = false
    @uploader.remove!
  end

  context 'the thumbnail version' do
    it "should scale down an image to be within 200x200" do
      @uploader.thumbnail.should be_no_larger_than(200, 200)
    end
  end
end
@synth

This comment has been minimized.

Show comment
Hide comment
@synth

synth Jul 21, 2014

+1, hitting this issue as well; we're changing the default_url's on subclasses but revert to the parent classes implementation

synth commented Jul 21, 2014

+1, hitting this issue as well; we're changing the default_url's on subclasses but revert to the parent classes implementation

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue commented Sep 29, 2014

+1

@satyapatidar

This comment has been minimized.

Show comment
Hide comment
@satyapatidar

satyapatidar Dec 17, 2014

👍 happening same with overriding store_dir is picking the parent class imaplementation.

satyapatidar commented Dec 17, 2014

👍 happening same with overriding store_dir is picking the parent class imaplementation.

@eavgerinos

This comment has been minimized.

Show comment
Hide comment
@eavgerinos

eavgerinos Jun 4, 2015

Member

This is a general design issue that can be seen here.

Any patch that changes this logic without breaking something will be appreciated.

Member

eavgerinos commented Jun 4, 2015

This is a general design issue that can be seen here.

Any patch that changes this logic without breaking something will be appreciated.

@eavgerinos eavgerinos added bug and removed bug labels Jun 4, 2015

@chrhansen

This comment has been minimized.

Show comment
Hide comment
@chrhansen

chrhansen Jul 27, 2015

I'm also seeing this issue when I'm subclassing a base_uploader(PublicImageUploader < BaseUploader). If files are defaulted to fog_public = false in my base_uploader-class, then even when I set:

# public_image_uploader.rb
def self.fog_public
  true
end

In a subclass, this is only respected for the original file, but not in version-calls. So for now, I've done this

# public_image_uploader.rb
version :thumb do
  def fog_public
    self.class.fog_public
  end
  process resize_to_fill: [200, 200]
end

Based on suggestion here: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/uploader/versions.rb#L96 – but it hurts my eyes.

chrhansen commented Jul 27, 2015

I'm also seeing this issue when I'm subclassing a base_uploader(PublicImageUploader < BaseUploader). If files are defaulted to fog_public = false in my base_uploader-class, then even when I set:

# public_image_uploader.rb
def self.fog_public
  true
end

In a subclass, this is only respected for the original file, but not in version-calls. So for now, I've done this

# public_image_uploader.rb
version :thumb do
  def fog_public
    self.class.fog_public
  end
  process resize_to_fill: [200, 200]
end

Based on suggestion here: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/uploader/versions.rb#L96 – but it hurts my eyes.

@thomasfedb

This comment has been minimized.

Show comment
Hide comment
@thomasfedb

thomasfedb Nov 16, 2015

Contributor

@chrhansen if you believe that you have found a genuine bug, rather than a design choice, please report a new issue.

Contributor

thomasfedb commented Nov 16, 2015

@chrhansen if you believe that you have found a genuine bug, rather than a design choice, please report a new issue.

@tbrammar

This comment has been minimized.

Show comment
Hide comment
@tbrammar

tbrammar Feb 1, 2017

We've just run into this same issue. As this is by design perhaps it might be helpful to highlight this in the Readme text?

tbrammar commented Feb 1, 2017

We've just run into this same issue. As this is by design perhaps it might be helpful to highlight this in the Readme text?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment