From bb8c6e1f9eccae8650c70ab46ca487ec878cda0b Mon Sep 17 00:00:00 2001 From: Mitsuhiro Shibuya Date: Mon, 14 Oct 2024 18:46:36 +0900 Subject: [PATCH] Clear download errors on reassigning remote urls Closes #2725 --- lib/carrierwave/mounter.rb | 1 + spec/mount_multiple_spec.rb | 9 +++++++++ spec/mount_single_spec.rb | 7 +++++++ spec/orm/activerecord_spec.rb | 20 ++++++++++++++++++++ 4 files changed, 37 insertions(+) diff --git a/lib/carrierwave/mounter.rb b/lib/carrierwave/mounter.rb index 44044d7b9..a0a0dc095 100644 --- a/lib/carrierwave/mounter.rb +++ b/lib/carrierwave/mounter.rb @@ -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 diff --git a/spec/mount_multiple_spec.rb b/spec/mount_multiple_spec.rb index 1fd4c6109..b1d3c23cf 100644 --- a/spec/mount_multiple_spec.rb +++ b/spec/mount_multiple_spec.rb @@ -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 diff --git a/spec/mount_single_spec.rb b/spec/mount_single_spec.rb index ed1ecbd38..a1a66c3d3 100644 --- a/spec/mount_single_spec.rb +++ b/spec/mount_single_spec.rb @@ -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 diff --git a/spec/orm/activerecord_spec.rb b/spec/orm/activerecord_spec.rb index 535656e42..c89252ae3 100644 --- a/spec/orm/activerecord_spec.rb +++ b/spec/orm/activerecord_spec.rb @@ -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 @@ -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 @@ -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 @@ -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