DEV: Rename with_secure_uploads? to should_secure_uploads? on Post (#26549)

This method name is a bit confusing; with_secure_uploads implies
it may return a block or something with the uploads of the post,
and has_secure_uploads implies that it's checking whether the post
is linked to any secure uploads.

should_secure_uploads? communicates the true intent of this method --
which is to say whether uploads attached to this post should be
secure or not.

app/models/post.rb

@@ -326,7 +326,7 @@ class Post < ActiveRecord::Base
     options[:user_id] = self.last_editor_id
     options[:omit_nofollow] = true if omit_nofollow?
 
-    if self.with_secure_uploads?
+    if self.should_secure_uploads?
       each_upload_url do |url|
         uri = URI.parse(url)
         if FileHelper.is_supported_media?(File.basename(uri.path))
@@ -565,7 +565,7 @@ class Post < ActiveRecord::Base
     ReviewableFlaggedPost.pending.find_by(target: self)
   end
 
-  def with_secure_uploads?
+  def should_secure_uploads?
     return false if !SiteSetting.secure_uploads?
     topic_including_deleted = Topic.with_deleted.find_by(id: self.topic_id)
     return false if topic_including_deleted.blank?

lib/cooked_post_processor.rb

@@ -22,7 +22,7 @@ class CookedPostProcessor
     @cooking_options = post.cooking_options || opts[:cooking_options] || {}
     @cooking_options[:topic_id] = post.topic_id
     @cooking_options = @cooking_options.symbolize_keys
-    @with_secure_uploads = @post.with_secure_uploads?
+    @should_secure_uploads = @post.should_secure_uploads?
     @category_id = @post&.topic&.category_id
 
     cooked = post.cook(post.raw, @cooking_options)
@@ -243,16 +243,16 @@ class CookedPostProcessor
           resized_h = (h * ratio).to_i
 
           if !cropped && upload.width && resized_w > upload.width
-            cooked_url = UrlHelper.cook_url(upload.url, secure: @post.with_secure_uploads?)
+            cooked_url = UrlHelper.cook_url(upload.url, secure: @should_secure_uploads)
             srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0\z/, "")}x"
           elsif t = upload.thumbnail(resized_w, resized_h)
-            cooked_url = UrlHelper.cook_url(t.url, secure: @post.with_secure_uploads?)
+            cooked_url = UrlHelper.cook_url(t.url, secure: @should_secure_uploads)
             srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0\z/, "")}x"
           end
 
           img[
             "srcset"
-          ] = "#{UrlHelper.cook_url(img["src"], secure: @post.with_secure_uploads?)}#{srcset}" if srcset.present?
+          ] = "#{UrlHelper.cook_url(img["src"], secure: @should_secure_uploads)}#{srcset}" if srcset.present?
         end
       end
     else
@@ -273,7 +273,7 @@ class CookedPostProcessor
 
     # then, the link to our larger image
     src_url = Upload.secure_uploads_url?(img["src"]) ? upload&.url : img["src"]
-    src = UrlHelper.cook_url(src_url || img["src"], secure: @post.with_secure_uploads?)
+    src = UrlHelper.cook_url(src_url || img["src"], secure: @should_secure_uploads)
     a = create_link_node("lightbox", src)
     img.add_next_sibling(a)
 
@@ -348,7 +348,7 @@ class CookedPostProcessor
           custom_emoji = img["class"]&.include?("emoji-custom") && Emoji.custom?(img["title"])
           img[selector] = UrlHelper.cook_url(
             img[selector].to_s,
-            secure: @post.with_secure_uploads? && !custom_emoji,
+            secure: @should_secure_uploads && !custom_emoji,
           )
         end
     end
@@ -415,7 +415,7 @@ class CookedPostProcessor
 
       still_an_image = false
     elsif info&.downloaded? && upload = info&.upload
-      img["src"] = UrlHelper.cook_url(upload.url, secure: @with_secure_uploads)
+      img["src"] = UrlHelper.cook_url(upload.url, secure: @should_secure_uploads)
       img["data-dominant-color"] = upload.dominant_color(calculate_if_missing: true).presence
       img.delete(PrettyText::BLOCKED_HOTLINKED_SRC_ATTR)
     end

lib/pretty_text.rb

@@ -653,7 +653,7 @@ module PrettyText
   def self.format_for_email(html, post = nil)
     doc = Nokogiri::HTML5.fragment(html)
     DiscourseEvent.trigger(:reduce_cooked, doc, post)
-    strip_secure_uploads(doc) if post&.with_secure_uploads?
+    strip_secure_uploads(doc) if post&.should_secure_uploads?
     strip_image_wrapping(doc)
     convert_vimeo_iframes(doc)
     make_all_links_absolute(doc)

lib/tasks/uploads.rake

@@ -604,7 +604,7 @@ task "uploads:secure_upload_analyse_and_update" => :environment do
       # If secure upload is enabled we need to first set the access control post of
       # all post uploads (even uploads that are linked to multiple posts). If the
       # upload is not set to secure upload then this has no other effect on the upload,
-      # but we _must_ know what the access control post is because the with_secure_uploads?
+      # but we _must_ know what the access control post is because the should_secure_uploads?
       # method is on the post, and this knows about the category security & PM status
       update_uploads_access_control_post if SiteSetting.secure_uploads?
 

