From ac18a0753470cf51b98fd7cac1f3cec170a278cb Mon Sep 17 00:00:00 2001 From: shalinibhawsingka-kreeti <54796268+shalinibhawsingka-kreeti@users.noreply.github.com> Date: Mon, 30 Dec 2019 14:42:43 +0530 Subject: [PATCH] [bug] running validations callback so as to prevent post_processing on invalid attachments (#16) --- lib/paperclip/attachment.rb | 14 ++++++++++---- lib/paperclip/has_attached_file.rb | 2 +- lib/paperclip/validators.rb | 2 +- spec/paperclip/attachment_spec.rb | 25 +++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/paperclip/attachment.rb b/lib/paperclip/attachment.rb index de5c45ec7..553af1ed1 100644 --- a/lib/paperclip/attachment.rb +++ b/lib/paperclip/attachment.rb @@ -491,14 +491,20 @@ def process_options(options_type, style) #:nodoc: def post_process(*style_args) #:nodoc: return if @queued_for_write[:original].nil? - - instance.run_paperclip_callbacks(:post_process) do - instance.run_paperclip_callbacks(:"#{name}_post_process") do - post_process_styles(*style_args) if !@options[:check_validity_before_processing] || instance.errors.none? + if !@options[:check_validity_before_processing] || check_validity? + instance.run_paperclip_callbacks(:post_process) do + instance.run_paperclip_callbacks(:"#{name}_post_process") do + post_process_styles(*style_args) + end end end end + def check_validity? + instance.run_paperclip_callbacks(:"#{name}_validate") + instance.errors.none? + end + def post_process_styles(*style_args) #:nodoc: if styles.include?(:original) && process_style?(:original, style_args) post_process_style(:original, styles[:original]) diff --git a/lib/paperclip/has_attached_file.rb b/lib/paperclip/has_attached_file.rb index f48dd6992..d278ca064 100644 --- a/lib/paperclip/has_attached_file.rb +++ b/lib/paperclip/has_attached_file.rb @@ -103,7 +103,7 @@ def add_active_record_callbacks def add_paperclip_callbacks @klass.send( :define_paperclip_callbacks, - :post_process, :"#{@name}_post_process" + :post_process, :"#{@name}_post_process", :"#{@name}_validate" ) end diff --git a/lib/paperclip/validators.rb b/lib/paperclip/validators.rb index a7bf1812c..b2aa02097 100644 --- a/lib/paperclip/validators.rb +++ b/lib/paperclip/validators.rb @@ -64,7 +64,7 @@ def validate_before_processing(validator_class, options) def create_validating_before_filter(attribute, validator_class, options) if_clause = options.delete(:if) unless_clause = options.delete(:unless) - send(:"before_#{attribute}_post_process", if: if_clause, unless: unless_clause) do |*_args| + send(:"before_#{attribute}_validate", if: if_clause, unless: unless_clause) do |*_args| validator_class.new(options.dup).validate(self) end end diff --git a/spec/paperclip/attachment_spec.rb b/spec/paperclip/attachment_spec.rb index ff82c286a..ff1a7fca7 100644 --- a/spec/paperclip/attachment_spec.rb +++ b/spec/paperclip/attachment_spec.rb @@ -880,6 +880,31 @@ def do_after_all; end expect(Paperclip::Thumbnail).not_to receive(:make) @dummy.avatar = @file end + + it "should not call process hooks if validation fails" do + Dummy.class_eval do + validates_attachment_content_type :avatar, content_type: 'image/jpeg' + end + expect(@dummy).not_to receive(:do_before_avatar) + expect(@dummy).not_to receive(:do_after_avatar) + expect(@dummy).not_to receive(:do_before_all) + expect(@dummy).not_to receive(:do_after_all) + expect(Paperclip::Thumbnail).not_to receive(:make) + @dummy.avatar = @file + end + + it "should call process hooks if validation would fail but check validity flag is false" do + Dummy.class_eval do + validates_attachment_content_type :avatar, content_type: 'image/jpeg' + end + @dummy.avatar.options[:check_validity_before_processing] = false + expect(@dummy).to receive(:do_before_avatar) + expect(@dummy).to receive(:do_after_avatar) + expect(@dummy).to receive(:do_before_all) + expect(@dummy).to receive(:do_after_all) + expect(Paperclip::Thumbnail).to receive(:make).and_return(@file) + @dummy.avatar = @file + end end context "Assigning an attachment" do