FIX: Improvements for uploads:disable_secure_uploads task (#31231)

This commit contains a couple of improvements for this
rake task.

* We no longer limit the uploads to only ones with Post
  upload references, it doesn't matter what the secure uploads
  are linked to, they should all be un-secured
* We now only get distinct uploads from the initial query,
  multiple upload references on the same upload caused
  double ups and confusing counts for the task
* We now also disable the secure_uploads_pm_only site
  setting at the same time

lib/tasks/uploads.rake

@@ -597,13 +597,9 @@ task "uploads:disable_secure_uploads" => :environment do
     puts "Disabling secure upload and resetting uploads to not secure in #{db}...", ""
 
     SiteSetting.secure_uploads = false
+    SiteSetting.secure_uploads_pm_only = false
 
-    secure_uploads =
-      Upload
-        .joins(:upload_references)
-        .where(upload_references: { target_type: "Post" })
-        .where(secure: true)
-
+    secure_uploads = Upload.where(secure: true).distinct
     secure_upload_count = secure_uploads.count
 
     puts "", "Marking #{secure_upload_count} uploads as not secure.", ""

spec/fabricators/draft_fabricator.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+Fabricator(:draft) do
+  user { Fabricate(:user) }
+  transient :category_id
+  transient :topic
+  transient :post
+  transient :archetype
+  transient :tags
+  draft_key do |transients|
+    topic = transients[:topic] || transients[:post]&.topic
+
+    return "#{Draft::EXISTING_TOPIC}_#{topic.id}" if topic
+
+    if transients[:archetype] == "regular"
+      Draft::NEW_TOPIC
+    else
+      Draft::NEW_PRIVATE_MESSAGE
+    end
+  end
+  owner { SecureRandom.hex(10) }
+  revisions { sequence(:revisions) { |n| n } }
+  sequence { sequence(:sequence) { |n| n } }
+  data do |transients|
+    topic = transients[:topic] || transients[:post]&.topic
+
+    {
+      reply: "This is my really long draft",
+      action: topic.present? ? "reply" : "createTopic",
+      categoryId: transients[:category_id] || topic&.category_id,
+      tags: transients[:tags],
+      archetypeId: transients[:archetype],
+      metaData: nil,
+      composerTime: SecureRandom.random_number(10_000),
+      typingTime: SecureRandom.random_number(10_000),
+    }
+  end
+end

spec/tasks/uploads_spec.rb

@@ -197,7 +197,8 @@ RSpec.describe "tasks/uploads" do
       UploadReference.create(target: post_1, upload: upload_1)
       UploadReference.create(target: post_1, upload: upload_2)
       UploadReference.create(target: post_2, upload: upload_3)
-      UploadReference.create(target: post_2, upload: upload4)
+      UploadReference.create(target: post_2, upload: upload_4)
+      UploadReference.create(target: draft, upload: upload_6)
     end
 
     after do
@@ -206,14 +207,16 @@ RSpec.describe "tasks/uploads" do
       end
     end
 
-    let!(:uploads) { [upload_1, upload_2, upload_3, upload4, upload5] }
+    let!(:uploads) { [upload_1, upload_2, upload_3, upload_4, upload_5] }
     let(:post_1) { Fabricate(:post) }
     let(:post_2) { Fabricate(:post) }
+    let(:draft) { Fabricate(:draft) }
     let(:upload_1) { Fabricate(:upload_s3, secure: true, access_control_post: post_1) }
     let(:upload_2) { Fabricate(:upload_s3, secure: true, access_control_post: post_1) }
     let(:upload_3) { Fabricate(:upload_s3, secure: true, access_control_post: post_2) }
-    let(:upload4) { Fabricate(:upload_s3, secure: true, access_control_post: post_2) }
-    let(:upload5) { Fabricate(:upload_s3, secure: false) }
+    let(:upload_4) { Fabricate(:upload_s3, secure: true, access_control_post: post_2) }
+    let(:upload_5) { Fabricate(:upload_s3, secure: false) }
+    let(:upload_6) { Fabricate(:upload_s3, secure: true) }
 
     it "disables the secure upload setting" do
       invoke_task
@@ -222,7 +225,9 @@ RSpec.describe "tasks/uploads" do
 
     it "updates all secure uploads to secure: false" do
       invoke_task
-      [upload_1, upload_2, upload_3, upload4].each { |upl| expect(upl.reload.secure).to eq(false) }
+      [upload_1, upload_2, upload_3, upload_4, upload_6].each do |upl|
+        expect(upl.reload.secure).to eq(false)
+      end
     end
 
     it "sets the baked_version to NULL for affected posts" do
@@ -244,7 +249,7 @@ RSpec.describe "tasks/uploads" do
     it "updates the affected ACLs via the SyncAclsForUploads job" do
       invoke_task
       expect(Jobs::SyncAclsForUploads.jobs.last["args"][0]["upload_ids"]).to match_array(
-        [upload_1.id, upload_2.id, upload_3.id, upload4.id],
+        [upload_1.id, upload_2.id, upload_3.id, upload_4.id, upload_6.id],
       )
     end
   end