diff --git a/app/assets/javascripts/admin/addon/components/embeddable-host.hbs b/app/assets/javascripts/admin/addon/components/embeddable-host.hbs index 9ab723092fcc..f0a7d06f6871 100644 --- a/app/assets/javascripts/admin/addon/components/embeddable-host.hbs +++ b/app/assets/javascripts/admin/addon/components/embeddable-host.hbs @@ -26,6 +26,25 @@ class="small" /> + +
{{i18n "admin.embedding.tags"}}
+ + + +
{{i18n "admin.embedding.user"}}
+ + {{i18n "admin.embedding.category"}} {{category-badge this.category allowUncategorized=true}} + + {{this.tags}} + + + {{this.user}} + diff --git a/app/assets/javascripts/admin/addon/components/embeddable-host.js b/app/assets/javascripts/admin/addon/components/embeddable-host.js index 12bd423983d0..6eba3399073a 100644 --- a/app/assets/javascripts/admin/addon/components/embeddable-host.js +++ b/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; diff --git a/app/assets/javascripts/admin/addon/templates/embedding.hbs b/app/assets/javascripts/admin/addon/templates/embedding.hbs index f9301df3a9e8..7cf572a4aba1 100644 --- a/app/assets/javascripts/admin/addon/templates/embedding.hbs +++ b/app/assets/javascripts/admin/addon/templates/embedding.hbs @@ -2,9 +2,18 @@ {{#if this.embedding.embeddable_hosts}} - - - + + + + + {{#if this.embedding.embed_by_username}} + + {{else}} + + {{/if}} diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index e2ad75b3cdbb..06cbf9b55905 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/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; diff --git a/app/controllers/admin/embeddable_hosts_controller.rb b/app/controllers/admin/embeddable_hosts_controller.rb index 56b44de378f3..c8edbb71d5cf 100644 --- a/app/controllers/admin/embeddable_hosts_controller.rb +++ b/app/controllers/admin/embeddable_hosts_controller.rb @@ -28,21 +28,56 @@ def save_host(host, action) 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 diff --git a/app/models/embeddable_host.rb b/app/models/embeddable_host.rb index 2b25be6e5819..90dcb85b1232 100644 --- a/app/models/embeddable_host.rb +++ b/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 @@ def host_must_be_valid # updated_at :datetime not null # class_name :string # allowed_paths :string +# user_id :integer # diff --git a/app/models/embeddable_host_tag.rb b/app/models/embeddable_host_tag.rb new file mode 100644 index 000000000000..0ee9ada4f12f --- /dev/null +++ b/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) +# diff --git a/app/models/tag.rb b/app/models/tag.rb index 7a935be68af5..0d0b3917ef1d 100644 --- a/app/models/tag.rb +++ b/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 diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 17aaab835fa3..8d5263f8d025 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -60,7 +60,10 @@ def self.import(user, url, title, contents, category_id: nil, cook_method: nil, # 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 @@ def self.import(user, url, title, contents, category_id: nil, cook_method: nil, 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 @@ def self.import(user, url, title, contents, category_id: nil, cook_method: nil, 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) diff --git a/app/models/user.rb b/app/models/user.rb index 8a5d182965a2..62f3c5c60adc 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/serializers/embeddable_host_serializer.rb b/app/serializers/embeddable_host_serializer.rb index d6a36de5c271..e9da2dd0f7a8 100644 --- a/app/serializers/embeddable_host_serializer.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ec558b23254b..9a501e454236 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6808,6 +6808,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" diff --git a/db/migrate/20240430051551_add_user_id_to_embeddable_hosts.rb b/db/migrate/20240430051551_add_user_id_to_embeddable_hosts.rb new file mode 100644 index 000000000000..7fb87bd10d9c --- /dev/null +++ b/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 diff --git a/db/migrate/20240430052017_create_embeddable_host_tags.rb b/db/migrate/20240430052017_create_embeddable_host_tags.rb new file mode 100644 index 000000000000..46ab6f974b3e --- /dev/null +++ b/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 diff --git a/spec/fabricators/embeddable_host_tag_fabricator.rb b/spec/fabricators/embeddable_host_tag_fabricator.rb new file mode 100644 index 000000000000..94c08689b816 --- /dev/null +++ b/spec/fabricators/embeddable_host_tag_fabricator.rb @@ -0,0 +1,6 @@ +#frozen_string_literal: true + +Fabricator(:embeddable_host_tag) do + embeddable_host + tag +end diff --git a/spec/models/embeddable_host_tag_spec.rb b/spec/models/embeddable_host_tag_spec.rb new file mode 100644 index 000000000000..245a7aa9906f --- /dev/null +++ b/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 diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb index d3e7608c7029..10d670253d1c 100644 --- a/spec/models/topic_embed_spec.rb +++ b/spec/models/topic_embed_spec.rb @@ -299,6 +299,53 @@ 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 diff --git a/spec/requests/admin/embeddable_hosts_controller_spec.rb b/spec/requests/admin/embeddable_hosts_controller_spec.rb index 9380ed4cb3de..9df7ef634cab 100644 --- a/spec/requests/admin/embeddable_hosts_controller_spec.rb +++ b/spec/requests/admin/embeddable_hosts_controller_spec.rb @@ -21,6 +21,68 @@ ).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 diff --git a/spec/system/admin_embeddable_hosts_spec.rb b/spec/system/admin_embeddable_hosts_spec.rb new file mode 100644 index 000000000000..0f033786e1cc --- /dev/null +++ b/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
{{i18n "admin.embedding.host"}}{{i18n "admin.embedding.allowed_paths"}}{{i18n "admin.embedding.category"}}{{i18n "admin.embedding.host"}}{{i18n "admin.embedding.allowed_paths"}}{{i18n "admin.embedding.category"}}{{i18n "admin.embedding.tags"}}{{i18n + "admin.embedding.post_author" + author=this.embedding.embed_by_username + }}{{i18n "admin.embedding.post_author"}}