IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2026-08-03 01:49:44 -05:00
Code Issues Packages Projects Releases Wiki Activity

DEV: Use tag_name/tag.name instead of tag_id/tag.id where the name is actually used (#36226)

Browse Source 
In plenty of areas where tags are dealt with, we use `tag_id` when
actually referring to `tag_name`. This is super confusing.

We're making some changes to how tags are dealt with, so to prevent
adding more tech debt we're fixing this confusion first.
This commit is contained in:
Natalie Tay
2025-12-29 13:20:06 +08:00
committed by GitHub
parent 4f1a9093a4
commit 1b60b632f2
45 changed files with 412 additions and 241 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/controllers/tags_controller.rb
+55 -37
View File
@@ -144,7 +144,7 @@ class TagsController < ::ApplicationController
define_method("show_#{filter}") do
parent_tag_name =
Tag
.where_name(params[:tag_id])
.where_name(params[:tag_name])
.where.not(target_tag_id: nil)
.joins(
"JOIN tags parent_tags ON parent_tags.id = tags.target_tag_id AND tags.target_tag_id != tags.id",
@@ -152,24 +152,24 @@ class TagsController < ::ApplicationController
.pick("parent_tags.name")
if parent_tag_name
params[:tag_id] = parent_tag_name
params[:tag_name] = parent_tag_name
return redirect_to url_for(params.to_unsafe_hash)
end
@tag_id = params[:tag_id].force_encoding("UTF-8")
@tag_name = params[:tag_name].force_encoding("UTF-8")
@additional_tags =
params[:additional_tag_ids].to_s.split("/").map { |t| t.force_encoding("UTF-8") }
params[:additional_tag_names].to_s.split("/").map { |t| t.force_encoding("UTF-8") }
if @additional_tags.present?
additional_tags_trimmed = @additional_tags.dup
additional_tags_trimmed.delete(@tag_id)
additional_tags_trimmed.delete(@tag_name)
additional_tags_trimmed = additional_tags_trimmed&.uniq
if additional_tags_trimmed != @additional_tags
if additional_tags_trimmed.present?
params[:additional_tag_ids] = additional_tags_trimmed&.join("/")
params[:additional_tag_names] = additional_tags_trimmed&.join("/")
else
params[:additional_tag_ids] = nil
params[:additional_tag_names] = nil
end
return redirect_to url_for(params.to_unsafe_hash)
@@ -193,13 +193,13 @@ class TagsController < ::ApplicationController
@list.prev_topics_url = construct_url_with(:prev, list_opts)
@rss = "tag"
@title = I18n.t("rss_by_tag", tag: tag_params.join(" & "))
@description_meta = Tag.where(name: @tag_id).pick(:description) || @title
@description_meta = Tag.where(name: @tag_name).pick(:description) || @title
canonical_params = params.slice(:category_slug_path_with_id, :tag_id)
canonical_params = params.slice(:category_slug_path_with_id, :tag_name)
canonical_method = url_method(canonical_params)
canonical_url "#{Discourse.base_url_no_prefix}#{public_send(canonical_method, *(canonical_params.values.map { |t| t.force_encoding("UTF-8") }))}"
if @list.topics.size == 0 && params[:tag_id] != "none" && !Tag.where_name(@tag_id).exists?
if @list.topics.size == 0 && params[:tag_name] != "none" && !Tag.where_name(@tag_name).exists?
raise Discourse::NotFound.new("tag not found", check_permalinks: true)
else
respond_with_list(@list)
@@ -216,23 +216,35 @@ class TagsController < ::ApplicationController
end
def update
tag = Tag.find_by_name(params[:tag_id])
if params[:tag][:id]
warning =
"Updating a tag name by `id` attribute is unsupported. Use the `name` attribute instead."
Discourse.deprecate(warning, since: "2025.12.0-latest", drop_from: "2026.2.0-latest")
return render_json_error(warning)
end
tag_name_param = params[:tag_name]
new_tag = params[:tag]
new_tag_name = new_tag[:name]
new_tag_description = new_tag[:description]
tag = Tag.find_by_name(tag_name_param)
raise Discourse::NotFound if tag.nil?
guardian.ensure_can_edit_tag!(tag)
if (params[:tag][:id].present?)
new_tag_name = DiscourseTagging.clean_tag(params[:tag][:id])
tag.name = new_tag_name
end
tag.description = params[:tag][:description] if params[:tag]&.has_key?(:description)
tag.name = DiscourseTagging.clean_tag(new_tag_name) if new_tag_name.present?
tag.description = new_tag_description if new_tag_description.present?
if tag.save
StaffActionLogger.new(current_user).log_custom(
"renamed_tag",
previous_value: params[:tag_id],
new_value: new_tag_name,
)
render json: { tag: { id: tag.name, description: tag.description } }
if tag.name != tag_name_param
StaffActionLogger.new(current_user).log_custom(
"renamed_tag",
previous_value: tag_name_param,
new_value: tag.name,
)
end
render json: { tag: { id: tag.id, name: tag.name, description: tag.description } }
else
render_json_error tag.errors.full_messages
end
@@ -298,7 +310,7 @@ class TagsController < ::ApplicationController
def destroy
guardian.ensure_can_admin_tags!
tag_name = params[:tag_id]
tag_name = params[:tag_name]
tag = Tag.find_by_name(tag_name)
raise Discourse::NotFound if tag.nil?
@@ -312,13 +324,13 @@ class TagsController < ::ApplicationController
def tag_feed
discourse_expires_in 1.minute
tag_id = params[:tag_id]
@link = "#{Discourse.base_url}/tag/#{tag_id}"
@description = I18n.t("rss_by_tag", tag: tag_id)
tag_name = params[:tag_name]
@link = "#{Discourse.base_url}/tag/#{tag_name}"
@description = I18n.t("rss_by_tag", tag: tag_name)
@title = "#{SiteSetting.title} - #{@description}"
@atom_link = "#{Discourse.base_url}/tag/#{tag_id}.rss"
@atom_link = "#{Discourse.base_url}/tag/#{tag_name}.rss"
query = TopicQuery.new(current_user, tags: [tag_id])
query = TopicQuery.new(current_user, tags: [tag_name])
latest_results = query.latest_results
@topic_list = query.create_list(:by_tag, {}, latest_results)
@@ -354,7 +366,7 @@ class TagsController < ::ApplicationController
json_response = { results: tags }
if clean_name && !tags.find { |h| h[:id].downcase == clean_name.downcase } &&
if clean_name && !tags.find { |h| h[:name].downcase == clean_name.downcase } &&
tag = Tag.where_name(clean_name).first
# filter_allowed_tags determined that the tag entered is not allowed
json_response[:forbidden] = params[:q]
@@ -402,16 +414,22 @@ class TagsController < ::ApplicationController
end
def notifications
tag = Tag.where_name(params[:tag_id]).first
tag = Tag.where_name(params[:tag_name]).first
raise Discourse::NotFound unless tag
level =
tag.tag_users.where(user: current_user).first.try(:notification_level) ||
TagUser.notification_levels[:regular]
render json: { tag_notification: { id: tag.name, notification_level: level.to_i } }
render json: {
tag_notification: {
id: tag.id,
name: tag.name,
notification_level: level.to_i,
},
}
end
def update_notifications
tag = Tag.find_by_name(params[:tag_id])
tag = Tag.find_by_name(params[:tag_name])
raise Discourse::NotFound unless tag
level = params[:tag_notification][:notification_level].to_i
TagUser.change(current_user.id, tag.id, level)
@@ -462,7 +480,7 @@ class TagsController < ::ApplicationController
private
def fetch_tag
@tag = Tag.find_by_name(params[:tag_id].force_encoding("UTF-8"))
@tag = Tag.find_by_name(params[:tag_name].force_encoding("UTF-8"))
raise Discourse::NotFound unless @tag
end
@@ -471,7 +489,7 @@ class TagsController < ::ApplicationController
end
def ensure_visible
if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id])
if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_name])
raise Discourse::NotFound
end
end
@@ -487,7 +505,7 @@ class TagsController < ::ApplicationController
next if topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags
attrs = {
id: t.name,
id: t.id,
text: t.name,
name: t.name,
description: t.description,
@@ -520,7 +538,7 @@ class TagsController < ::ApplicationController
permalink = Permalink.find_by_url("c/#{params[:category_slug_path_with_id]}")
if permalink.present? && permalink.category_id
return(
redirect_to "#{Discourse.base_path}/tags#{permalink.target_url}/#{params[:tag_id]}",
redirect_to "#{Discourse.base_path}/tags#{permalink.target_url}/#{params[:tag_name]}",
status: :moved_permanently
)
end
@@ -616,7 +634,7 @@ class TagsController < ::ApplicationController
params[:no_subcategories] == "true"
options[:per_page] = params[:per_page].to_i.clamp(1, 30) if params[:per_page].present?
if params[:tag_id] == "none"
if params[:tag_name] == "none"
options.delete(:tags)
options[:no_tags] = true
else
app/controllers/topics_controller.rb
+1 -1
View File
@@ -1119,7 +1119,7 @@ class TopicsController < ApplicationController
else
TopicQuery.new(current_user).new_results(limit: false)
end
if tag_name = params[:tag_id]
if tag_name = params[:tag_name]
tag_name = DiscourseTagging.visible_tags(guardian).where(name: tag_name).pluck(:name).first
end
app/models/tag.rb
+2 -2
View File
@@ -177,7 +177,7 @@ class Tag < ActiveRecord::Base
user_id = allowed_user.id
DB.query_hash(<<~SQL).map!(&:symbolize_keys!)
SELECT tags.name as id, tags.name as text, COUNT(topics.id) AS count
SELECT tags.id as id, tags.name as name, COUNT(topics.id) AS count
FROM tags
JOIN topic_tags ON tags.id = topic_tags.tag_id
JOIN topics ON topics.id = topic_tags.topic_id
@@ -193,7 +193,7 @@ class Tag < ActiveRecord::Base
JOIN group_users gu ON gu.user_id = #{user_id.to_i}
AND gu.group_id = tg.group_id
)
GROUP BY tags.name
GROUP BY tags.id, tags.name
ORDER BY count DESC
LIMIT #{limit.to_i}
SQL
app/views/list/list.erb
+4 -4
View File
@@ -2,10 +2,10 @@
<%= server_plugin_outlet "topic_list_header" %>
<%- if SiteSetting.tagging_enabled && @tag_id %>
<%- if SiteSetting.tagging_enabled && @tag_name %>
<h1>
<%= link_to "#{Discourse.base_url}/tag/#{@tag_id}", itemprop: 'item' do %>
<span itemprop='name'><%= @tag_id %></span>
<%= link_to "#{Discourse.base_url}/tag/#{@tag_name}", itemprop: 'item' do %>
<span itemprop='name'><%= @tag_name %></span>
<% end %>
</h1>
<% end %>
@@ -142,7 +142,7 @@
<%= auto_discovery_link_tag(:rss, { action: :category_feed }, rel: 'alternate nofollow', title: t('rss_topics_in_category', category: @category.name)) %>
<%= raw crawlable_meta_data(title: @category.name, description: @category.description, image: @category.uploaded_logo&.url.presence) %>
<% end %>
<% elsif @tag_id %>
<% elsif @tag_name %>
<% content_for :head do %>
<%= raw crawlable_meta_data(title: @title, description: @description_meta) %>
<% end %>
config/routes.rb
+13 -12
View File
@@ -726,7 +726,7 @@ Discourse::Application.routes.draw do
username: RouteFormat.username,
group_name: RouteFormat.username,
}
get "#{root_path}/:username/messages/tags/:tag_id" => "list#private_messages_tag",
get "#{root_path}/:username/messages/tags/:tag_name" => "list#private_messages_tag",
:constraints => {
username: RouteFormat.username,
}
@@ -1453,7 +1453,7 @@ Discourse::Application.routes.draw do
:defaults => {
format: :json,
}
get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag",
get "private-messages-tags/:username/:tag_name.json" => "list#private_messages_tag",
:as => "topics_private_messages_tag",
:defaults => {
format: :json,
@@ -1657,7 +1657,7 @@ Discourse::Application.routes.draw do
get ".well-known/apple-app-site-association" => "metadata#app_association_ios", :format => false
get "opensearch" => "metadata#opensearch", :constraints => { format: :xml }
scope "/tag/:tag_id" do
scope "/tag/:tag_name" do
constraints format: :json do
get "/" => "tags#show", :as => "tag_show"
get "/info" => "tags#info"
@@ -1692,44 +1692,45 @@ Discourse::Application.routes.draw do
get "/unused" => "tags#list_unused"
delete "/unused" => "tags#destroy_unused"
constraints(tag_id: %r{[^/]+?}, format: /json|rss/) do
constraints(tag_name: %r{[^/]+?}, format: /json|rss/) do
scope path: "/c/*category_slug_path_with_id" do
Discourse.filters.each do |filter|
get "/none/:tag_id/l/#{filter}" => "tags#show_#{filter}",
get "/none/:tag_name/l/#{filter}" => "tags#show_#{filter}",
:as => "tag_category_none_show_#{filter}",
:defaults => {
no_subcategories: true,
}
get "/all/:tag_id/l/#{filter}" => "tags#show_#{filter}",
get "/all/:tag_name/l/#{filter}" => "tags#show_#{filter}",
:as => "tag_category_all_show_#{filter}",
:defaults => {
no_subcategories: false,
}
end
get "/none/:tag_id" => "tags#show",
get "/none/:tag_name" => "tags#show",
:as => "tag_category_none_show",
:defaults => {
no_subcategories: true,
}
get "/all/:tag_id" => "tags#show",
get "/all/:tag_name" => "tags#show",
:as => "tag_category_all_show",
:defaults => {
no_subcategories: false,
}
Discourse.filters.each do |filter|
get "/:tag_id/l/#{filter}" => "tags#show_#{filter}",
get "/:tag_name/l/#{filter}" => "tags#show_#{filter}",
:as => "tag_category_show_#{filter}"
end
get "/:tag_id" => "tags#show", :as => "tag_category_show"
get "/:tag_name" => "tags#show", :as => "tag_category_show"
end
get "/intersection/:tag_id/*additional_tag_ids" => "tags#show", :as => "tag_intersection"
get "/intersection/:tag_name/*additional_tag_names" => "tags#show",
:as => "tag_intersection"
end
get "*tag_id", to: redirect(relative_url_root + "tag/%{tag_id}")
get "*tag_name", to: redirect(relative_url_root + "tag/%{tag_name}")
end
resources :tag_groups, constraints: StaffConstraint.new, except: [:edit]
frontend/discourse/app/adapters/tag.js
+4 -2
View File
@@ -1,7 +1,9 @@
import RESTAdapter from "discourse/adapters/rest";
export default class TagAdapter extends RESTAdapter {
pathFor(store, type, id) {
return id ? `/tag/${id}` : `/tags`;
primaryKey = "name";
pathFor(store, type, tagName) {
return tagName ? `/tag/${tagName}` : `/tags`;
}
}
frontend/discourse/app/components/bread-crumbs.gjs
+26 -5
View File
@@ -13,6 +13,7 @@ import deprecated from "discourse/lib/deprecated";
import CategoryDrop from "discourse/select-kit/components/category-drop";
import TagDrop from "discourse/select-kit/components/tag-drop";
import TagsIntersectionChooser from "discourse/select-kit/components/tags-intersection-chooser";
import deprecatedOutletArgument from "../helpers/deprecated-outlet-argument";
@tagName("ol")
@classNameBindings("hidden:hidden", ":category-breadcrumb")
@@ -140,7 +141,7 @@ export default class BreadCrumbs extends Component {
@name="bread-crumbs-left"
@connectorTagName="li"
@outletArgs={{lazyHash
tagId=this.tag.id
tag=this.tag
additionalTags=this.additionalTags
noSubcategories=this.noSubcategories
showTagsSection=this.showTagsSection
@@ -149,6 +150,16 @@ export default class BreadCrumbs extends Component {
editingCategory=this.editingCategory
editingCategoryTab=this.editingCategoryTab
}}
@deprecatedArgs={{lazyHash
tagId=(deprecatedOutletArgument
value=this.tag.name
message="The argument 'tagId' is deprecated on the outlet 'bread-crumbs-left', use 'tag.name' instead"
id="discourse.plugin-connector.deprecated-arg.bread-crumbs-left"
since="2025.12.0-latest"
dropFrom="2026.2.0-latest"
silence="discourse.header-service-topic"
)
}}
/>
{{#each this.categoryBreadcrumbs as |breadcrumb|}}
@@ -162,7 +173,7 @@ export default class BreadCrumbs extends Component {
<CategoryDrop
@category={{breadcrumb.category}}
@categories={{breadcrumb.options}}
@tagId={{this.tag.id}}
@tag={{this.tag}}
@editingCategory={{this.editingCategory}}
@editingCategoryTab={{this.editingCategoryTab}}
@options={{hash
@@ -185,7 +196,7 @@ export default class BreadCrumbs extends Component {
<li>
<TagsIntersectionChooser
@currentCategory={{this.category}}
@mainTag={{this.tag.id}}
@mainTag={{this.tag.name}}
@additionalTags={{this.additionalTags}}
@options={{hash categoryId=this.category.id}}
/>
@@ -195,7 +206,7 @@ export default class BreadCrumbs extends Component {
<TagDrop
@currentCategory={{this.category}}
@noSubcategories={{this.noSubcategories}}
@tagId={{this.tag.id}}
@tag={{this.tag}}
/>
</li>
{{/if}}
@@ -205,7 +216,7 @@ export default class BreadCrumbs extends Component {
@name="bread-crumbs-right"
@connectorTagName="li"
@outletArgs={{lazyHash
tagId=this.tag.id
tag=this.tag
additionalTags=this.additionalTags
noSubcategories=this.noSubcategories
showTagsSection=this.showTagsSection
@@ -214,6 +225,16 @@ export default class BreadCrumbs extends Component {
editingCategory=this.editingCategory
editingCategoryTab=this.editingCategoryTab
}}
@deprecatedArgs={{lazyHash
tagId=(deprecatedOutletArgument
value=this.tag.name
message="The argument 'tagId' is deprecated on the outlet 'bread-crumbs-right', use 'tag.name' instead"
id="discourse.plugin-connector.deprecated-arg.bread-crumbs-right"
since="2025.12.0-latest"
dropFrom="2026.2.0-latest"
silence="discourse.header-service-topic"
)
}}
/>
</template>
}
frontend/discourse/app/components/d-navigation.gjs
+2 -2
View File
@@ -121,7 +121,7 @@ export default class DNavigation extends Component {
return canEdit;
}
@discourseComputed("additionalTags", "category", "tag.id")
@discourseComputed("additionalTags", "category", "tag.name")
showToggleInfo(additionalTags, category, tagId) {
return !additionalTags && !category && tagId !== "none";
}
@@ -130,7 +130,7 @@ export default class DNavigation extends Component {
"filterType",
"category",
"noSubcategories",
"tag.id",
"tag.name",
"router.currentRoute.queryParams",
"skipCategoriesNavItem"
)
frontend/discourse/app/components/discovery/accessible-discovery-heading.gjs
+5 -3
View File
@@ -50,11 +50,13 @@ export default class AccessibleDiscoveryHeading extends Component {
// tag intersections don't have additional filters
if (type === "multi_tag") {
return i18n("discovery.headings.multi_tag.default", {
tags: [tag?.id, ...(additionalTags || [])].filter(Boolean).join(" + "),
tags: [tag?.name, ...(additionalTags || [])]
.filter(Boolean)
.join(" + "),
});
}
if (tag?.id === "none" && !additionalTags?.length) {
if (tag?.name === "none" && !additionalTags?.length) {
const noTagsType = category ? "category" : "all";
const prefix = `discovery.headings.no_tags.${noTagsType}`;
const specificKey = key ? `${prefix}.${key}` : null;
@@ -79,7 +81,7 @@ export default class AccessibleDiscoveryHeading extends Component {
const params = {
category: category?.name,
tag: tag?.id,
tag: tag?.name,
filter: key,
};
frontend/discourse/app/components/discovery/navigation.gjs
+1 -1
View File
@@ -81,7 +81,7 @@ export default class DiscoveryNavigation extends Component {
<template>
<AddCategoryTagClasses
@category={{@category}}
@tags={{if @tag (array @tag.id)}}
@tags={{if @tag (array @tag.name)}}
/>
<AccessibleDiscoveryHeading
frontend/discourse/app/components/discovery/topics.gjs
+3 -3
View File
@@ -107,7 +107,7 @@ export default class DiscoveryTopics extends Component {
return this.topicTrackingState.countUnread({
categoryId: this.args.category?.id,
noSubcategories: this.args.noSubcategories,
tagId: this.args.tag?.id,
tagId: this.args.tag?.name,
});
} else {
return 0;
@@ -121,7 +121,7 @@ export default class DiscoveryTopics extends Component {
return this.topicTrackingState.countNew({
categoryId: this.args.category?.id,
noSubcategories: this.args.noSubcategories,
tagId: this.args.tag?.id,
tagId: this.args.tag?.name,
});
} else {
return 0;
@@ -159,7 +159,7 @@ export default class DiscoveryTopics extends Component {
}
return i18n("topics.bottom.tag", {
tag: tag.id,
tag: tag.name,
});
} else {
if (topicsLength === 0) {
frontend/discourse/app/components/search-menu/results/type/tag.gjs
+1 -1
View File
@@ -4,7 +4,7 @@ import { or } from "discourse/truth-helpers";
const Tag = <template>
{{icon "tag"}}
{{discourseTag (or @result.id @result) tagName="span"}}
{{discourseTag (or @result.name @result) tagName="span"}}
</template>;
export default Tag;
frontend/discourse/app/components/tag-info.gjs
+15 -11
View File
@@ -90,7 +90,7 @@ export default class TagInfo extends Component {
}
this.set("loading", true);
return this.store
.find("tag-info", this.tag.id)
.find("tag-info", this.tag.name)
.then((result) => {
this.set("tagInfo", result);
this.set(
@@ -113,7 +113,7 @@ export default class TagInfo extends Component {
);
this.setProperties({
editing: true,
newTagName: this.tag.id,
newTagName: this.tag.name,
newTagDescription: this.tagInfo.description,
});
}
@@ -121,7 +121,7 @@ export default class TagInfo extends Component {
@action
unlinkSynonym(tag, event) {
event?.preventDefault();
ajax(`/tag/${this.tagInfo.name}/synonyms/${tag.id}`, {
ajax(`/tag/${this.tagInfo.name}/synonyms/${tag.name}`, {
type: "DELETE",
})
.then(() => removeValueFromArray(this.tagInfo.synonyms, tag))
@@ -157,18 +157,22 @@ export default class TagInfo extends Component {
@action
finishedEditing() {
const oldTagName = this.tag.id;
const oldTagName = this.tag.name;
this.newTagDescription = this.newTagDescription?.replaceAll("\n", "<br>");
this.tag
.update({ id: this.newTagName, description: this.newTagDescription })
.update({
name: this.newTagName,
description: this.newTagDescription,
})
.then((result) => {
this.set("editing", false);
this.tagInfo.set("description", this.newTagDescription);
if (
result.responseJson.tag &&
oldTagName !== result.responseJson.tag.id
) {
this.router.transitionTo("tag.show", result.responseJson.tag.id);
if (result.responseJson.tag) {
const newTagName = result.responseJson.tag.name;
if (oldTagName !== newTagName) {
this.router.transitionTo("tag.show", newTagName);
}
}
})
.catch(popupAjaxError);
@@ -335,7 +339,7 @@ export default class TagInfo extends Component {
<div class="tag-list">
{{#each this.tagInfo.synonyms as |tag|}}
<div class="tag-box">
{{discourseTag tag.id pmOnly=tag.pmOnly tagName="div"}}
{{discourseTag tag.name pmOnly=tag.pmOnly tagName="div"}}
{{#if this.editSynonymsMode}}
<a
href
frontend/discourse/app/components/tag-list.gjs
+1 -1
View File
@@ -59,7 +59,7 @@ export default class TagList extends Component {
{{#each this.sortedTags as |tag|}}
<div class="tag-box">
{{discourseTag
tag.id
tag.name
description=tag.description
isPrivateMessage=this.isPrivateMessage
pmOnly=tag.pmOnly
frontend/discourse/app/controllers/discovery/list.js
+1 -1
View File
@@ -180,7 +180,7 @@ export default class DiscoveryListController extends Controller {
createTopic() {
this.composer.openNewTopic({
category: this.createTopicTargetCategory,
tags: [this.model.tag?.id, ...(this.model.additionalTags ?? [])]
tags: [this.model.tag?.name, ...(this.model.additionalTags ?? [])]
.filter(Boolean)
.filter((t) => !["none", "all"].includes(t))
.join(","),
frontend/discourse/app/controllers/user-private-messages/tags/index.js
+3 -3
View File
@@ -6,13 +6,13 @@ export default class extends Controller {
@tracked tagsForUser = null;
@tracked sortedByCount = true;
@tracked sortedByName = false;
@tracked sortProperties = ["count:desc", "id"];
@tracked sortProperties = ["count:desc", "name"];
@action
sortByCount(event) {
event?.preventDefault();
this.sortProperties = ["count:desc", "id"];
this.sortProperties = ["count:desc", "name"];
this.sortedByCount = true;
this.sortedByName = false;
}
@@ -21,7 +21,7 @@ export default class extends Controller {
sortById(event) {
event?.preventDefault();
this.sortProperties = ["id"];
this.sortProperties = ["name"];
this.sortedByCount = false;
this.sortedByName = true;
}
frontend/discourse/app/lib/search.js
+5 -3
View File
@@ -103,10 +103,12 @@ export function translateResults(results, opts) {
results.tags = results.tags
.map(function (tag) {
const tagName = escapeExpression(tag.name);
const id = tag.id;
const name = escapeExpression(tag.name);
return EmberObject.create({
id: tagName,
url: getURL("/tag/" + tagName),
id,
name,
url: getURL("/tag/" + name),
});
})
.filter((item) => item != null);
frontend/discourse/app/lib/url.js
+4 -4
View File
@@ -515,7 +515,7 @@ export function prefixProtocol(url) {
return `https://${url}`;
}
export function getCategoryAndTagUrl(category, subcategories, tag) {
export function getCategoryAndTagUrl(category, subcategories, tagName) {
let url;
if (category) {
@@ -531,10 +531,10 @@ export function getCategoryAndTagUrl(category, subcategories, tag) {
}
}
if (tag) {
if (tagName) {
url = url
? "/tags" + url + "/" + tag.toLowerCase()
: "/tag/" + tag.toLowerCase();
? "/tags" + url + "/" + tagName.toLowerCase()
: "/tag/" + tagName.toLowerCase();
}
return getURL(url || "/");
frontend/discourse/app/models/rest.js
+2 -1
View File
@@ -33,6 +33,7 @@ export default class RestModel extends EmberObject {
@equal("__state", "new") isNew;
@equal("__state", "created") isCreated;
primaryKey = "id";
@tracked __state;
beforeCreate() {}
@@ -52,7 +53,7 @@ export default class RestModel extends EmberObject {
this.set("isSaving", true);
return this.store
.update(this.__type, this.id, props)
.update(this.__type, this.get(this.primaryKey), props)
.then((res) => {
const payload = this.__munge(res.payload || res.responseJson);
frontend/discourse/app/models/tag-notification.js
+5
View File
@@ -0,0 +1,5 @@
import RestModel from "discourse/models/rest";
export default class TagNotification extends RestModel {
primaryKey = "name";
}
frontend/discourse/app/models/tag.js
+7 -3
View File
@@ -3,6 +3,10 @@ import discourseComputed from "discourse/lib/decorators";
import RestModel from "discourse/models/rest";
export default class Tag extends RestModel {
// Use tag name instead of numeric id as the primary key
// since backend tag routes use tag name in the URL path
primaryKey = "name";
@readOnly("pm_only") pmOnly;
@discourseComputed("count", "pm_count")
@@ -10,14 +14,14 @@ export default class Tag extends RestModel {
return pmCount ? count + pmCount : count;
}
@discourseComputed("id")
searchContext(id) {
@discourseComputed("id", "name")
searchContext(id, name) {
return {
type: "tag",
id,
/** @type Tag */
tag: this,
name: id,
name,
};
}
}
frontend/discourse/app/models/topic.js
+1 -1
View File
@@ -246,7 +246,7 @@ export default class Topic extends RestModel {
data.include_subcategories = include_subcategories;
}
if (tag) {
data.tag_id = tag.id;
data.tag_name = tag.name;
}
if (topicIds) {
data.topic_ids = topicIds;
frontend/discourse/app/routes/app-route-map.js
+10 -10
View File
@@ -219,43 +219,43 @@ export default function () {
this.route("full-page-search", { path: "/search" });
this.route("tag", function () {
this.route("show", { path: "/:tag_id" });
this.route("show", { path: "/:tag_name" });
Site.currentProp("filters").forEach((filter) => {
this.route("show" + capitalize(filter), {
path: "/:tag_id/l/" + filter,
path: "/:tag_name/l/" + filter,
});
});
});
this.route("tags", function () {
this.route("showCategory", {
path: "/c/*category_slug_path_with_id/:tag_id",
path: "/c/*category_slug_path_with_id/:tag_name",
});
this.route("showCategoryAll", {
path: "/c/*category_slug_path_with_id/all/:tag_id",
path: "/c/*category_slug_path_with_id/all/:tag_name",
});
this.route("showCategoryNone", {
path: "/c/*category_slug_path_with_id/none/:tag_id",
path: "/c/*category_slug_path_with_id/none/:tag_name",
});
Site.currentProp("filters").forEach((filter) => {
this.route("showCategory" + capitalize(filter), {
path: "/c/*category_slug_path_with_id/:tag_id/l/" + filter,
path: "/c/*category_slug_path_with_id/:tag_name/l/" + filter,
});
this.route("showCategoryAll" + capitalize(filter), {
path: "/c/*category_slug_path_with_id/all/:tag_id/l/" + filter,
path: "/c/*category_slug_path_with_id/all/:tag_name/l/" + filter,
});
this.route("showCategoryNone" + capitalize(filter), {
path: "/c/*category_slug_path_with_id/none/:tag_id/l/" + filter,
path: "/c/*category_slug_path_with_id/none/:tag_name/l/" + filter,
});
});
this.route("intersection", {
path: "intersection/:tag_id/*additional_tags",
path: "intersection/:tag_name/*additional_tags",
});
// legacy route
this.route("legacyRedirect", { path: "/:tag_id" });
this.route("legacyRedirect", { path: "/:tag_name" });
});
this.route("tagGroups", { path: "/tag_groups" }, function () {
frontend/discourse/app/routes/tag/show.js
+19 -23
View File
@@ -40,35 +40,30 @@ export default class TagShowRoute extends DiscourseRoute {
}
async model(params, transition) {
const tagIdFromParams = escapeExpression(params.tag_id);
const name = escapeExpression(params.tag_name);
const id = params.tag_id;
const tag = this.store.createRecord("tag", {
id: tagIdFromParams,
id,
name,
});
// Handles renaming a tag, since we refer to the tag.id instead
// of tag.name which is the actual identifier.
if (tag.id !== tagIdFromParams) {
tag.set("id", tagIdFromParams);
}
let additionalTags;
if (params.additional_tags) {
additionalTags = params.additional_tags.split("/").map((t) => {
return this.store.createRecord("tag", {
id: escapeExpression(t),
}).id;
name: escapeExpression(t),
}).name;
});
}
const filterType = filterTypeForMode(this.navMode);
let tagNotification;
if (tag && tag.id !== NONE && this.currentUser && !additionalTags) {
// If logged in, we should get the tag's user settings
if (tag && name !== NONE && this.currentUser && !additionalTags) {
tagNotification = await this.store.find(
"tagNotification",
tag.id.toLowerCase()
name.toLowerCase()
);
}
@@ -80,7 +75,7 @@ export default class TagShowRoute extends DiscourseRoute {
{}
);
const topicFilter = this.navMode;
const tagId = tag ? tag.id.toLowerCase() : NONE;
const tagName = name ? name.toLowerCase() : NONE;
let filter;
if (category) {
@@ -91,9 +86,9 @@ export default class TagShowRoute extends DiscourseRoute {
filter += this.noSubcategories ? `/${NONE}` : `/${ALL}`;
}
filter += `/${tagId}/l/${topicFilter}`;
filter += `/${tagName}/l/${topicFilter}`;
} else if (additionalTags) {
filter = `tags/intersection/${tagId}/${additionalTags.join("/")}`;
filter = `tags/intersection/${tagName}/${additionalTags.join("/")}`;
if (transition.to.queryParams["category"]) {
filteredQueryParams["category"] = transition.to.queryParams["category"];
@@ -102,7 +97,7 @@ export default class TagShowRoute extends DiscourseRoute {
);
}
} else {
filter = `tag/${tagId}/l/${topicFilter}`;
filter = `tag/${tagName}/l/${topicFilter}`;
}
if (
@@ -115,7 +110,7 @@ export default class TagShowRoute extends DiscourseRoute {
return this.router.replaceWith(
"tags.showCategoryNone",
params.category_slug_path_with_id,
tagId
tagName
);
}
@@ -132,7 +127,8 @@ export default class TagShowRoute extends DiscourseRoute {
if (list.topic_list.tags && list.topic_list.tags.length === 1) {
// Update name of tag (case might be different)
tag.setProperties({
id: list.topic_list.tags[0].name,
id: list.topic_list.tags[0].id,
name: list.topic_list.tags[0].name,
staff: list.topic_list.tags[0].staff,
});
}
@@ -159,7 +155,7 @@ export default class TagShowRoute extends DiscourseRoute {
if (model.category || model.additionalTags) {
const tagIntersectionSearchContext = {
type: "tagIntersection",
tagId: model.tag.id,
tagId: model.tag.name,
tag: model.tag,
additionalTags: model.additionalTags || null,
categoryId: model.category?.id || null,
@@ -176,18 +172,18 @@ export default class TagShowRoute extends DiscourseRoute {
const filterText = i18n(`filters.${this.navMode.replace("/", ".")}.title`);
const model = this.currentModel;
const tag = model?.tag?.id;
const tag = model?.tag?.name;
if (tag && tag !== NONE) {
if (model.category) {
return i18n("tagging.filters.with_category", {
filter: filterText,
tag: model.tag.id,
tag: model.tag.name,
category: model.category.displayName,
});
} else {
return i18n("tagging.filters.without_category", {
filter: filterText,
tag: model.tag.id,
tag: model.tag.name,
});
}
} else {
frontend/discourse/app/routes/tags/legacy-redirect.js
+1 -1
View File
@@ -7,7 +7,7 @@ export default class TagsLegacyRedirect extends Route {
beforeModel() {
this.router.transitionTo(
"tag.show",
this.paramsFor("tags.legacyRedirect").tag_id
this.paramsFor("tags.legacyRedirect").tag_name
);
}
}
frontend/discourse/app/routes/user-private-messages/tags/index.js
+2 -2
View File
@@ -23,8 +23,8 @@ export default class UserPrivateMessagesTagsIndex extends DiscourseRoute {
controller.setProperties({
model,
sortProperties: this.siteSettings.tags_sort_alphabetically
? ["id"]
: ["count:desc", "id"],
? ["name"]
: ["count:desc", "name"],
tagsForUser: this.modelFor("user").get("username_lower"),
});
frontend/discourse/app/templates/full-page-search.gjs
+1 -1
View File
@@ -310,7 +310,7 @@ export default <template>
{{#each @controller.model.tags as |tag|}}
<div class="fps-tag-item">
<a href={{tag.url}}>
{{tag.id}}
{{tag.name}}
</a>
</div>
{{/each}}
frontend/discourse/select-kit/components/category-drop.js
+1 -1
View File
@@ -253,7 +253,7 @@ export default class CategoryDrop extends ComboBoxComponent {
route = getCategoryAndTagUrl(
category,
categoryId !== NO_CATEGORIES_ID,
this.tagId
this.tag?.name
);
}
frontend/discourse/select-kit/components/mini-tag-chooser.js
+6 -1
View File
@@ -34,11 +34,16 @@ import TagRow from "./tag-row";
maximum: "maxTagsPerTopic",
autoInsertNoneItem: false,
useHeaderFilter: false,
valueProperty: "name",
nameProperty: "name",
})
@pluginApiIdentifiers(["mini-tag-chooser"])
export default class MiniTagChooser extends MultiSelectComponent {
@service tagUtils;
valueProperty = "name";
nameProperty = "name";
@empty("value") noTags;
@or("allowCreate", "site.can_create_tag") allowAnyTag;
@@ -159,6 +164,6 @@ export default class MiniTagChooser extends MultiSelectComponent {
this.set("selectKit.options.translatedFilterPlaceholder", null);
}
return results.filter((r) => !makeArray(this.tags).includes(r.id));
return results.filter((r) => !makeArray(this.tags).includes(r.name));
}
}
frontend/discourse/select-kit/components/tag-chooser.js
+7 -3
View File
@@ -20,11 +20,15 @@ import TagChooserRow from "./tag-chooser-row";
limit: null,
allowAny: "canCreateTag",
maximum: "maximumTagCount",
valueProperty: "name",
})
@pluginApiIdentifiers("tag-chooser")
export default class TagChooser extends MultiSelectComponent {
@service tagUtils;
valueProperty = "name";
nameProperty = "name";
blockedTags = null;
excludeSynonyms = false;
excludeHasSynonyms = false;
@@ -150,14 +154,14 @@ export default class TagChooser extends MultiSelectComponent {
if (this.blockedTags) {
results = results.filter((result) => {
return !this.blockedTags.includes(result.id);
return !this.blockedTags.includes(result.name);
});
}
if (this.siteSettings.tags_sort_alphabetically) {
results = results.sort((a, b) => a.id > b.id);
results = results.sort((a, b) => a.name > b.name);
}
return uniqueItemsFromArray(results, "id");
return uniqueItemsFromArray(results, "name");
}
}
frontend/discourse/select-kit/components/tag-drop.js
+26 -23
View File
@@ -39,12 +39,12 @@ const MORE_TAGS_COLLECTION = "MORE_TAGS_COLLECTION";
export default class TagDrop extends ComboBoxComponent {
@service tagUtils;
@readOnly("tag.id") value;
@setting("max_tag_search_results") maxTagSearchResults;
@setting("tags_sort_alphabetically") sortTagsAlphabetically;
@setting("max_tags_in_filter_list") maxTagsInFilterList;
@readOnly("tagId") value;
init() {
super.init(...arguments);
@@ -75,7 +75,7 @@ export default class TagDrop extends ComboBoxComponent {
}
modifyNoSelection() {
if (this.tagId === NONE_TAG) {
if (this.value === NONE_TAG) {
return this.defaultItem(NO_TAG_ID, i18n("tagging.selector_no_tags"));
} else {
return this.defaultItem(ALL_TAGS_ID, i18n("tagging.selector_tags"));
@@ -83,36 +83,38 @@ export default class TagDrop extends ComboBoxComponent {
}
modifySelection(content) {
if (this.tagId === NONE_TAG) {
content = this.defaultItem(NO_TAG_ID, i18n("tagging.selector_no_tags"));
} else if (this.tagId) {
content = this.defaultItem(this.tagId, this.tagId);
if (this.value === NONE_TAG) {
return this.defaultItem(NO_TAG_ID, i18n("tagging.selector_no_tags"));
}
if (this.value && this.tag?.name) {
return this.defaultItem(this.value, this.tag.name);
}
return content;
}
@computed("tagId")
@computed("value")
get tagClass() {
return this.tagId ? `tag-${this.tagId}` : "tag_all";
return this.value ? `tag-${this.value}` : "tag_all";
}
modifyComponentForRow() {
return TagRow;
}
@computed("tagId")
@computed("tag.id")
get shortcuts() {
const shortcuts = [];
if (this.tagId) {
if (this.tag?.id) {
shortcuts.push({
id: ALL_TAGS_ID,
name: i18n("tagging.selector_remove_filter"),
});
}
if (this.tagId !== NONE_TAG) {
if (this.tag?.id !== NONE_TAG) {
shortcuts.push({
id: NO_TAG_ID,
name: i18n("tagging.selector_no_tags"),
@@ -192,10 +194,9 @@ export default class TagDrop extends ComboBoxComponent {
}
return json.results
.sort((a, b) => a.id > b.id)
.sort((a, b) => a.name > b.name)
.map((r) => {
const content = this.defaultItem(r.id, r.text);
content.targetTagId = r.target_tag || r.id;
const content = this.defaultItem(r.id, r.name);
if (!this.currentCategory) {
content.count = r.count;
}
@@ -205,17 +206,19 @@ export default class TagDrop extends ComboBoxComponent {
}
@action
onChange(tagId, tag) {
if (tagId === NO_TAG_ID) {
tagId = NONE_TAG;
} else if (tagId === ALL_TAGS_ID) {
tagId = null;
} else if (tag && tag.targetTagId) {
tagId = tag.targetTagId;
onChange(value, tag) {
let tagName;
if (value === NO_TAG_ID) {
tagName = NONE_TAG;
} else if (value === ALL_TAGS_ID) {
tagName = null;
} else if (tag && tag.name) {
tagName = tag.name;
}
DiscourseURL.routeToUrl(
getCategoryAndTagUrl(this.currentCategory, !this.noSubcategories, tagId)
getCategoryAndTagUrl(this.currentCategory, !this.noSubcategories, tagName)
);
}
}
frontend/discourse/select-kit/components/tag-row.gjs
+2 -2
View File
@@ -13,13 +13,13 @@ export default class TagRow extends SelectKitRowComponent {
<template>
{{#if this.isTag}}
{{discourseTag
this.rowValue
this.rowName
noHref=true
description=this.item.description
count=this.item.count
}}
{{else}}
<span class="name">{{this.item.name}}</span>
<span class="name">{{this.rowName}}</span>
{{/if}}
</template>
}
frontend/discourse/tests/acceptance/search-test.js
+2 -2
View File
@@ -52,7 +52,7 @@ acceptance("Search - Anonymous", function (needs) {
server.get("/tag/important/notifications", () => {
return helper.response({
tag_notification: { id: "important", notification_level: 2 },
tag_notification: { id: 1, name: "important", notification_level: 2 },
});
});
});
@@ -809,7 +809,7 @@ acceptance("Search - with tagging enabled", function (needs) {
server.get("/tag/dev/notifications", () => {
return helper.response({
tag_notification: { id: "dev", notification_level: 2 },
tag_notification: { id: 1, name: "dev", notification_level: 2 },
});
});
frontend/discourse/tests/acceptance/sidebar-user-tags-section-test.js
+1 -1
View File
@@ -39,7 +39,7 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) {
needs.pretender((server, helper) => {
server.get("/tag/:tagId/notifications", (request) => {
return helper.response({
tag_notification: { id: request.params.tagId },
tag_notification: { id: 1, name: request.params.tagId },
});
});
frontend/discourse/tests/acceptance/tags-test.js
+18 -10
View File
@@ -15,7 +15,7 @@ acceptance("Tags", function (needs) {
needs.pretender((server, helper) => {
server.get("/tag/test/notifications", () =>
helper.response({
tag_notification: { id: "test", notification_level: 2 },
tag_notification: { id: 42, name: "test", notification_level: 2 },
})
);
@@ -130,7 +130,7 @@ acceptance("Tags listed by group", function (needs) {
needs.pretender((server, helper) => {
server.get("/tag/regular-tag/notifications", () =>
helper.response({
tag_notification: { id: "regular-tag", notification_level: 1 },
tag_notification: { id: 1, name: "regular-tag", notification_level: 1 },
})
);
@@ -158,7 +158,11 @@ acceptance("Tags listed by group", function (needs) {
server.get("/tag/staff-only-tag/notifications", () =>
helper.response({
tag_notification: { id: "staff-only-tag", notification_level: 1 },
tag_notification: {
id: 1,
name: "staff-only-tag",
notification_level: 1,
},
})
);
@@ -256,7 +260,8 @@ acceptance("Tag info", function (needs) {
server.get("/tag/:tag_name/notifications", (request) => {
return helper.response({
tag_notification: {
id: request.params.tag_name,
id: 1,
name: request.params.tag_name,
notification_level: 1,
},
});
@@ -338,11 +343,13 @@ acceptance("Tag info", function (needs) {
staff: false,
synonyms: [
{
id: "containers",
id: "22",
name: "containers",
text: "containers",
},
{
id: "planter",
id: "33",
name: "planter",
text: "planter",
},
],
@@ -370,12 +377,13 @@ acceptance("Tag info", function (needs) {
});
server.put("/tag/happy-monkey", (request) => {
const data = helper.parsePostData(request.requestBody);
return helper.response({ tag: { id: data.tag.id } });
return helper.response({
tag: { id: data.tag.id, name: data.tag.name },
});
});
server.get("/tag/happy-monkey/info", () => {
return helper.response({
__rest_serializer: "1",
tag_info: {
id: 13,
name: "happy-monkey",
@@ -392,7 +400,6 @@ acceptance("Tag info", function (needs) {
server.get("/tag/happy-monkey2/info", () => {
return helper.response({
__rest_serializer: "1",
tag_info: {
id: 13,
name: "happy-monkey2",
@@ -644,7 +651,8 @@ acceptance("Tag show - create topic", function (needs) {
server.get("/tag/:tag_name/notifications", (request) => {
return helper.response({
tag_notification: {
id: request.params.tag_name,
id: 1,
name: request.params.tag_name,
notification_level: 1,
},
});
frontend/discourse/tests/acceptance/user-private-messages-test.js
+1 -1
View File
@@ -75,7 +75,7 @@ function withGroupMessagesSetup(needs) {
needs.pretender((server, helper) => {
server.get("/tags/personal_messages/:username.json", () => {
return helper.response({ tags: [{ id: "tag1" }] });
return helper.response({ tags: [{ id: 1, name: "tag1" }] });
});
server.get("/t/13.json", () => {
frontend/discourse/tests/helpers/create-pretender.js
+3 -3
View File
@@ -110,16 +110,16 @@ export function applyDefaultHandlers(pretender) {
pretender.get("/tags", () => {
return response({
tags: [
{ id: "eviltrout", count: 1 },
{ id: 123, name: "eviltrout", text: "eviltrout", count: 1 },
{
id: "planned",
id: 234,
name: "planned",
text: "planned",
count: 7,
pm_only: false,
},
{
id: "private",
id: 345,
name: "private",
text: "private",
count: 0,
frontend/discourse/tests/integration/components/accessible-discovery-heading-test.gjs
+2 -2
View File
@@ -21,7 +21,7 @@ module("Component | discovery/accessible-discovery-heading", function (hooks) {
test("it renders the correct label for a single tag", async function (assert) {
this.setProperties({
filter: "top",
tag: { id: "javascript" },
tag: { id: 1, name: "javascript" },
});
await render(
@@ -40,7 +40,7 @@ module("Component | discovery/accessible-discovery-heading", function (hooks) {
this.setProperties({
filter: "latest",
category: { name: "Development" },
tag: { id: "javascript" },
tag: { id: 1, name: "javascript" },
});
await render(
frontend/discourse/tests/integration/components/select-kit/tag-drop-test.gjs
+3 -3
View File
@@ -18,7 +18,7 @@ module("Integration | Component | select-kit/tag-drop", function (hooks) {
pretender.get("/tags/filter/search", (params) => {
if (params.queryParams.q === "dav") {
return response({
results: [{ id: "David", name: "David", count: 2, pm_only: false }],
results: [{ id: 123, name: "David", count: 2, pm_only: false }],
});
}
});
@@ -32,8 +32,8 @@ module("Integration | Component | select-kit/tag-drop", function (hooks) {
<template>
<TagDrop
@currentCategory={{category}}
@tagId="jeff"
@options={{hash tagId="jeff"}}
@tag={{hash id=1 name="jeff"}}
@options={{hash}}
/>
</template>
);
lib/topic_query_params.rb
+1 -1
View File
@@ -3,7 +3,7 @@
module TopicQueryParams
def build_topic_list_options
options = {}
params[:tags] = [params[:tag_id], *Array(params[:tags])].uniq if params[:tag_id].present?
params[:tags] = [params[:tag_name], *Array(params[:tags])].uniq if params[:tag_name].present?
TopicQuery.public_valid_options.each do |key|
if params.key?(key) && (val = params[key]).present?
plugins/discourse-templates/assets/javascripts/discourse/components/d-templates/filterable-list.gjs
+18 -1
View File
@@ -29,6 +29,23 @@ export default class DTemplatesFilterableList extends Component {
@tracked selectedTag = ALL_TAGS_ID;
@tracked availableTags = [];
@computed("availableTags.[]", "selectedTag")
get selectedTagObject() {
if (!this.selectedTag || this.selectedTag === ALL_TAGS_ID) {
return null;
}
if (this.selectedTag === NO_TAG_ID) {
return { name: NO_TAG_ID };
}
return (
this.availableTags.find((tag) => tag.id === this.selectedTag) || {
name: this.selectedTag,
}
);
}
@computed("replies", "selectedTag", "listFilter")
get filteredReplies() {
const filterTitle = this.listFilter.toLowerCase();
@@ -142,7 +159,7 @@ export default class DTemplatesFilterableList extends Component {
{{#if this.siteSettings.tagging_enabled}}
<TagDrop
@availableTags={{this.availableTags}}
@tagId={{this.selectedTag}}
@tag={{this.selectedTagObject}}
@onChangeSelectedTag={{this.changeSelectedTag}}
/>
{{/if}}
spec/models/tag_spec.rb
+1 -1
View File
@@ -211,7 +211,7 @@ RSpec.describe Tag do
SiteSetting.pm_tags_allowed_for_groups = "1|2|3"
tags = Tag.pm_tags(guardian: Guardian.new(admin), allowed_user: regular_user)
expect(tags.length).to eq(2)
expect(tags.map { |t| t[:id] }).to contain_exactly("tag-0", "tag-1")
expect(tags.map { |t| t[:name] }).to contain_exactly("tag-0", "tag-1")
end
end
spec/requests/api/tags_spec.rb
+4 -1
View File
@@ -186,11 +186,14 @@ RSpec.describe "tags" do
type: :object,
properties: {
id: {
type: :string,
type: :integer,
},
text: {
type: :string,
},
name: {
type: :string,
},
count: {
type: :integer,
},
spec/requests/tags_controller_spec.rb
+117 -42
View File
@@ -39,13 +39,13 @@ RSpec.describe TagsController do
tags = response.parsed_body["tags"]
serialized_tag = tags.find { |t| t["id"] == test_tag.name }
serialized_tag = tags.find { |t| t["name"] == test_tag.name }
expect(serialized_tag["count"]).to eq(0)
expect(serialized_tag["pm_count"]).to eq(nil)
expect(serialized_tag["pm_only"]).to eq(false)
serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
serialized_tag = tags.find { |t| t["name"] == topic_tag.name }
expect(serialized_tag["count"]).to eq(1)
expect(serialized_tag["pm_count"]).to eq(nil)
@@ -82,17 +82,17 @@ RSpec.describe TagsController do
tags = response.parsed_body["tags"]
serialized_tag = tags.find { |t| t["id"] == test_tag.name }
serialized_tag = tags.find { |t| t["name"] == test_tag.name }
expect(serialized_tag["pm_count"]).to eq(0)
expect(serialized_tag["pm_only"]).to eq(false)
serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
serialized_tag = tags.find { |t| t["name"] == topic_tag.name }
expect(serialized_tag["pm_count"]).to eq(5)
expect(serialized_tag["pm_only"]).to eq(false)
serialized_tag = tags.find { |t| t["id"] == pm_only_tag.name }
serialized_tag = tags.find { |t| t["name"] == pm_only_tag.name }
expect(serialized_tag["pm_count"]).to eq(1)
expect(serialized_tag["pm_only"]).to eq(true)
@@ -136,11 +136,11 @@ RSpec.describe TagsController do
get "/tags.json"
tags = response.parsed_body["tags"]
serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
serialized_tag = tags.find { |t| t["name"] == topic_tag.name }
expect(serialized_tag["count"]).to eq(2)
expect(serialized_tag["pm_count"]).to eq(5)
serialized_tag = tags.find { |t| t["id"] == test_tag.name }
serialized_tag = tags.find { |t| t["name"] == test_tag.name }
expect(serialized_tag["count"]).to eq(0)
expect(serialized_tag["pm_count"]).to eq(1)
end
@@ -164,9 +164,10 @@ RSpec.describe TagsController do
it "hides pm tags" do
get "/tags.json"
tags = response.parsed_body["tags"]
expect(tags.length).to eq(1)
expect(tags[0]["id"]).to eq(topic_tag.name)
expect(response.parsed_body["tags"]).to match(
[include(name: topic_tag.name, id: topic_tag.id)],
)
end
end
end
@@ -185,8 +186,12 @@ RSpec.describe TagsController do
expect(tags.length).to eq(0)
group = response.parsed_body.dig("extras", "tag_groups")&.first
expect(group).to be_present
expect(group["tags"].length).to eq(2)
expect(group["tags"].map { |t| t["id"] }).to contain_exactly(test_tag.name, topic_tag.name)
expect(group["tags"]).to match(
[
include(name: test_tag.name, id: test_tag.id),
include(name: topic_tag.name, id: topic_tag.id),
],
)
end
it "does not result in N+1 queries with multiple tag_groups" do
@@ -689,15 +694,64 @@ RSpec.describe TagsController do
sign_in(admin)
end
it "shows an unsupported error for tag id parameter" do
put "/tag/#{tag.name}.json", params: { tag: { id: "new-id" } }
expect(response.status).to eq(422)
expect(response.parsed_body["errors"]).to include(
"Updating a tag name by `id` attribute is unsupported. Use the `name` attribute instead.",
)
end
it "triggers a extensibility event" do
event =
DiscourseEvent
.track_events { put "/tag/#{tag.name}.json", params: { tag: { id: "hello" } } }
.track_events { put "/tag/#{tag.name}.json", params: { tag: { name: "hello" } } }
.last
expect(event[:event_name]).to eq(:tag_updated)
expect(event[:params].first).to eq(tag)
end
it "updates the tag" do
put "/tag/#{tag.name}.json", params: { tag: { description: "New description" } }
expect(response.status).to eq(200)
expect(tag.reload.description).to eq("New description")
end
it "returns 403 for non-admins" do
sign_in(regular_user)
put "/tag/#{tag.name}.json", params: { tag: { description: "New description" } }
expect(response.status).to eq(403)
end
it "returns 404 for non-existing tags" do
put "/tag/nonexistenttag.json", params: { tag: { description: "New description" } }
expect(response.status).to eq(404)
end
it "logs the update into a UserHistory" do
put "/tag/#{tag.name}.json", params: { tag: { name: "new tag" } }
expect(response.status).to eq(200)
history = UserHistory.where(action: UserHistory.actions[:custom_staff]).last
expect(history).to have_attributes(
custom_type: "renamed_tag",
acting_user_id: admin.id,
previous_value: tag.name,
new_value: "new-tag",
)
end
it "does not log a UserHistory if the tag name is not changed" do
expect {
put "/tag/#{tag.name}.json", params: { tag: { description: "Updated description" } }
}.to_not change { UserHistory.count }
end
end
describe "#personal_messages" do
@@ -740,7 +794,7 @@ RSpec.describe TagsController do
expect(response.status).to eq(200)
tag = response.parsed_body["tags"]
expect(tag[0]["id"]).to eq("test")
expect(tag[0]["name"]).to eq("test")
end
end
@@ -753,7 +807,7 @@ RSpec.describe TagsController do
expect(response.status).to eq(200)
tag = response.parsed_body["tags"]
expect(tag[0]["id"]).to eq("test")
expect(tag[0]["name"]).to eq("test")
end
it "can see their own pm tags" do
@@ -762,7 +816,7 @@ RSpec.describe TagsController do
expect(response.status).to eq(200)
tag = response.parsed_body["tags"]
expect(tag[0]["id"]).to eq("test")
expect(tag[0]["name"]).to eq("test")
end
it "works with usernames with a period" do
@@ -813,7 +867,7 @@ RSpec.describe TagsController do
multi_tag_topic
all_tag_topic
get "/tag/#{tag.name}/l/latest.json", params: { additional_tag_ids: other_tag.name }
get "/tag/#{tag.name}/l/latest.json", params: { additional_tag_names: other_tag.name }
expect(response.status).to eq(200)
@@ -830,7 +884,7 @@ RSpec.describe TagsController do
get "/tag/#{tag.name}/l/latest.json",
params: {
additional_tag_ids: "#{other_tag.name}/#{third_tag.name}",
additional_tag_names: "#{other_tag.name}/#{third_tag.name}",
}
expect(response.status).to eq(200)
@@ -844,7 +898,7 @@ RSpec.describe TagsController do
it "does not find any tags when a tag which doesn't exist is passed" do
single_tag_topic
get "/tag/#{tag.name}/l/latest.json", params: { additional_tag_ids: "notatag" }
get "/tag/#{tag.name}/l/latest.json", params: { additional_tag_names: "notatag" }
expect(response.status).to eq(200)
@@ -1010,21 +1064,27 @@ RSpec.describe TagsController do
context "with tagging enabled" do
it "can return some tags" do
tag_names = %w[stuff stinky stumped]
tag_names.each { |name| Fabricate(:tag, name: name) }
stuff = Fabricate(:tag, name: "stuff")
stumped = Fabricate(:tag, name: "stumped")
Fabricate(:tag, name: "stinky")
get "/tags/filter/search.json", params: { q: "stu" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }.sort).to eq(%w[stuff stumped])
expect(response.parsed_body["results"]).to match(
[include(name: "stuff", id: stuff.id), include(name: "stumped", id: stumped.id)],
)
end
it "returns tags ordered by public_topic_count, and prioritises exact matches" do
Fabricate(:tag, name: "tag1", public_topic_count: 10, staff_topic_count: 10)
Fabricate(:tag, name: "tag2", public_topic_count: 100, staff_topic_count: 100)
Fabricate(:tag, name: "tag", public_topic_count: 1, staff_topic_count: 1)
tag = Fabricate(:tag, name: "tag", public_topic_count: 1, staff_topic_count: 1)
tag1 = Fabricate(:tag, name: "tag1", public_topic_count: 10, staff_topic_count: 10)
tag2 = Fabricate(:tag, name: "tag2", public_topic_count: 100, staff_topic_count: 100)
get "/tags/filter/search.json", params: { q: "tag", limit: 2 }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to eq(%w[tag tag2])
expect(response.parsed_body["results"]).to match(
[include(name: tag.name, id: tag.id), include(name: tag2.name, id: tag2.id)],
)
end
context "with category restriction" do
@@ -1065,7 +1125,8 @@ RSpec.describe TagsController do
Fabricate(:tag, name: "nope")
get "/tags/filter/search.json", params: { categoryId: category.id }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }.sort).to eq([yup.name])
expect(response.parsed_body["results"]).to match([include(name: yup.name, id: yup.id)])
end
end
@@ -1075,17 +1136,18 @@ RSpec.describe TagsController do
it "can return synonyms" do
get "/tags/filter/search.json", params: { q: "plant" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to contain_exactly(
"plant",
"plants",
expect(response.parsed_body["results"]).to match(
[include(name: tag.name, id: tag.id), include(name: synonym.name, id: synonym.id)],
)
end
it "can omit synonyms" do
get "/tags/filter/search.json", params: { q: "plant", excludeSynonyms: "true" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to contain_exactly("plant")
expect(response.parsed_body["results"]).to match([include(name: tag.name, id: tag.id)])
end
it "can return a message about synonyms not being allowed" do
@@ -1101,31 +1163,40 @@ RSpec.describe TagsController do
it "matches tags after sanitizing input" do
Fabricate(:tag, name: "yup")
Fabricate(:tag, name: "nope")
nope = Fabricate(:tag, name: "nope")
get "/tags/filter/search.json", params: { q: "N/ope" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }.sort).to eq(["nope"])
expect(response.parsed_body["results"]).to match([include(name: nope.name, id: nope.id)])
end
it "can return tags that are in secured categories but are allowed to be used" do
c = Fabricate(:private_category, group: Fabricate(:group))
Fabricate(:topic, category: c, tags: [Fabricate(:tag, name: "cooltag")])
tag = Fabricate(:tag, name: "cooltag")
Fabricate(:topic, category: c, tags: [tag])
get "/tags/filter/search.json", params: { q: "cool" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to eq(["cooltag"])
expect(response.parsed_body["results"]).to match([include(name: tag.name, id: tag.id)])
end
it "supports Chinese and Russian" do
tag_names = %w[房地产 тема-в-разработке]
tag_names.each { |name| Fabricate(:tag, name: name) }
chinese_tag = Fabricate(:tag, name: "房屋买卖")
russian_tag = Fabricate(:tag, name: "тестовая-тема")
get "/tags/filter/search.json", params: { q: "房" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to eq(["房地产"])
expect(response.parsed_body["results"]).to match(
[include(name: chinese_tag.name, id: chinese_tag.id)],
)
get "/tags/filter/search.json", params: { q: "тема" }
expect(response.status).to eq(200)
expect(response.parsed_body["results"].map { |j| j["id"] }).to eq(["тема-в-разработке"])
expect(response.parsed_body["results"]).to match(
[include(name: russian_tag.name, id: russian_tag.id)],
)
end
it "can return all the results" do
@@ -1141,9 +1212,13 @@ RSpec.describe TagsController do
}
expect(response.status).to eq(200)
expect_same_tag_names(
response.parsed_body["results"].map { |j| j["id"] },
%w[common1 common2 group1tag group1tag2],
expect(response.parsed_body["results"]).to match(
[
include(name: "common1"),
include(name: "common2"),
include(name: "group1tag"),
include(name: "group1tag2"),
],
)
end
spec/requests/topics_controller_spec.rb
+5 -5
View File
@@ -4561,7 +4561,7 @@ RSpec.describe TopicsController do
it "dismisses topics for tag" do
TopicTrackingState.expects(:publish_dismiss_new).with(user.id, topic_ids: [tag_topic.id])
put "/topics/reset-new.json?tag_id=#{tag.name}"
put "/topics/reset-new.json?tag_name=#{tag.name}"
expect(DismissedTopicUser.where(user_id: user.id).pluck(:topic_id)).to eq([tag_topic.id])
end
@@ -4583,7 +4583,7 @@ RSpec.describe TopicsController do
group.add(user)
messages =
MessageBus.track_publish do
put "/topics/reset-new.json", params: { tag_id: restricted_tag.name }
put "/topics/reset-new.json", params: { tag_name: restricted_tag.name }
end
expect(messages.size).to eq(1)
expect(messages[0].data["payload"]["topic_ids"]).to contain_exactly(
@@ -4597,7 +4597,7 @@ RSpec.describe TopicsController do
it "ignores the tag param and dismisses all topics if the user can't see the tag" do
messages =
MessageBus.track_publish do
put "/topics/reset-new.json", params: { tag_id: restricted_tag.name }
put "/topics/reset-new.json", params: { tag_name: restricted_tag.name }
end
expect(messages.size).to eq(1)
expect(messages[0].data["payload"]["topic_ids"]).to contain_exactly(
@@ -4625,7 +4625,7 @@ RSpec.describe TopicsController do
user.id,
topic_ids: [tag_and_category_topic.id],
)
put "/topics/reset-new.json?tag_id=#{tag.name}&category_id=#{category.id}"
put "/topics/reset-new.json?tag_name=#{tag.name}&category_id=#{category.id}"
expect(DismissedTopicUser.where(user_id: user.id).pluck(:topic_id)).to eq(
[tag_and_category_topic.id],
)
@@ -4856,7 +4856,7 @@ RSpec.describe TopicsController do
dismiss_topics: true,
dismiss_posts: true,
untrack: true,
tag_id: tag.name,
tag_name: tag.name,
}
expect(response.status).to eq(200)