diff --git a/lib/carrierwave/uploader/store.rb b/lib/carrierwave/uploader/store.rb index 55d0c0640..a4d0d1e0b 100644 --- a/lib/carrierwave/uploader/store.rb +++ b/lib/carrierwave/uploader/store.rb @@ -14,6 +14,8 @@ def initialize(*) @file, @filename, @cache_id, @identifier, @deduplication_index = nil end } + + after :store, :show_warning_when_filename_is_unavailable end ## @@ -133,6 +135,16 @@ def full_filename(for_file) forcing_extension(for_file) end + def show_warning_when_filename_is_unavailable(_) + return if filename + + warn <<~MESSAGE + [WARNING] Your uploader's #filename method defined at #{method(:filename).source_location.join(':')} didn't return value after storing the file. + It's likely that the method is safeguarded with `if original_filename`, which were necessary for pre-3.x CarrierWave but is no longer necessary. + Removing it is recommended, as it is known to cause issues depending on the usecase: https://github.com/carrierwaveuploader/carrierwave/issues/2708#{' '} + MESSAGE + end + def storage @storage ||= self.class.storage.new(self) end diff --git a/spec/uploader/store_spec.rb b/spec/uploader/store_spec.rb index a3bdca975..5a6c6767c 100644 --- a/spec/uploader/store_spec.rb +++ b/spec/uploader/store_spec.rb @@ -247,6 +247,22 @@ def filename; "foo.jpg"; end end end + context "with a filename safeguarded by 'if original_filename'" do + before do + @uploader_class.class_eval do + def filename + "foo.jpg" if original_filename + end + end + end + + it "shows warning on store" do + expect(@uploader).to receive(:warn).with(/Your uploader's #filename method .+ didn't return value/) + @file = File.open(file_path('test.jpg')) + @uploader.store!(@file) + end + end + describe 'without a store dir' do before do @uploader_class.class_eval do