From 311b080447db36204f2de51aaba963dcb522b2d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Thu, 30 Nov 2023 16:48:25 +0100
Subject: [PATCH] Use named subjects in specs

---
 .../flag_quarantined_uploads_spec.rb          | 54 +++++++++----------
 .../remove_orphaned_scanned_uploads_spec.rb   | 27 +++++-----
 2 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/spec/jobs/scheduled/flag_quarantined_uploads_spec.rb b/spec/jobs/scheduled/flag_quarantined_uploads_spec.rb
index b64c9d8..9fd28b0 100644
--- a/spec/jobs/scheduled/flag_quarantined_uploads_spec.rb
+++ b/spec/jobs/scheduled/flag_quarantined_uploads_spec.rb
@@ -4,51 +4,49 @@
 
 describe Jobs::FlagQuarantinedUploads do
   describe "#execute" do
+    subject(:execute) { described_class.new.execute({}) }
+
     let(:scan_message) { "Win.Test.EICAR_HDB-1 FOUND" }
     let(:upload) { Fabricate(:upload) }
-
-    before do
-      SiteSetting.flag_malicious_uploads = true
-      @scanned_upload =
-        ScannedUpload.create!(upload: upload, quarantined: true, scan_result: scan_message)
+    let(:reviewable_upload) { ReviewableUpload.find_by(target: upload) }
+    let!(:scanned_upload) do
+      ScannedUpload.create!(upload: upload, quarantined: true, scan_result: scan_message)
     end
 
-    it "flags the upload if it was previously quarantined" do
-      subject.execute({})
-
-      reviewable_upload = ReviewableUpload.find_by(target: upload)
+    before { SiteSetting.flag_malicious_uploads = true }
 
-      expect(reviewable_upload).to be_present
+    it "flags the upload if it was previously quarantined" do
+      execute
       expect(reviewable_upload.payload["scan_message"]).to eq(scan_message)
     end
 
-    it "does nothing if the flag_malicious_uploads flag is disabled" do
-      SiteSetting.flag_malicious_uploads = false
-
-      subject.execute({})
-      reviewable_upload = ReviewableUpload.find_by(target: upload)
+    context "when the flag_malicious_uploads flag is disabled" do
+      before { SiteSetting.flag_malicious_uploads = false }
 
-      expect(reviewable_upload).to be_nil
+      it "does nothing" do
+        execute
+        expect(reviewable_upload).to be_nil
+      end
     end
 
-    it "does nothing if a reviewable already exists" do
-      @scanned_upload.flag_upload
-
-      subject.execute({})
+    context "when a reviewable already exists" do
+      before { scanned_upload.flag_upload }
 
-      reviewable_upload = ReviewableUpload.find_by(target: upload)
-      scores = reviewable_upload.reviewable_scores
+      it "does nothing" do
+        execute
 
-      expect(scores.size).to eq(1)
+        expect(reviewable_upload.reviewable_scores.size).to eq(1)
+      end
     end
 
-    it "does nothing if the scanned upload is not quarantined" do
-      @scanned_upload.update!(quarantined: false)
+    context "when the scanned upload is not quarantined" do
+      before { scanned_upload.update!(quarantined: false) }
 
-      subject.execute({})
-      reviewable_upload = ReviewableUpload.find_by(target: upload)
+      it "does nothing" do
+        execute
 
-      expect(reviewable_upload).to be_nil
+        expect(reviewable_upload).to be_nil
+      end
     end
   end
 end
diff --git a/spec/jobs/scheduled/remove_orphaned_scanned_uploads_spec.rb b/spec/jobs/scheduled/remove_orphaned_scanned_uploads_spec.rb
index 9b9a882..1e5d81f 100644
--- a/spec/jobs/scheduled/remove_orphaned_scanned_uploads_spec.rb
+++ b/spec/jobs/scheduled/remove_orphaned_scanned_uploads_spec.rb
@@ -3,25 +3,24 @@
 require "rails_helper"
 
 describe Jobs::RemoveOrphanedScannedUploads do
-  let(:upload) { Fabricate(:upload) }
+  subject(:execute) { described_class.new.execute({}) }
 
-  before do
-    SiteSetting.discourse_antivirus_enabled = true
-    @scanned_upload = ScannedUpload.create_new!(upload)
-  end
+  let(:upload) { Fabricate(:upload) }
+  let!(:scanned_upload) { ScannedUpload.create_new!(upload) }
 
-  it "does nothing if the upload still exists" do
-    subject.execute({})
+  before { SiteSetting.discourse_antivirus_enabled = true }
 
-    expect(@scanned_upload.reload).to be_present
+  context "when the upload still exists" do
+    it "does nothing" do
+      expect { execute }.not_to change { scanned_upload.reload }
+    end
   end
 
-  it "deletes the scanned upload if the upload is gone" do
-    upload.destroy!
-
-    subject.execute({})
-    orphaned_upload = ScannedUpload.find_by(id: @scanned_upload.id)
+  context "when the upload is gone" do
+    before { upload.destroy! }
 
-    expect(orphaned_upload).to be_nil
+    it "deletes the scanned upload" do
+      expect { execute }.to change { ScannedUpload.where(id: scanned_upload.id).count }.by(-1)
+    end
   end
 end