Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions app/assets/javascripts/admin/addon/components/embeddable-host.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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" />
Expand Down
12 changes: 11 additions & 1 deletion app/assets/javascripts/admin/addon/components/embeddable-host.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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")
Expand All @@ -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) {
Expand All @@ -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;

Expand Down
15 changes: 12 additions & 3 deletions app/assets/javascripts/admin/addon/templates/embedding.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 4 additions & 0 deletions app/assets/stylesheets/common/admin/customize.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
63 changes: 49 additions & 14 deletions app/controllers/admin/embeddable_hosts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions app/models/embeddable_host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -81,4 +84,5 @@ def host_must_be_valid
# updated_at :datetime not null
# class_name :string
# allowed_paths :string
# user_id :integer
#
27 changes: 27 additions & 0 deletions app/models/embeddable_host_tag.rb
Original file line number Diff line number Diff line change
@@ -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)
#
3 changes: 3 additions & 0 deletions app/models/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions app/models/topic_embed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 9 additions & 1 deletion app/serializers/embeddable_host_serializer.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions db/migrate/20240430051551_add_user_id_to_embeddable_hosts.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions db/migrate/20240430052017_create_embeddable_host_tags.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions spec/fabricators/embeddable_host_tag_fabricator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#frozen_string_literal: true

Fabricator(:embeddable_host_tag) do
embeddable_host
tag
end