FEATURE: Extend embeddable hosts with Individual tags and author assignments (#26868)

* FEATURE: Extend embeddable hosts with tags and author assignments

app/assets/javascripts/admin/addon/components/embeddable-host.hbs

@@ -26,6 +26,25 @@
       class="small"
     />
   </td>
+  <td class="editing-input">
+    <div class="label">{{i18n "admin.embedding.tags"}}</div>
+    <TagChooser
+      @tags={{this.tags}}
+      @everyTag={{true}}
+      @excludeSynonyms={{true}}
+      @unlimitedTagCount={{true}}
+      @onChange={{fn (mut this.tags)}}
+      @options={{hash filterPlaceholder="category.tags_placeholder"}}
+    />
+  </td>
+  <td class="editing-input">
+    <div class="label">{{i18n "admin.embedding.user"}}</div>
+    <UserChooser
+      @value={{this.user}}
+      @onChange={{action "onUserChange"}}
+      @options={{hash maximum=1 excludeCurrentUser=false}}
+    />
+  </td>
   <td class="editing-controls">
     <DButton
       @icon="check"
@@ -55,6 +74,12 @@
     <div class="label">{{i18n "admin.embedding.category"}}</div>
     {{category-badge this.category allowUncategorized=true}}
   </td>
+  <td>
+    {{this.tags}}
+  </td>
+  <td>
+    {{this.user}}
+  </td>
   <td class="controls">
     <DButton @icon="pencil-alt" @action={{this.edit}} />
     <DButton @icon="far-trash-alt" @action={{this.delete}} class="btn-danger" />

app/assets/javascripts/admin/addon/components/embeddable-host.js

@@ -18,6 +18,8 @@ export default class EmbeddableHost extends Component.extend(
   editToggled = false;
   categoryId = null;
   category = null;
+  tags = null;
+  user = null;
 
   @or("host.isNew", "editToggled") editing;
 
@@ -29,6 +31,8 @@ export default class EmbeddableHost extends Component.extend(
     const category = Category.findById(categoryId);
 
     this.set("category", category);
+    this.set("tags", host.tags || []);
+    this.set("user", host.user);
   }
 
   @discourseComputed("buffered.host", "host.isSaving")
@@ -40,7 +44,10 @@ export default class EmbeddableHost extends Component.extend(
   edit() {
     this.set("editToggled", true);
   }
-
+  @action
+  onUserChange(user) {
+    this.set("user", user);
+  }
   @action
   save() {
     if (this.cantSave) {
@@ -53,6 +60,9 @@ export default class EmbeddableHost extends Component.extend(
       "class_name"
     );
     props.category_id = this.category.id;
+    props.tags = this.tags;
+    props.user =
+      Array.isArray(this.user) && this.user.length > 0 ? this.user[0] : null;
 
     const host = this.host;
 

app/assets/javascripts/admin/addon/templates/embedding.hbs

@@ -2,9 +2,18 @@
   {{#if this.embedding.embeddable_hosts}}
     <table class="embedding grid">
       <thead>
-        <th style="width: 30%">{{i18n "admin.embedding.host"}}</th>
-        <th style="width: 30%">{{i18n "admin.embedding.allowed_paths"}}</th>
-        <th style="width: 30%">{{i18n "admin.embedding.category"}}</th>
+        <th style="width: 18%">{{i18n "admin.embedding.host"}}</th>
+        <th style="width: 18%">{{i18n "admin.embedding.allowed_paths"}}</th>
+        <th style="width: 18%">{{i18n "admin.embedding.category"}}</th>
+        <th style="width: 18%">{{i18n "admin.embedding.tags"}}</th>
+        {{#if this.embedding.embed_by_username}}
+          <th style="width: 18%">{{i18n
+              "admin.embedding.post_author"
+              author=this.embedding.embed_by_username
+            }}</th>
+        {{else}}
+          <th style="width: 18%">{{i18n "admin.embedding.post_author"}}</th>
+        {{/if}}
         <th style="width: 10%">&nbsp;</th>
       </thead>
       <tbody>

app/assets/stylesheets/common/admin/customize.scss

@@ -878,6 +878,10 @@ table.permalinks {
   margin-bottom: 2em;
   table.grid {
     margin-bottom: 1em;
+    .tag-chooser,
+    .user-chooser {
+      width: 100%;
+    }
     tr td {
       word-wrap: break-word;
       max-width: 25vw;

app/controllers/admin/embeddable_hosts_controller.rb

@@ -28,21 +28,56 @@ class Admin::EmbeddableHostsController < Admin::AdminController
     host.category_id = params[:embeddable_host][:category_id]
     host.category_id = SiteSetting.uncategorized_category_id if host.category_id.blank?
 
-    if host.save
-      changes = host.saved_changes if action == :update
-      StaffActionLogger.new(current_user).log_embeddable_host(
-        host,
-        UserHistory.actions[:"embeddable_host_#{action}"],
-        changes: changes,
-      )
-      render_serialized(
-        host,
-        EmbeddableHostSerializer,
-        root: "embeddable_host",
-        rest_serializer: true,
-      )
+    username = params[:embeddable_host][:user]
+
+    if username.blank?
+      host.user = nil
     else
-      render_json_error(host)
+      host.user = User.find_by_username(username)
+    end
+
+    ActiveRecord::Base.transaction do
+      if host.save
+        manage_tags(host, params[:embeddable_host][:tags]&.map(&:strip))
+
+        changes = host.saved_changes if action == :update
+        StaffActionLogger.new(current_user).log_embeddable_host(
+          host,
+          UserHistory.actions[:"embeddable_host_#{action}"],
+          changes: changes,
+        )
+        render_serialized(
+          host,
+          EmbeddableHostSerializer,
+          root: "embeddable_host",
+          rest_serializer: true,
+        )
+      else
+        render_json_error(host)
+        raise ActiveRecord::Rollback
+      end
     end
   end
+
+  def manage_tags(host, tags)
+    if tags.blank?
+      host.tags.clear
+      return
+    end
+
+    existing_tags = Tag.where(name: tags).index_by(&:name)
+    tags_to_associate = []
+
+    tags.each do |tag_name|
+      tag = existing_tags[tag_name] || Tag.create(name: tag_name)
+      if tag.persisted?
+        tags_to_associate << tag
+      else
+        host.errors.add(:tags, "Failed to create or find tag: #{tag_name}")
+        raise ActiveRecord::Rollback
+      end
+    end
+
+    host.tags = tags_to_associate
+  end
 end

app/models/embeddable_host.rb

@@ -3,6 +3,9 @@
 class EmbeddableHost < ActiveRecord::Base
   validate :host_must_be_valid
   belongs_to :category
+  belongs_to :user, optional: true
+  has_many :embeddable_host_tags
+  has_many :tags, through: :embeddable_host_tags
   after_destroy :reset_embedding_settings
 
   before_validation do
@@ -81,4 +84,5 @@ end
 #  updated_at    :datetime         not null
 #  class_name    :string
 #  allowed_paths :string
+#  user_id       :integer
 #

app/models/embeddable_host_tag.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class EmbeddableHostTag < ActiveRecord::Base
+  belongs_to :embeddable_host
+  belongs_to :tag
+
+  validates :embeddable_host_id, presence: true
+  validates :tag_id, presence: true
+  validates :embeddable_host_id, uniqueness: { scope: :tag_id }
+end
+
+# == Schema Information
+#
+# Table name: embeddable_host_tags
+#
+#  id                 :bigint           not null, primary key
+#  embeddable_host_id :integer          not null
+#  tag_id             :integer          not null
+#  created_at         :datetime         not null
+#  updated_at         :datetime         not null
+#
+# Indexes
+#
+#  index_embeddable_host_tags_on_embeddable_host_id             (embeddable_host_id)
+#  index_embeddable_host_tags_on_embeddable_host_id_and_tag_id  (embeddable_host_id,tag_id) UNIQUE
+#  index_embeddable_host_tags_on_tag_id                         (tag_id)
+#

app/models/tag.rb

@@ -57,6 +57,9 @@ class Tag < ActiveRecord::Base
   has_many :synonyms, class_name: "Tag", foreign_key: "target_tag_id", dependent: :destroy
   has_many :sidebar_section_links, as: :linkable, dependent: :delete_all
 
+  has_many :embeddable_host_tags
+  has_many :embeddable_hosts, through: :embeddable_host_tags
+
   before_save :sanitize_description
 
   after_save :index_search

app/models/topic_embed.rb

@@ -60,7 +60,10 @@ class TopicEmbed < ActiveRecord::Base
     # If there is no embed, create a topic, post and the embed.
     if embed.blank?
       Topic.transaction do
-        eh = EmbeddableHost.record_for_url(url)
+        if eh = EmbeddableHost.record_for_url(url)
+          tags = eh.tags.presence || tags
+          user = eh.user.presence || user
+        end
 
         cook_method ||=
           if SiteSetting.embed_support_markdown
@@ -99,6 +102,11 @@ class TopicEmbed < ActiveRecord::Base
       absolutize_urls(url, contents)
       post = embed.post
 
+      if eh = EmbeddableHost.record_for_url(url)
+        tags = eh.tags.presence || tags
+        user = eh.user.presence || user
+      end
+
       # Update the topic if it changed
       if post&.topic
         if post.user != user
@@ -113,8 +121,16 @@ class TopicEmbed < ActiveRecord::Base
           post.reload
         end
 
-        if (content_sha1 != embed.content_sha1) || (title && title != post&.topic&.title)
+        existing_tag_names = post.topic.tags.pluck(:name).sort
+        incoming_tag_names = Array(tags).map(&:name).sort
+
+        tags_changed = existing_tag_names != incoming_tag_names
+
+        if (content_sha1 != embed.content_sha1) || (title && title != post&.topic&.title) ||
+             tags_changed
           changes = { raw: absolutize_urls(url, contents) }
+
+          changes[:tags] = incoming_tag_names if SiteSetting.tagging_enabled && tags_changed
           changes[:title] = title if title.present?
 
           post.revise(user, changes, skip_validations: true, bypass_rate_limiter: true)

app/models/user.rb

@@ -141,6 +141,7 @@ class User < ActiveRecord::Base
   belongs_to :uploaded_avatar, class_name: "Upload"
 
   has_many :sidebar_section_links, dependent: :delete_all
+  has_many :embeddable_hosts
 
   delegate :last_sent_email_address, to: :email_logs
 

app/serializers/embeddable_host_serializer.rb

@@ -1,9 +1,17 @@
 # frozen_string_literal: true
 
 class EmbeddableHostSerializer < ApplicationSerializer
-  TO_SERIALIZE = %i[id host allowed_paths class_name category_id]
+  TO_SERIALIZE = %i[id host allowed_paths class_name category_id tags user]
 
   attributes *TO_SERIALIZE
 
   TO_SERIALIZE.each { |attr| define_method(attr) { object.public_send(attr) } }
+
+  def user
+    object.user&.username
+  end
+
+  def tags
+    object.tags.map(&:name)
+  end
 end

config/locales/client.en.yml

@@ -6816,6 +6816,8 @@ en:
         allowed_paths: "Path Allowlist"
         edit: "edit"
         category: "Post to Category"
+        tags: "Topic Tags"
+        post_author: "Post Author - Defaults to %{author}"
         add_host: "Add Host"
         settings: "Embedding Settings"
         crawling_settings: "Crawler Settings"

db/migrate/20240430051551_add_user_id_to_embeddable_hosts.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+class AddUserIdToEmbeddableHosts < ActiveRecord::Migration[7.0]
+  def change
+    add_column :embeddable_hosts, :user_id, :integer
+  end
+end

db/migrate/20240430052017_create_embeddable_host_tags.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+class CreateEmbeddableHostTags < ActiveRecord::Migration[7.0]
+  def change
+    create_table :embeddable_host_tags do |t|
+      t.integer :embeddable_host_id, null: false
+      t.integer :tag_id, null: false
+
+      t.timestamps
+    end
+
+    add_index :embeddable_host_tags, :embeddable_host_id
+    add_index :embeddable_host_tags, :tag_id
+    add_index :embeddable_host_tags, %i[embeddable_host_id tag_id], unique: true
+  end
+end

spec/fabricators/embeddable_host_tag_fabricator.rb

@@ -0,0 +1,6 @@
+#frozen_string_literal: true
+
+Fabricator(:embeddable_host_tag) do
+  embeddable_host
+  tag
+end

spec/models/embeddable_host_tag_spec.rb

@@ -0,0 +1,58 @@
+#frozen_string_literal: true
+
+RSpec.describe EmbeddableHostTag, type: :model do
+  describe "associations" do
+    it "belongs to an embeddable_host" do
+      expect(described_class.reflect_on_association(:embeddable_host).macro).to eq(:belongs_to)
+    end
+
+    it "belongs to a tag" do
+      expect(described_class.reflect_on_association(:tag).macro).to eq(:belongs_to)
+    end
+  end
+
+  describe "validations" do
+    subject { Fabricate(:embeddable_host_tag) }
+
+    it { is_expected.to validate_presence_of(:embeddable_host_id) }
+    it { is_expected.to validate_presence_of(:tag_id) }
+    it { is_expected.to validate_uniqueness_of(:embeddable_host_id).scoped_to(:tag_id) }
+  end
+
+  describe "functionality" do
+    context "when creating valid associations" do
+      let(:embeddable_host) { Fabricate(:embeddable_host) }
+      let(:tag) { Fabricate(:tag) }
+
+      it "successfully creates an embeddable_host_tag with valid inputs" do
+        host_tag = EmbeddableHostTag.new(embeddable_host: embeddable_host, tag: tag)
+        expect(host_tag.save).to be true
+      end
+    end
+
+    context "when attempting to create duplicate associations" do
+      let(:embeddable_host) { Fabricate(:embeddable_host) }
+      let(:tag) { Fabricate(:tag) }
+
+      before { EmbeddableHostTag.create!(embeddable_host: embeddable_host, tag: tag) }
+
+      it "prevents duplicate embeddable_host_tag from being saved" do
+        duplicate_host_tag = EmbeddableHostTag.new(embeddable_host: embeddable_host, tag: tag)
+        expect(duplicate_host_tag.valid?).to be false
+        expect(duplicate_host_tag.errors[:embeddable_host_id]).to include("has already been taken")
+      end
+    end
+
+    context "with missing fields" do
+      it "fails to save without an embeddable_host" do
+        host_tag = EmbeddableHostTag.new(tag: Fabricate(:tag))
+        expect(host_tag.valid?).to be false
+      end
+
+      it "fails to save without a tag" do
+        host_tag = EmbeddableHostTag.new(embeddable_host: Fabricate(:embeddable_host))
+        expect(host_tag.valid?).to be false
+      end
+    end
+  end
+end

spec/models/topic_embed_spec.rb

@@ -299,6 +299,53 @@ RSpec.describe TopicEmbed do
       end
     end
 
+    context "with specified user and tags" do
+      fab!(:tag1) { Fabricate(:tag, name: "interesting") }
+      fab!(:tag2) { Fabricate(:tag, name: "article") }
+
+      let!(:new_user) { Fabricate(:user) }
+      let(:tags) { [tag1.name, tag2.name] }
+      let(:imported_post) { TopicEmbed.import(new_user, url, title, contents, tags: tags) }
+
+      it "assigns the specified user as the author" do
+        expect(imported_post.user).to eq(new_user)
+      end
+
+      it "associates the specified tags with the topic" do
+        expect(imported_post.topic.tags).to contain_exactly(tag1, tag2)
+      end
+    end
+
+    context "when updating an existing post with new tags and a different user" do
+      fab!(:tag1) { Fabricate(:tag, name: "interesting") }
+      fab!(:tag2) { Fabricate(:tag, name: "article") }
+
+      let!(:admin) { Fabricate(:admin) }
+      let!(:new_admin) { Fabricate(:admin) }
+      let(:tags) { [tag1.name, tag2.name] }
+
+      before { SiteSetting.tagging_enabled = true }
+
+      it "updates the user and adds new tags" do
+        original_post = TopicEmbed.import(admin, url, title, contents)
+
+        expect(original_post.user).to eq(admin)
+        expect(original_post.topic.tags).to be_empty
+
+        embeddable_host.update!(
+          tags: [tag1, tag2],
+          user: new_admin,
+          category: category,
+          host: "eviltrout.com",
+        )
+
+        edited_post = TopicEmbed.import(admin, url, title, contents)
+
+        expect(edited_post.user).to eq(new_admin)
+        expect(edited_post.topic.tags).to match_array([tag1, tag2])
+      end
+    end
+
     describe "embedded content truncation" do
       MAX_LENGTH_BEFORE_TRUNCATION = 100
 

spec/requests/admin/embeddable_hosts_controller_spec.rb

@@ -21,6 +21,68 @@ RSpec.describe Admin::EmbeddableHostsController do
           ).exists?,
         ).to eq(true)
       end
+
+      it "creates an embeddable host with associated tags" do
+        tag1 = Fabricate(:tag, name: "tag1")
+        tag2 = Fabricate(:tag, name: "tag2")
+
+        post "/admin/embeddable_hosts.json",
+             params: {
+               embeddable_host: {
+                 host: "example.com",
+                 tags: %w[tag1 tag2],
+               },
+             }
+
+        expect(response.status).to eq(200)
+        expect(EmbeddableHost.last.tags).to contain_exactly(tag1, tag2)
+      end
+
+      it "updates an embeddable host with associated tags" do
+        tag1 = Fabricate(:tag, name: "newTag1")
+        tag2 = Fabricate(:tag, name: "newTag2")
+
+        put "/admin/embeddable_hosts/#{embeddable_host.id}.json",
+            params: {
+              embeddable_host: {
+                host: "updated-example.com",
+                tags: %w[newTag1 newTag2],
+              },
+            }
+
+        expect(response.status).to eq(200)
+        expect(EmbeddableHost.find(embeddable_host.id).tags).to contain_exactly(tag1, tag2)
+      end
+
+      it "creates an embeddable host with an associated author" do
+        user = Fabricate(:user, username: "johndoe")
+
+        post "/admin/embeddable_hosts.json",
+             params: {
+               embeddable_host: {
+                 host: "example.com",
+                 user: "johndoe",
+               },
+             }
+
+        expect(response.status).to eq(200)
+        expect(EmbeddableHost.last.user).to eq(user)
+      end
+
+      it "updates an embeddable host with a new author" do
+        new_user = Fabricate(:user, username: "johndoe")
+
+        put "/admin/embeddable_hosts/#{embeddable_host.id}.json",
+            params: {
+              embeddable_host: {
+                host: "updated-example.com",
+                user: "johndoe",
+              },
+            }
+
+        expect(response.status).to eq(200)
+        expect(EmbeddableHost.find(embeddable_host.id).user).to eq(new_user)
+      end
     end
 
     shared_examples "embeddable host creation not allowed" do

spec/system/admin_embeddable_hosts_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+RSpec.describe "Admin EmbeddableHost Management", type: :system do
+  fab!(:admin)
+  fab!(:author) { Fabricate(:admin) }
+  fab!(:category)
+  fab!(:category2) { Fabricate(:category) }
+  fab!(:tag)
+  fab!(:tag2) { Fabricate(:tag) }
+
+  before { sign_in(admin) }
+
+  it "allows admin to add and edit embeddable hosts" do
+    visit "/admin/customize/embedding"
+
+    find("button.btn-icon-text", text: "Add Host").click
+    within find("tr.ember-view") do
+      find('input[placeholder="example.com"]').set("awesome-discourse-site.local")
+      find('input[placeholder="/blog/.*"]').set("/blog/.*")
+
+      category_chooser = PageObjects::Components::SelectKit.new(".category-chooser")
+      category_chooser.expand
+      category_chooser.select_row_by_name(category.name)
+
+      tag_chooser = PageObjects::Components::SelectKit.new(".tag-chooser")
+      tag_chooser.expand
+      tag_chooser.select_row_by_name(tag.name)
+
+      find(".user-chooser").click
+      find(".select-kit-body .select-kit-filter input").fill_in with: author.username
+      find(".select-kit-body", text: author.username).click
+    end
+    find("td.editing-controls .btn.btn-primary").click
+    expect(page).to have_content("awesome-discourse-site.local")
+    expect(page).to have_content("/blog/.*")
+    expect(page).not_to have_content("#{tag.name},#{tag2.name}")
+    expect(page).to have_content("#{tag.name}")
+
+    # Editing
+
+    find(".embeddable-hosts tr:first-child .controls svg.d-icon-pencil-alt").find(
+      :xpath,
+      "..",
+    ).click
+
+    within find(".embeddable-hosts tr:first-child.ember-view") do
+      find('input[placeholder="example.com"]').set("updated-example.com")
+      find('input[placeholder="/blog/.*"]').set("/updated-blog/.*")
+
+      category_chooser = PageObjects::Components::SelectKit.new(".category-chooser")
+      category_chooser.expand
+      category_chooser.select_row_by_name(category2.name)
+
+      tag_chooser = PageObjects::Components::SelectKit.new(".tag-chooser")
+      tag_chooser.expand
+      tag_chooser.select_row_by_name(tag2.name)
+    end
+
+    find("td.editing-controls .btn.btn-primary").click
+    expect(page).to have_content("updated-example.com")
+    expect(page).to have_content("/updated-blog/.*")
+    expect(page).to have_content("#{tag.name},#{tag2.name}")
+  end
+end