Apply embed unlisted setting consistently (#24294)

Applies the embed_unlisted site setting consistently across topic embeds, including those created via the WP Discourse plugin. Relatedly, adds a embed exception to can_create_unlisted_topic? check. Users creating embedded topics are not always staff.

app/models/topic_embed.rb

@@ -72,19 +72,11 @@ class TopicEmbed < ActiveRecord::Base
           cook_method: cook_method,
           category: category_id || eh.try(:category_id),
           tags: SiteSetting.tagging_enabled ? tags : nil,
-        }
-        create_args[:visible] = false if SiteSetting.embed_unlisted?
-
-        creator = PostCreator.new(user, create_args)
-        post = creator.create
-        if post.present?
-          TopicEmbed.create!(
-            topic_id: post.topic_id,
           embed_url: url,
-            content_sha1: content_sha1,
+          embed_content_sha1: content_sha1,
-            post_id: post.id,
+        }
-          )
+
-        end
+        post = PostCreator.create(user, create_args)
       end
     else
       absolutize_urls(url, contents)

config/locales/server.en.yml

@@ -2294,7 +2294,7 @@ en:
     embed_topics_list: "Support HTML embedding of topics lists"
     embed_set_canonical_url: "Set the canonical URL for embedded topics to the embedded content's URL."
     embed_truncate: "Truncate the embedded posts."
-    embed_unlisted: "Imported topics will be unlisted until a user replies."
+    embed_unlisted: "Embedded topics will be unlisted until a user replies."
     embed_support_markdown: "Support Markdown formatting for embedded posts."
     allowed_embed_selectors: "A comma separated list of CSS elements that are allowed in embeds."
     allowed_href_schemes: "Schemes allowed in links in addition to http and https."

lib/guardian/topic_guardian.rb

@@ -198,7 +198,9 @@ module TopicGuardian
     can_moderate?(topic) || can_perform_action_available_to_group_moderators?(topic)
   end
 
-  alias can_create_unlisted_topic? can_toggle_topic_visibility?
+  def can_create_unlisted_topic?(topic, has_topic_embed = false)
+    can_toggle_topic_visibility?(topic) || has_topic_embed
+  end
 
   def can_convert_topic?(topic)
     return false if topic.blank?

lib/post_creator.rb

@@ -55,6 +55,8 @@ class PostCreator
   #     pinned_globally       - Is the topic pinned globally (optional)
   #     shared_draft          - Is the topic meant to be a shared draft
   #     topic_opts            - Options to be overwritten for topic
+  #     embed_url             - Creates a TopicEmbed for the topic
+  #     embed_content_sha1    - Sets the content_sha1 of the TopicEmbed
   #
   def initialize(user, opts)
     # TODO: we should reload user in case it is tainted, should take in a user_id as opposed to user
@@ -65,7 +67,10 @@ class PostCreator
 
     opts[:title] = pg_clean_up(opts[:title]) if opts[:title]&.include?("\u0000")
     opts[:raw] = pg_clean_up(opts[:raw]) if opts[:raw]&.include?("\u0000")
-    opts[:visible] = false if opts[:visible].nil? && opts[:hidden_reason_id].present?
+    opts[:visible] = false if (
+      (opts[:visible].nil? && opts[:hidden_reason_id].present?) ||
+        (opts[:embed_url].present? && SiteSetting.embed_unlisted?)
+    )
 
     opts.delete(:reply_to_post_number) unless opts[:topic_id]
   end
@@ -390,9 +395,6 @@ class PostCreator
     end
   end
 
-  # You can supply an `embed_url` for a post to set up the embedded relationship.
-  # This is used by the wp-discourse plugin to associate a remote post with a
-  # discourse post.
   def create_embedded_topic
     return unless @opts[:embed_url].present?
 
@@ -400,7 +402,12 @@ class PostCreator
     raise Discourse::InvalidParameters.new(:embed_url) unless original_uri.is_a?(URI::HTTP)
 
     embed =
-      TopicEmbed.new(topic_id: @post.topic_id, post_id: @post.id, embed_url: @opts[:embed_url])
+      TopicEmbed.new(
+        topic_id: @post.topic_id,
+        post_id: @post.id,
+        embed_url: @opts[:embed_url],
+        content_sha1: @opts[:embed_content_sha1],
+      )
     rollback_from_errors!(embed) unless embed.save
   end
 

lib/topic_creator.rb

@@ -67,7 +67,8 @@ class TopicCreator
   private
 
   def validate_visibility(topic)
-    if !@opts[:skip_validations] && !topic.visible && !guardian.can_create_unlisted_topic?(topic)
+    if !@opts[:skip_validations] && !topic.visible &&
+         !guardian.can_create_unlisted_topic?(topic, !!opts[:embed_url])
       topic.errors.add(:base, :unable_to_unlist)
     end
   end

spec/lib/post_creator_spec.rb

@@ -1464,8 +1464,15 @@ RSpec.describe PostCreator do
       creator.create
       expect(creator.errors).to be_blank
       expect(TopicEmbed.where(embed_url: embed_url).exists?).to eq(true)
+    end
 
-      # If we try to create another topic with the embed url, should fail
+    it "does not create topics with the same embed url" do
+      PostCreator.create(
+        user,
+        embed_url: embed_url,
+        title: "Reviews of Science Ovens",
+        raw: "Did you know that you can use microwaves to cook your dinner? Science!",
+      )
       creator =
         PostCreator.new(
           user,
@@ -1477,6 +1484,22 @@ RSpec.describe PostCreator do
       expect(result).to be_present
       expect(creator.errors).to be_present
     end
+
+    it "sets the embed content sha1" do
+      content = "Did you know that you can use microwaves to cook your dinner? Science!"
+      content_sha1 = Digest::SHA1.hexdigest(content)
+      creator =
+        PostCreator.new(
+          user,
+          embed_url: embed_url,
+          embed_content_sha1: content_sha1,
+          title: "Reviews of Science Ovens",
+          raw: content,
+        )
+      creator.create
+      expect(creator.errors).to be_blank
+      expect(TopicEmbed.where(content_sha1: content_sha1).exists?).to eq(true)
+    end
   end
 
   describe "read credit for creator" do

spec/lib/topic_creator_spec.rb

@@ -641,6 +641,18 @@ RSpec.describe TopicCreator do
       it "is valid for an admin" do
         expect(TopicCreator.new(admin, Guardian.new(admin), unlisted_attrs).valid?).to eq(true)
       end
+
+      context "when embedded" do
+        let(:embedded_unlisted_attrs) do
+          unlisted_attrs.merge(embed_url: "http://eviltrout.com/stupid-url")
+        end
+
+        it "is valid for a non-staff user" do
+          expect(TopicCreator.new(user, Guardian.new(user), embedded_unlisted_attrs).valid?).to eq(
+            true,
+          )
+        end
+      end
     end
   end
 end