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

Do not retrieve disabled versions #1351

Conversation

filipegiusti
Copy link
Contributor

Fixes #1350 (copy from it's content below)

The code speaks for itself

require 'carrierwave'

class BugUploader < CarrierWave::Uploader::Base
  version :thumb, if: :always_false do
    def default_url
      'default.png'
    end
  end

  def root
    '~'
  end

  def always_false(*args)
    false
  end
end

up1 = BugUploader.new
up1.store!(File.open(__FILE__))
puts up1.thumb.url
# default.png

up2 = BugUploader.new
up2.retrieve_from_store!('lalala.png')
puts up2.thumb.url
# /uploads/thumb_lalala.png

@bensie
Copy link
Member

bensie commented Apr 3, 2014

@filipegiusti We need to ensure that files are never orphaned. The current implementation of conditional versions is more about the save process and less about URL fetching, where versions are processed conditionally at that time. At a later time, this condition may be different but the file may still be on disk.

@filipegiusti
Copy link
Contributor Author

I understand what you're saying. We can comply to both, on removal we can try to destroy all versions if it doesn't exist it will silently fail and we won't have orphans, and on retrieval we only get the ones that the conditional succeeds. Wdyt?

@filipegiusti
Copy link
Contributor Author

@bensie I've update the code to comply with your requirements, I don't see any downside in merging it as is.

@filipegiusti
Copy link
Contributor Author

Anyone listening? ⏰

@bensie
Copy link
Member

bensie commented Nov 19, 2014

@filipegiusti Sorry it's been so long! Would you mind rebasing off master so we can merge?

@filipegiusti
Copy link
Contributor Author

I'll do that next week!

@bensie bensie merged commit b9e0b65 into carrierwaveuploader:master Nov 19, 2014
@bensie
Copy link
Member

bensie commented Nov 19, 2014

I went ahead and got it merged, thanks!

@filipegiusti filipegiusti deleted the do_not_retrieve_disabled_versions branch November 20, 2014 14:40
@filipegiusti
Copy link
Contributor Author

Awesome, thanks!

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

Successfully merging this pull request may close these issues.

Conditional version returns wrong url when retrieving from store (but works when storing)
2 participants