Skip to content

Commit

Permalink
Clear download errors on reassigning remote urls
Browse files Browse the repository at this point in the history
Closes #2725
  • Loading branch information
mshibuya committed Oct 19, 2024
1 parent ce272b7 commit bb8c6e1
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/carrierwave/mounter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def remote_urls=(urls)
urls = Array.wrap(urls).reject(&:blank?)
return if urls.blank?
end
@download_errors.clear
@remote_urls = urls

clear_unstaged
Expand Down
9 changes: 9 additions & 0 deletions spec/mount_multiple_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,15 @@ def monkey

it { is_expected.to include(a_kind_of(CarrierWave::DownloadError)) }
end

context "on the second attempt" do
it "clears the existing download errors" do
instance.remote_images_urls = ["http://www.example.com/missing.jpg"]
is_expected.not_to be_empty
instance.remote_images_urls = ["http://www.example.com/#{test_file_name}"]
is_expected.to be_empty
end
end
end

describe '#write_images_identifier' do
Expand Down
7 changes: 7 additions & 0 deletions spec/mount_single_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,13 @@ def monkey
@instance.remote_image_url = "http://www.example.com/missing.jpg"
expect(@instance.image_download_error).to be_an_instance_of(CarrierWave::DownloadError)
end

it "clears the existing download error on the second attempt" do
@instance.remote_image_url = "http://www.example.com/missing.jpg"
expect(@instance.image_download_error).to be_an_instance_of(CarrierWave::DownloadError)
@instance.remote_image_url = "http://www.example.com/test.jpg"
expect(@instance.image_download_error).to be_nil
end
end

describe '#image_download_error' do
Expand Down
20 changes: 20 additions & 0 deletions spec/orm/activerecord_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ def monkey
describe "#remote_image_url=" do
before do
stub_request(:get, "http://www.example.com/test.jpg").to_return(body: File.read(file_path("test.jpg")))
stub_request(:get, "http://www.example.com/missing.jpg").to_return(status: 404)
end

# FIXME: ideally image_changed? and remote_image_url_changed? would return true
Expand All @@ -569,6 +570,15 @@ def monkey
expect(@event.image_changed?).to be_falsey
end

it "should clear validation errors from the previous attempt" do
@event.remote_image_url = 'http://www.example.com/missing.jpg'
expect(@event).to_not be_valid
expect(@event.errors[:image]).to eq(['could not download file: 404 ""'])
@event.remote_image_url = 'http://www.example.com/test.jpg'
expect(@event).to be_valid
expect(@event.errors).to be_empty
end

context 'when validating download' do
before do
@uploader.class_eval do
Expand Down Expand Up @@ -1570,6 +1580,7 @@ def monkey
describe "#remote_images_urls=" do
before do
stub_request(:get, "http://www.example.com/test.jpg").to_return(body: File.read(file_path("test.jpg")))
stub_request(:get, "http://www.example.com/missing.jpg").to_return(status: 404)
end

# FIXME: ideally images_changed? and remote_images_urls_changed? would return true
Expand All @@ -1582,6 +1593,15 @@ def monkey
expect(@event.images_changed?).to be_falsey
end

it "should clear validation errors from the previous attempt" do
@event.remote_images_urls = ['http://www.example.com/missing.jpg']
expect(@event).to_not be_valid
expect(@event.errors[:images]).to eq(['could not download file: 404 ""'])
@event.remote_images_urls = ['http://www.example.com/test.jpg']
expect(@event).to be_valid
expect(@event.errors).to be_empty
end

context 'when validating download' do
before do
@uploader.class_eval do
Expand Down

0 comments on commit bb8c6e1

Please sign in to comment.