lib/upload_security.rb

@@ -101,7 +101,7 @@ class UploadSecurity
   def secure_context_checks
     {
       login_required: "login is required",
-      access_control_post_has_secure_uploads: "access control post dictates security",
+      access_control_post_should_secure_uploads: "access control post dictates security",
       secure_creation_for_modifiers: "one or more creation for_modifiers was satisfied",
       uploading_in_composer: "uploading via the composer",
       already_secure: "upload is already secure",
@@ -112,7 +112,7 @@ class UploadSecurity
   # of whether an upload should be secure or not, and thus should be returned
   # immediately if there is an access control post.
   def priority_check?(check)
-    check == :access_control_post_has_secure_uploads && access_control_post
+    check == :access_control_post_should_secure_uploads && access_control_post
   end
 
   def perform_check(check)
@@ -169,15 +169,15 @@ class UploadSecurity
   # Whether the upload should remain secure or not after posting depends on its context,
   # which is based on the post it is linked to via access_control_post_id.
   #
-  # If that post is with_secure_uploads? then the upload should also be secure.
+  # If that post should_secure_uploads? then the upload should also be secure.
   #
   # This may change to false if the upload was set to secure on upload e.g. in
   # a post composer then it turned out that the post itself was not in a secure context.
   #
   # A post is with secure uploads if it is a private message or in a read restricted
-  # category. See `Post#with_secure_uploads?` for the full definition.
-  def access_control_post_has_secure_uploads_check
-    access_control_post&.with_secure_uploads?
+  # category. See `Post#should_secure_uploads?` for the full definition.
+  def access_control_post_should_secure_uploads_check
+    access_control_post&.should_secure_uploads?
   end
 
   def uploading_in_composer_check

plugins/chat/lib/chat/message_processor.rb

@@ -8,7 +8,7 @@ module Chat
     def initialize(chat_message, opts = {})
       @model = chat_message
       @previous_cooked = (chat_message.cooked || "").dup
-      @with_secure_uploads = false
+      @should_secure_uploads = false
       @size_cache = {}
       @opts = opts
 

spec/lib/cooked_post_processor_spec.rb

@@ -1160,7 +1160,7 @@ RSpec.describe CookedPostProcessor do
         Oneboxer.unstub(:onebox)
       end
 
-      context "when the post is with_secure_uploads and the upload is secure and secure uploads is enabled" do
+      context "when the post is should_secure_uploads and the upload is secure and secure uploads is enabled" do
         before do
           setup_s3
           upload.update(secure: true)

spec/models/post_spec.rb

@@ -148,12 +148,12 @@ RSpec.describe Post do
     end
   end
 
-  describe "with_secure_uploads?" do
+  describe "should_secure_uploads?" do
     let(:topic) { Fabricate(:topic) }
     let!(:post) { Fabricate(:post, topic: topic) }
 
     it "returns false if secure uploads is not enabled" do
-      expect(post.with_secure_uploads?).to eq(false)
+      expect(post.should_secure_uploads?).to eq(false)
     end
 
     context "when secure uploads is enabled" do
@@ -167,14 +167,14 @@ RSpec.describe Post do
         before { SiteSetting.login_required = true }
 
         it "returns true" do
-          expect(post.with_secure_uploads?).to eq(true)
+          expect(post.should_secure_uploads?).to eq(true)
         end
 
         context "if secure_uploads_pm_only" do
           before { SiteSetting.secure_uploads_pm_only = true }
 
           it "returns false" do
-            expect(post.with_secure_uploads?).to eq(false)
+            expect(post.should_secure_uploads?).to eq(false)
           end
         end
       end
@@ -184,7 +184,7 @@ RSpec.describe Post do
         before { topic.change_category_to_id(category.id) }
 
         it "returns true" do
-          expect(post.with_secure_uploads?).to eq(true)
+          expect(post.should_secure_uploads?).to eq(true)
         end
 
         context "when the topic is deleted" do
@@ -194,7 +194,7 @@ RSpec.describe Post do
           end
 
           it "returns true" do
-            expect(post.with_secure_uploads?).to eq(true)
+            expect(post.should_secure_uploads?).to eq(true)
           end
         end
 
@@ -202,7 +202,7 @@ RSpec.describe Post do
           before { SiteSetting.secure_uploads_pm_only = true }
 
           it "returns false" do
-            expect(post.with_secure_uploads?).to eq(false)
+            expect(post.should_secure_uploads?).to eq(false)
           end
         end
       end
@@ -211,14 +211,14 @@ RSpec.describe Post do
         let(:topic) { Fabricate(:private_message_topic) }
 
         it "returns true" do
-          expect(post.with_secure_uploads?).to eq(true)
+          expect(post.should_secure_uploads?).to eq(true)
         end
 
         context "when the topic is deleted" do
           before { topic.trash! }
 
           it "returns true" do
-            expect(post.with_secure_uploads?).to eq(true)
+            expect(post.should_secure_uploads?).to eq(true)
           end
         end
 
@@ -226,7 +226,7 @@ RSpec.describe Post do
           before { SiteSetting.secure_uploads_pm_only = true }
 
           it "returns true" do
-            expect(post.with_secure_uploads?).to eq(true)
+            expect(post.should_secure_uploads?).to eq(true)
           end
         end
       end