FIX: Validate upload is still valid after calling the "before_upload_creation" event (#13091)

FIX: Validate upload is still valid after calling the “before_upload_creation” event (#13091)

Since we use the event to perform additional validations on the file, we should check if it added any errors to the upload before saving it. This change makes the UploadCreator more consistent since we no longer have to rely on exceptions.

diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 9e4f961..942b891 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -153,9 +153,10 @@ class UploadCreator
         @upload.assign_attributes(attrs)
       end
 
-      return @upload unless @upload.save(validate: @opts[:validate])
-
-      DiscourseEvent.trigger(:before_upload_creation, @file, is_image, @opts[:for_export])
+      # Callbacks using this event to validate the upload or the file must add errors to the
+      # upload errors object.
+      DiscourseEvent.trigger(:before_upload_creation, @file, is_image, @upload, @opts[:validate])
+      return @upload unless @upload.errors.empty? && @upload.save(validate: @opts[:validate])
 
       # store the file and update its url
       File.open(@file.path) do |f|
diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb
index 3fafb32..a9662ed 100644
--- a/spec/lib/upload_creator_spec.rb
+++ b/spec/lib/upload_creator_spec.rb
@@ -599,4 +599,30 @@ RSpec.describe UploadCreator do
       end
     end
   end
+
+  describe 'before_upload_creation event' do
+    let(:filename) { "logo.jpg" }
+    let(:file) { file_from_fixtures(filename) }
+
+    before do
+      setup_s3
+      stub_s3_store
+    end
+
+    it 'does not save the upload if an event added errors to the upload' do
+      error = 'This upload is invalid'
+
+      event = Proc.new do |file, is_image, upload|
+        upload.errors.add(:base, error)
+      end
+
+      DiscourseEvent.on(:before_upload_creation, &event)
+
+      created_upload = UploadCreator.new(file, filename).create_for(user.id)
+
+      expect(created_upload.persisted?).to eq(false)
+      expect(created_upload.errors).to contain_exactly(error)
+      DiscourseEvent.off(:before_upload_creation, &event)
+    end
+  end
 end

GitHub sha: fa57316a4e4545744178f1fba97f794186f89ac6

This commit appears in #13091 which was approved by eviltrout and SamSaffron. It was merged by romanrizzi.