diff --git a/app/assets/javascripts/admin/templates/web-hooks-show.hbs b/app/assets/javascripts/admin/templates/web-hooks-show.hbs index 3cd6db8c0da..cc49f3ac58c 100644 --- a/app/assets/javascripts/admin/templates/web-hooks-show.hbs +++ b/app/assets/javascripts/admin/templates/web-hooks-show.hbs @@ -54,7 +54,7 @@ {{#if showTagsFilter}}
- {{tag-chooser tags=model.tag_names everyTag=true}} + {{tag-chooser tags=model.tag_names everyTag=true excludeSynonyms=true}}
{{i18n 'admin.web_hooks.tags_filter_instructions'}}
{{/if}} diff --git a/app/assets/javascripts/discourse/adapters/tag-info.js.es6 b/app/assets/javascripts/discourse/adapters/tag-info.js.es6 new file mode 100644 index 00000000000..ca04b6d212c --- /dev/null +++ b/app/assets/javascripts/discourse/adapters/tag-info.js.es6 @@ -0,0 +1,7 @@ +import RESTAdapter from "discourse/adapters/rest"; + +export default RESTAdapter.extend({ + pathFor(store, type, id) { + return "/tags/" + id + "/info"; + } +}); diff --git a/app/assets/javascripts/discourse/components/tag-info.js.es6 b/app/assets/javascripts/discourse/components/tag-info.js.es6 new file mode 100644 index 00000000000..bb143c5b0c0 --- /dev/null +++ b/app/assets/javascripts/discourse/components/tag-info.js.es6 @@ -0,0 +1,133 @@ +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import showModal from "discourse/lib/show-modal"; +import { + default as discourseComputed, + observes +} from "discourse-common/utils/decorators"; +import Component from "@ember/component"; +import { reads, and } from "@ember/object/computed"; +import { isEmpty } from "@ember/utils"; +import Category from "discourse/models/category"; + +export default Component.extend({ + tagName: "", + loading: false, + tagInfo: null, + newSynonyms: null, + showEditControls: false, + canAdminTag: reads("currentUser.staff"), + editSynonymsMode: and("canAdminTag", "showEditControls"), + + @discourseComputed("tagInfo.tag_group_names") + tagGroupsInfo(tagGroupNames) { + return I18n.t("tagging.tag_groups_info", { + count: tagGroupNames.length, + tag_groups: tagGroupNames.join(", ") + }); + }, + + @discourseComputed("tagInfo.categories") + categoriesInfo(categories) { + return I18n.t("tagging.category_restrictions", { + count: categories.length + }); + }, + + @discourseComputed( + "tagInfo.tag_group_names", + "tagInfo.categories", + "tagInfo.synonyms" + ) + nothingToShow(tagGroupNames, categories, synonyms) { + return isEmpty(tagGroupNames) && isEmpty(categories) && isEmpty(synonyms); + }, + + @observes("expanded") + toggleExpanded() { + if (this.expanded && !this.tagInfo) { + this.loadTagInfo(); + } + }, + + loadTagInfo() { + if (this.loading) { + return; + } + this.set("loading", true); + return this.store + .find("tag-info", this.tag.id) + .then(result => { + this.set("tagInfo", result); + this.set( + "tagInfo.synonyms", + result.synonyms.map(s => this.store.createRecord("tag", s)) + ); + this.set( + "tagInfo.categories", + result.category_ids.map(id => Category.findById(id)) + ); + }) + .finally(() => this.set("loading", false)); + }, + + actions: { + toggleEditControls() { + this.toggleProperty("showEditControls"); + }, + + renameTag() { + showModal("rename-tag", { model: this.tag }); + }, + + deleteTag() { + this.sendAction("deleteAction", this.tagInfo); + }, + + unlinkSynonym(tag) { + ajax(`/tags/${this.tagInfo.name}/synonyms/${tag.id}`, { + type: "DELETE" + }) + .then(() => this.tagInfo.synonyms.removeObject(tag)) + .catch(() => bootbox.alert(I18n.t("generic_error"))); + }, + + deleteSynonym(tag) { + bootbox.confirm( + I18n.t("tagging.delete_synonym_confirm", { tag_name: tag.text }), + result => { + if (!result) return; + + tag + .destroyRecord() + .then(() => this.tagInfo.synonyms.removeObject(tag)) + .catch(() => bootbox.alert(I18n.t("generic_error"))); + } + ); + }, + + addSynonyms() { + ajax(`/tags/${this.tagInfo.name}/synonyms`, { + type: "POST", + data: { + synonyms: this.newSynonyms + } + }) + .then(result => { + if (result.success) { + this.set("newSynonyms", null); + this.loadTagInfo(); + } else if (result.failed_tags) { + bootbox.alert( + I18n.t("tagging.add_synonyms_failed", { + tag_names: Object.keys(result.failed_tags).join(", ") + }) + ); + } else { + bootbox.alert(I18n.t("generic_error")); + } + }) + .catch(popupAjaxError); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 b/app/assets/javascripts/discourse/controllers/tags-show.js.es6 index 34aa673c967..a34a368a1a4 100644 --- a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/controllers/tags-show.js.es6 @@ -26,6 +26,7 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { search: null, max_posts: null, q: null, + showInfo: false, categories: alias("site.categoriesList"), @@ -79,9 +80,9 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { return Discourse.SiteSettings.show_filter_by_tag; }, - @discourseComputed("additionalTags", "canAdminTag", "category") - showAdminControls(additionalTags, canAdminTag, category) { - return !additionalTags && canAdminTag && !category; + @discourseComputed("additionalTags", "category", "tag.id") + showToggleInfo(additionalTags, category, tagId) { + return !additionalTags && !category && tagId !== "none"; }, loadMoreTopics() { @@ -121,6 +122,10 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { this.send("invalidateModel"); }, + toggleInfo() { + this.toggleProperty("showInfo"); + }, + refresh() { // TODO: this probably doesn't work anymore return this.store @@ -131,15 +136,23 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { }); }, - deleteTag() { + deleteTag(tagInfo) { const numTopics = this.get("list.topic_list.tags.firstObject.topic_count") || 0; - const confirmText = + let confirmText = numTopics === 0 ? I18n.t("tagging.delete_confirm_no_topics") : I18n.t("tagging.delete_confirm", { count: numTopics }); + if (tagInfo.synonyms.length > 0) { + confirmText += + " " + + I18n.t("tagging.delete_confirm_synonyms", { + count: tagInfo.synonyms.length + }); + } + bootbox.confirm(confirmText, result => { if (!result) return; diff --git a/app/assets/javascripts/discourse/lib/render-tag.js.es6 b/app/assets/javascripts/discourse/lib/render-tag.js.es6 index 7a836786080..5206e2c9007 100644 --- a/app/assets/javascripts/discourse/lib/render-tag.js.es6 +++ b/app/assets/javascripts/discourse/lib/render-tag.js.es6 @@ -28,6 +28,9 @@ function defaultRenderTag(tag, params) { if (Discourse.SiteSettings.tag_style || params.style) { classes.push(params.style || Discourse.SiteSettings.tag_style); } + if (params.size) { + classes.push(params.size); + } let val = "<" + diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index ccbb0292cbb..44b6bc6e1d1 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -56,7 +56,10 @@ export default DiscourseRoute.extend(FilterModeMixin, { afterModel(tag, transition) { const controller = this.controllerFor("tags.show"); - controller.set("loading", true); + controller.setProperties({ + loading: true, + showInfo: false + }); const params = filterQueryParams(transition.to.queryParams, {}); const category = this.categorySlugPathWithID diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs index 4d27d0c101d..681e5e24dca 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs @@ -5,8 +5,9 @@ filterPlaceholder="category.tags_placeholder" tags=category.allowed_tags everyTag=true + excludeSynonyms=true unlimitedTagCount=true}} - +
diff --git a/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs b/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs index 7436a829f60..512f9c0df5e 100644 --- a/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs +++ b/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs @@ -9,7 +9,8 @@ everyTag=true allowCreate=true filterPlaceholder="tagging.groups.tags_placeholder" - unlimitedTagCount=true}} + unlimitedTagCount=true + excludeSynonyms=true}}
@@ -19,6 +20,7 @@ everyTag=true maximum=1 allowCreate=true + excludeSynonyms=true filterPlaceholder="tagging.groups.parent_tag_placeholder"}} {{i18n 'tagging.groups.parent_tag_description'}}
diff --git a/app/assets/javascripts/discourse/templates/components/tag-info.hbs b/app/assets/javascripts/discourse/templates/components/tag-info.hbs new file mode 100644 index 00000000000..0fe9b385968 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/tag-info.hbs @@ -0,0 +1,73 @@ +{{#if expanded}} +
+ {{#if tagInfo}} +
+ {{discourse-tag tagInfo.name tagName="div" size="large"}} + {{#if canAdminTag}} + {{d-button class="btn-default" action=(action "renameTag") icon="pencil-alt" label="tagging.rename_tag" id="rename-tag"}} + {{d-button class="btn-default" action=(action "toggleEditControls") icon="cog" label="tagging.edit_synonyms" id="edit-synonyms"}} + {{#if deleteAction}} + {{d-button class="btn-danger delete-tag" action=(action "deleteTag") icon="far-trash-alt" label="tagging.delete_tag" id="delete-tag"}} + {{/if}} + {{/if}} +
+
+ {{#if tagInfo.tag_group_names}} + {{tagGroupsInfo}} + {{/if}} + {{#if tagInfo.categories}} + {{categoriesInfo}} +
+ {{#each tagInfo.categories as |category|}} + {{category-link category}} + {{/each}} + {{/if}} + {{#if nothingToShow}} + {{i18n "tagging.default_info"}} + {{/if}} +
+ {{#if tagInfo.synonyms}} +
+

{{i18n "tagging.synonyms"}}

+
{{{i18n "tagging.synonyms_description" base_tag_name=tagInfo.name}}}
+
+ {{#each tagInfo.synonyms as |tag|}} +
+ {{discourse-tag tag.id pmOnly=tag.pmOnly tagName="div"}} + {{#if editSynonymsMode}} + + {{d-icon "unlink" title="tagging.remove_synonym"}} + + + {{d-icon "far-trash-alt" title="tagging.delete_tag"}} + + {{/if}} +
+ {{/each}} +
+
+
+ {{/if}} + {{#if editSynonymsMode}} +
+ + {{tag-chooser + id="add-synonyms" + tags=newSynonyms + everyTag=true + excludeSynonyms=true + excludeHasSynonyms=true + unlimitedTagCount=true}} +
+ {{d-button + class="btn-default" + action=(action "addSynonyms") + disabled=addSynonymsDisabled + label="tagging.add_synonyms"}} + {{/if}} + {{/if}} + {{#if loading}} +
{{i18n 'loading'}}
+ {{/if}} +
+{{/if}} diff --git a/app/assets/javascripts/discourse/templates/components/tag-list.hbs b/app/assets/javascripts/discourse/templates/components/tag-list.hbs index dcbb0035e45..96ef2cd78c5 100644 --- a/app/assets/javascripts/discourse/templates/components/tag-list.hbs +++ b/app/assets/javascripts/discourse/templates/components/tag-list.hbs @@ -13,4 +13,3 @@ {{/each}}
-
diff --git a/app/assets/javascripts/discourse/templates/tags/show.hbs b/app/assets/javascripts/discourse/templates/tags/show.hbs index d63e7b7cb64..d7f98ea87b0 100644 --- a/app/assets/javascripts/discourse/templates/tags/show.hbs +++ b/app/assets/javascripts/discourse/templates/tags/show.hbs @@ -39,14 +39,17 @@ label=createTopicLabel action=(route-action "createTopic")}} - {{#if showAdminControls}} - {{d-button action=(route-action "renameTag") actionParam=tag icon="pencil-alt" class="admin-tag"}} - {{d-button action=(action "deleteTag") icon="far-trash-alt" class="admin-tag btn-danger"}} - {{/if}} + {{#if showToggleInfo}} + {{d-button icon="tag" label="tagging.info" action=(action "toggleInfo") id="show-tag-info"}} + {{/if}}
+{{#if showToggleInfo}} + {{tag-info tag=tag expanded=showInfo list=list deleteAction=(action "deleteTag")}} +{{/if}} + {{plugin-outlet name="discovery-list-container-top"}}
diff --git a/app/assets/javascripts/select-kit/components/single-select.js.es6 b/app/assets/javascripts/select-kit/components/single-select.js.es6 index 92c86cc4c5c..14047fbb038 100644 --- a/app/assets/javascripts/select-kit/components/single-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/single-select.js.es6 @@ -285,7 +285,11 @@ export default SelectKitComponent.extend({ this ); - this._boundaryActionHandler("onSelect", computedContentItem.value); + this._boundaryActionHandler( + "onSelect", + computedContentItem.value, + computedContentItem.originalContent + ); this._boundaryActionHandler("onSelectAny", computedContentItem); this.autoHighlight(); diff --git a/app/assets/javascripts/select-kit/components/tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/tag-chooser.js.es6 index 0d49d97176b..103a294060d 100644 --- a/app/assets/javascripts/select-kit/components/tag-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/tag-chooser.js.es6 @@ -17,6 +17,8 @@ export default MultiSelectComponent.extend(TagsMixin, { attributeBindings: ["categoryId"], allowCreate: null, allowAny: alias("allowCreate"), + excludeSynonyms: false, + excludeHasSynonyms: false, init() { this._super(...arguments); @@ -118,6 +120,8 @@ export default MultiSelectComponent.extend(TagsMixin, { } if (!this.everyTag) data.filterForInput = true; + if (this.excludeSynonyms) data.excludeSynonyms = true; + if (this.excludeHasSynonyms) data.excludeHasSynonyms = true; this.searchTags("/tags/filter/search", data, this._transformJson); }, diff --git a/app/assets/javascripts/select-kit/components/tag-drop.js.es6 b/app/assets/javascripts/select-kit/components/tag-drop.js.es6 index 601398eb40a..5de47f178b9 100644 --- a/app/assets/javascripts/select-kit/components/tag-drop.js.es6 +++ b/app/assets/javascripts/select-kit/components/tag-drop.js.es6 @@ -168,12 +168,16 @@ export default ComboBoxComponent.extend(TagsMixin, { results = results.sort((a, b) => a.id > b.id); return results.map(r => { - return { id: r.id, name: r.text }; + return { + id: r.id, + name: r.text, + targetTagId: r.target_tag || r.id + }; }); }, actions: { - onSelect(tagId) { + onSelect(tagId, tag) { let url; if (tagId === "all-tags") { @@ -189,7 +193,12 @@ export default ComboBoxComponent.extend(TagsMixin, { }`; } - url = Discourse.getURL(`${url}/${tagId.toLowerCase()}`); + if (tag && tag.targetTagId) { + url += `/${tag.targetTagId.toLowerCase()}`; + } else { + url += `/${tagId.toLowerCase()}`; + } + url = Discourse.getURL(url); } DiscourseURL.routeTo(url); diff --git a/app/assets/stylesheets/common/base/tagging.scss b/app/assets/stylesheets/common/base/tagging.scss index 0502b687170..ee886255789 100644 --- a/app/assets/stylesheets/common/base/tagging.scss +++ b/app/assets/stylesheets/common/base/tagging.scss @@ -5,6 +5,8 @@ .tag-list { margin-top: 2em; + padding-bottom: 1em; + border-bottom: 1px solid $primary-low; } #list-area .tag-list h3 { @@ -88,6 +90,10 @@ $tag-color: $primary-medium; color: $header-primary_high !important; } + &.large { + font-size: $font-up-2; + } + &.box { background-color: $primary-low; color: $primary-high; @@ -104,6 +110,25 @@ $tag-color: $primary-medium; margin-right: 0; color: $primary-high; } + + &.bullet { + margin-right: 0.5em; + display: inline-flex; + align-items: center; + &:before { + background: $primary-low-mid; + margin-right: 5px; + position: relative; + width: 9px; + height: 9px; + display: inline-block; + content: ""; + } + &.large:before { + width: 13px; + height: 13px; + } + } } .discourse-tags, @@ -152,21 +177,6 @@ $tag-color: $primary-medium; } } -.discourse-tag.bullet { - margin-right: 0.5em; - display: inline-flex; - align-items: center; - &:before { - background: $primary-low-mid; - margin-right: 5px; - position: relative; - width: 9px; - height: 9px; - display: inline-block; - content: ""; - } -} - header .discourse-tag { color: $tag-color; } @@ -258,3 +268,26 @@ header .discourse-tag { } } } + +.tag-info { + margin-top: 1em; + margin-bottom: 1em; + + .delete-tag { + float: right; + } + .synonyms-list, + .add-synonyms, + .tag-associations { + margin-top: 1em; + } + .tag-list { + border: none; + .d-icon { + color: $primary-medium; + } + } + .field { + margin-bottom: 5px; + } +} diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index e2aee6abe6b..55ec294cdf1 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -5,6 +5,7 @@ class TagsController < ::ApplicationController include TopicQueryParams before_action :ensure_tags_enabled + before_action :ensure_visible, only: [:show, :info] requires_login except: [ :index, @@ -12,13 +13,16 @@ class TagsController < ::ApplicationController :tag_feed, :search, :check_hashtag, + :info, Discourse.anonymous_filters.map { |f| :"show_#{f}" } ].flatten skip_before_action :check_xhr, only: [:tag_feed, :show, :index] before_action :set_category_from_params, except: [:index, :update, :destroy, - :tag_feed, :search, :notifications, :update_notifications, :personal_messages] + :tag_feed, :search, :notifications, :update_notifications, :personal_messages, :info] + + before_action :fetch_tag, only: [:info, :create_synonyms, :destroy_synonym] def index @description_meta = I18n.t("tags.title") @@ -31,21 +35,21 @@ class TagsController < ::ApplicationController ungrouped_tags = ungrouped_tags.where("tags.topic_count > 0") unless show_all_tags grouped_tag_counts = TagGroup.visible(guardian).order('name ASC').includes(:tags).map do |tag_group| - { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(tag_group.tags) } + { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(tag_group.tags.where(target_tag_id: nil)) } end @tags = self.class.tag_counts_json(ungrouped_tags) @extras = { tag_groups: grouped_tag_counts } else tags = show_all_tags ? Tag.all : Tag.where("tags.topic_count > 0") - unrestricted_tags = DiscourseTagging.filter_visible(tags, guardian) + unrestricted_tags = DiscourseTagging.filter_visible(tags.where(target_tag_id: nil), guardian) categories = Category.where("id IN (SELECT category_id FROM category_tags)") .where("id IN (?)", guardian.allowed_category_ids) .includes(:tags) category_tag_counts = categories.map do |c| - { id: c.id, tags: self.class.tag_counts_json(c.tags) } + { id: c.id, tags: self.class.tag_counts_json(c.tags.where(target_tag_id: nil)) } end @tags = self.class.tag_counts_json(unrestricted_tags) @@ -98,11 +102,13 @@ class TagsController < ::ApplicationController end def show - raise Discourse::NotFound if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id]) - show_latest end + def info + render_serialized(@tag, DetailedTagSerializer, root: :tag_info) + end + def update guardian.ensure_can_admin_tags! @@ -196,7 +202,9 @@ class TagsController < ::ApplicationController filter_params = { for_input: params[:filterForInput], selected_tags: params[:selected_tags], - limit: params[:limit] + limit: params[:limit], + exclude_synonyms: params[:excludeSynonyms], + exclude_has_synonyms: params[:excludeHasSynonyms] } if params[:categoryId] @@ -224,19 +232,25 @@ class TagsController < ::ApplicationController # filter_allowed_tags determined that the tag entered is not allowed json_response[:forbidden] = params[:q] - category_names = tag.categories.where(id: guardian.allowed_category_ids).pluck(:name) - category_names += Category.joins(tag_groups: :tags).where(id: guardian.allowed_category_ids, "tags.id": tag.id).pluck(:name) - - if category_names.present? - category_names.uniq! - json_response[:forbidden_message] = I18n.t( - "tags.forbidden.restricted_to", - count: category_names.count, - tag_name: tag.name, - category_names: category_names.join(", ") - ) + if filter_params[:exclude_synonyms] && tag.synonym? + json_response[:forbidden_message] = I18n.t("tags.forbidden.synonym", tag_name: tag.target_tag.name) + elsif filter_params[:exclude_has_synonyms] && tag.synonyms.exists? + json_response[:forbidden_message] = I18n.t("tags.forbidden.has_synonyms", tag_name: tag.name) else - json_response[:forbidden_message] = I18n.t("tags.forbidden.in_this_category", tag_name: tag.name) + category_names = tag.categories.where(id: guardian.allowed_category_ids).pluck(:name) + category_names += Category.joins(tag_groups: :tags).where(id: guardian.allowed_category_ids, "tags.id": tag.id).pluck(:name) + + if category_names.present? + category_names.uniq! + json_response[:forbidden_message] = I18n.t( + "tags.forbidden.restricted_to", + count: category_names.count, + tag_name: tag.name, + category_names: category_names.join(", ") + ) + else + json_response[:forbidden_message] = I18n.t("tags.forbidden.in_this_category", tag_name: tag.name) + end end end @@ -276,14 +290,56 @@ class TagsController < ::ApplicationController render json: { tags: pm_tags } end + def create_synonyms + guardian.ensure_can_admin_tags! + value = DiscourseTagging.add_or_create_synonyms_by_name(@tag, params[:synonyms]) + if value.is_a?(Array) + render json: failed_json.merge( + failed_tags: value.inject({}) { |h, t| h[t.name] = t.errors.full_messages.first; h } + ) + else + render json: success_json + end + end + + def destroy_synonym + guardian.ensure_can_admin_tags! + synonym = Tag.where_name(params[:synonym_id]).first + raise Discourse::NotFound unless synonym + if synonym.target_tag == @tag + synonym.update!(target_tag: nil) + render json: success_json + else + render json: failed_json, status: 400 + end + end + private + def fetch_tag + @tag = Tag.find_by_name(params[:tag_id].force_encoding("UTF-8")) + raise Discourse::NotFound unless @tag + end + def ensure_tags_enabled raise Discourse::NotFound unless SiteSetting.tagging_enabled? end + def ensure_visible + raise Discourse::NotFound if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id]) + end + def self.tag_counts_json(tags) - tags.map { |t| { id: t.name, text: t.name, count: t.topic_count, pm_count: t.pm_topic_count } } + target_tags = Tag.where(id: tags.map(&:target_tag_id).compact.uniq).select(:id, :name) + tags.map do |t| + { + id: t.name, + text: t.name, + count: t.topic_count, + pm_count: t.pm_topic_count, + target_tag: t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil + } + end end def set_category_from_params diff --git a/app/models/tag.rb b/app/models/tag.rb index 375832614da..30b86b38cf0 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -5,15 +5,17 @@ class Tag < ActiveRecord::Base include HasDestroyedWebHook validates :name, presence: true, uniqueness: { case_sensitive: false } + validate :target_tag_validator, if: Proc.new { |t| t.new_record? || t.will_save_change_to_target_tag_id? } scope :where_name, ->(name) do name = Array(name).map(&:downcase) - where("lower(name) IN (?)", name) + where("lower(tags.name) IN (?)", name) end scope :unused, -> { where(topic_count: 0, pm_topic_count: 0) } + scope :base_tags, -> { where(target_tag_id: nil) } - has_many :tag_users # notification settings + has_many :tag_users, dependent: :destroy # notification settings has_many :topic_tags, dependent: :destroy has_many :topics, through: :topic_tags @@ -21,10 +23,14 @@ class Tag < ActiveRecord::Base has_many :category_tags, dependent: :destroy has_many :categories, through: :category_tags - has_many :tag_group_memberships + has_many :tag_group_memberships, dependent: :destroy has_many :tag_groups, through: :tag_group_memberships + belongs_to :target_tag, class_name: "Tag", optional: true + has_many :synonyms, class_name: "Tag", foreign_key: "target_tag_id", dependent: :destroy + after_save :index_search + after_save :update_synonym_associations after_commit :trigger_tag_created_event, on: :create after_commit :trigger_tag_updated_event, on: :update @@ -137,6 +143,25 @@ class Tag < ActiveRecord::Base SearchIndexer.index(self) end + def synonym? + !self.target_tag_id.nil? + end + + def target_tag_validator + if synonyms.exists? + errors.add(:target_tag_id, I18n.t("tags.synonyms_exist")) + elsif target_tag&.synonym? + errors.add(:target_tag_id, I18n.t("tags.invalid_target_tag")) + end + end + + def update_synonym_associations + if target_tag_id && saved_change_to_target_tag_id? + target_tag.tag_groups.each { |tag_group| tag_group.tags << self unless tag_group.tags.include?(self) } + target_tag.categories.each { |category| category.tags << self unless category.tags.include?(self) } + end + end + %i{ tag_created tag_updated diff --git a/app/models/tag_user.rb b/app/models/tag_user.rb index 820d750b663..b4bc5d9f179 100644 --- a/app/models/tag_user.rb +++ b/app/models/tag_user.rb @@ -21,6 +21,12 @@ class TagUser < ActiveRecord::Base tag_ids = tags.empty? ? [] : Tag.where_name(tags).pluck(:id) + Tag.where_name(tags).joins(:target_tag).each do |tag| + tag_ids[tag_ids.index(tag.id)] = tag.target_tag_id + end + + tag_ids.uniq! + remove = (old_ids - tag_ids) if remove.present? records.where('tag_id in (?)', remove).destroy_all @@ -41,7 +47,17 @@ class TagUser < ActiveRecord::Base end def self.change(user_id, tag_id, level) - tag_id = tag_id.id if tag_id.is_a?(::Tag) + if tag_id.is_a?(::Tag) + tag = tag_id + tag_id = tag.id + else + tag = Tag.find_by_id(tag_id) + end + + if tag.synonym? + tag_id = tag.target_tag_id + end + user_id = user_id.id if user_id.is_a?(::User) tag_id = tag_id.to_i diff --git a/app/serializers/detailed_tag_serializer.rb b/app/serializers/detailed_tag_serializer.rb new file mode 100644 index 00000000000..25b0c555c9e --- /dev/null +++ b/app/serializers/detailed_tag_serializer.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class DetailedTagSerializer < TagSerializer + attributes :synonyms, :tag_group_names + + has_many :categories, serializer: BasicCategorySerializer + + def synonyms + TagsController.tag_counts_json(object.synonyms) + end + + def categories + Category.secured(scope).where( + id: object.categories.pluck(:id) + + object.tag_groups.includes(:categories).map do |tg| + tg.categories.map(&:id) + end.flatten + ) + end + + def include_tag_group_names? + scope.is_admin? || SiteSetting.tags_listed_by_group == true + end + + def tag_group_names + object.tag_groups.map(&:name) + end +end diff --git a/app/serializers/tag_group_serializer.rb b/app/serializers/tag_group_serializer.rb index 3fa051bfa96..db61936fceb 100644 --- a/app/serializers/tag_group_serializer.rb +++ b/app/serializers/tag_group_serializer.rb @@ -4,7 +4,7 @@ class TagGroupSerializer < ApplicationSerializer attributes :id, :name, :tag_names, :parent_tag_name, :one_per_topic, :permissions def tag_names - object.tags.map(&:name).sort + object.tags.base_tags.map(&:name).sort end def parent_tag_name diff --git a/app/services/search_indexer.rb b/app/services/search_indexer.rb index 8b3c84944b9..17817c4c099 100644 --- a/app/services/search_indexer.rb +++ b/app/services/search_indexer.rb @@ -137,7 +137,12 @@ class SearchIndexer end category_name = topic.category&.name if topic - tag_names = topic.tags.pluck(:name).join(' ') if topic + if topic + tags = topic.tags.select(:id, :name) + unless tags.empty? + tag_names = (tags.map(&:name) + Tag.where(target_tag_id: tags.map(&:id)).pluck(:name)).join(' ') + end + end if Post === obj && obj.raw.present? && ( diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a7ce0e80b9a..8174e4aa633 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3043,11 +3043,30 @@ en: changed: "tags changed:" tags: "Tags" choose_for_topic: "optional tags" + info: "Info" + default_info: "This tag isn't restricted to any categories, and has no synonyms." + synonyms: "Synonyms" + synonyms_description: "When the following tags are used, they will be replaced with %{base_tag_name}." + tag_groups_info: + one: 'This tag belongs to the group "{{tag_groups}}".' + other: "This tag belongs to these groups: {{tag_groups}}." + category_restrictions: + one: "It can only be used in this category:" + other: "It can only be used in these categories:" + edit_synonyms: "Manage Synonyms" + add_synonyms_label: "Add synonyms:" + add_synonyms: "Add" + add_synonyms_failed: "The following tags couldn't be added as synonyms: %{tag_names}. Ensure they don't have synonyms and aren't synonyms of another tag." + remove_synonym: "Remove Synonym" + delete_synonym_confirm: 'Are you sure you want to delete the synonym "%{tag_name}"?' delete_tag: "Delete Tag" delete_confirm: one: "Are you sure you want to delete this tag and remove it from %{count} topic it is assigned to?" other: "Are you sure you want to delete this tag and remove it from {{count}} topics it is assigned to?" delete_confirm_no_topics: "Are you sure you want to delete this tag?" + delete_confirm_synonyms: + one: "Its synonym will also be deleted." + other: "Its {{count}} synonyms will also be deleted." rename_tag: "Rename Tag" rename_instructions: "Choose a new name for the tag:" sort_by: "Sort by:" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index bb82cba5b67..85dc5239576 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -4378,9 +4378,13 @@ en: restricted_to: one: '"%{tag_name}" is restricted to the "%{category_names}" category' other: '"%{tag_name}" is restricted to the following categories: %{category_names}' + synonym: 'Synonyms are not allowed. Use "%{tag_name}" instead.' + has_synonyms: '"%{tag_name}" cannot be used because it has synonyms.' required_tags_from_group: one: "You must include at least %{count} %{tag_group_name} tag." other: "You must include at least %{count} %{tag_group_name} tags." + invalid_target_tag: "cannot be a synonym of a synonym" + synonyms_exist: "is not allowed while synonyms exist" rss_by_tag: "Topics tagged %{tag}" finish_installation: diff --git a/config/routes.rb b/config/routes.rb index 7fd6e9cef81..0c32e16ec86 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -862,10 +862,13 @@ Discourse::Application.routes.draw do get '/:tag_id.rss' => 'tags#tag_feed' get '/:tag_id' => 'tags#show', as: 'tag_show' get '/intersection/:tag_id/*additional_tag_ids' => 'tags#show', as: 'tag_intersection' + get '/:tag_id/info' => 'tags#info' get '/:tag_id/notifications' => 'tags#notifications' put '/:tag_id/notifications' => 'tags#update_notifications' put '/:tag_id' => 'tags#update' delete '/:tag_id' => 'tags#destroy' + post '/:tag_id/synonyms' => 'tags#create_synonyms' + delete '/:tag_id/synonyms/:synonym_id' => 'tags#destroy_synonym' Discourse.filters.each do |filter| get "/:tag_id/l/#{filter}" => "tags#show_#{filter}", as: "tag_show_#{filter}" diff --git a/db/migrate/20191113193141_add_target_tag_id_to_tags.rb b/db/migrate/20191113193141_add_target_tag_id_to_tags.rb new file mode 100644 index 00000000000..75d19e272bb --- /dev/null +++ b/db/migrate/20191113193141_add_target_tag_id_to_tags.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTargetTagIdToTags < ActiveRecord::Migration[6.0] + def change + add_column :tags, :target_tag_id, :integer + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 5b7996a946b..50faf123a75 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -17,6 +17,12 @@ module DiscourseTagging if guardian.can_tag?(topic) tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || [] + if !tag_names.empty? + Tag.where_name(tag_names).joins(:target_tag).includes(:target_tag).each do |tag| + tag_names[tag_names.index(tag.name)] = tag.target_tag.name + end + end + old_tag_names = topic.tags.pluck(:name) || [] new_tag_names = tag_names - old_tag_names removed_tag_names = old_tag_names - tag_names @@ -190,6 +196,7 @@ module DiscourseTagging # for_topic: results are for tagging a topic # selected_tags: an array of tag names that are in the current selection # only_tag_names: limit results to tags with these names + # exclude_synonyms: exclude synonyms from results def self.filter_allowed_tags(guardian, opts = {}) selected_tag_ids = opts[:selected_tags] ? Tag.where_name(opts[:selected_tags]).pluck(:id) : [] category = opts[:category] @@ -215,7 +222,7 @@ module DiscourseTagging sql << <<~SQL SELECT t.id, t.name, t.topic_count, t.pm_topic_count, tgr.tgm_id as tgm_id, tgr.tag_group_id as tag_group_id, tgr.parent_tag_id as parent_tag_id, - tgr.one_per_topic as one_per_topic + tgr.one_per_topic as one_per_topic, t.target_tag_id FROM tags t INNER JOIN tag_group_restrictions tgr ON tgr.tag_id = t.id #{outer_join ? "LEFT OUTER" : "INNER"} @@ -307,6 +314,14 @@ module DiscourseTagging end end + if opts[:exclude_synonyms] + builder.where("target_tag_id IS NULL") + end + + if opts[:exclude_has_synonyms] + builder.where("id NOT IN (SELECT target_tag_id FROM tags WHERE target_tag_id IS NOT NULL)") + end + builder.limit(opts[:limit]) if opts[:limit] if opts[:order] builder.order_by(opts[:order]) @@ -383,15 +398,28 @@ module DiscourseTagging tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, Guardian.new(Discourse.system_user), opts) || [] if taggable.tags.pluck(:name).sort != tag_names.sort taggable.tags = Tag.where_name(tag_names).all - if taggable.tags.size < tag_names.size - new_tag_names = tag_names - taggable.tags.map(&:name) - new_tag_names.each do |name| - taggable.tags << Tag.create(name: name) - end + new_tag_names = taggable.tags.size < tag_names.size ? tag_names - taggable.tags.map(&:name) : [] + taggable.tags << Tag.where(target_tag_id: taggable.tags.map(&:id)).all + new_tag_names.each do |name| + taggable.tags << Tag.create(name: name) end end end + # Returns true if all were added successfully, or an Array of the + # tags that failed to be added, with errors on each Tag. + def self.add_or_create_synonyms_by_name(target_tag, synonym_names) + tag_names = DiscourseTagging.tags_for_saving(synonym_names, Guardian.new(Discourse.system_user)) || [] + existing = Tag.where_name(tag_names).all + target_tag.synonyms << existing + (tag_names - target_tag.synonyms.map(&:name)).each do |name| + target_tag.synonyms << Tag.create(name: name) + end + successful = existing.select { |t| !t.errors.present? } + TopicTag.where(tag_id: successful.map(&:id)).update_all(tag_id: target_tag.id) + (existing - successful).presence || true + end + def self.muted_tags(user) return [] unless user TagUser.lookup(user, :muted).joins(:tag).pluck('tags.name') diff --git a/lib/topic_query.rb b/lib/topic_query.rb index de789eadb19..7e63bb024a4 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -699,18 +699,15 @@ class TopicQuery end end - # ALL TAGS: something like this? - # Topic.joins(:tags).where('tags.name in (?)', @options[:tags]).group('topic_id').having('count(*)=?', @options[:tags].size).select('topic_id') - if SiteSetting.tagging_enabled result = result.preload(:tags) - tags = @options[:tags] + tags_arg = @options[:tags] - if tags && tags.size > 0 - tags = tags.split if String === tags + if tags_arg && tags_arg.size > 0 + tags_arg = tags_arg.split if String === tags_arg - tags = tags.map do |t| + tags_arg = tags_arg.map do |t| if String === t t.downcase else @@ -718,12 +715,12 @@ class TopicQuery end end + tags_query = tags_arg[0].is_a?(String) ? Tag.where_name(tags_arg) : Tag.where(id: tags_arg) + tags = tags_query.select(:id, :target_tag_id).map { |t| t.target_tag_id || t.id }.uniq + if @options[:match_all_tags] # ALL of the given tags: - tags_count = tags.length - tags = Tag.where_name(tags).pluck(:id) unless Integer === tags[0] - - if tags_count == tags.length + if tags_arg.length == tags.length tags.each_with_index do |tag, index| sql_alias = ['t', index].join result = result.joins("INNER JOIN topic_tags #{sql_alias} ON #{sql_alias}.topic_id = topics.id AND #{sql_alias}.tag_id = #{tag}") @@ -733,12 +730,7 @@ class TopicQuery end else # ANY of the given tags: - result = result.joins(:tags) - if Integer === tags[0] - result = result.where("tags.id in (?)", tags) - else - result = result.where("lower(tags.name) in (?)", tags) - end + result = result.joins(:tags).where("tags.id in (?)", tags) end # TODO: this is very side-effecty and should be changed diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb index 0b0bd1ad1ce..dcd0f7f1c3d 100644 --- a/spec/components/discourse_tagging_spec.rb +++ b/spec/components/discourse_tagging_spec.rb @@ -8,10 +8,6 @@ require 'discourse_tagging' describe DiscourseTagging do - def sorted_tag_names(tag_records) - tag_records.map(&:name).sort - end - fab!(:admin) { Fabricate(:admin) } fab!(:user) { Fabricate(:user) } let(:guardian) { Guardian.new(user) } @@ -132,6 +128,46 @@ describe DiscourseTagging do expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag3])) end end + + context 'tag synonyms' do + fab!(:base_tag) { Fabricate(:tag, name: 'discourse') } + fab!(:synonym) { Fabricate(:tag, name: 'discource', target_tag: base_tag) } + + it 'returns synonyms by default' do + tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user), + for_input: true, + term: 'disc' + ).map(&:name) + expect(tags).to contain_exactly(base_tag.name, synonym.name) + end + + it 'excludes synonyms with exclude_synonyms param' do + tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user), + for_input: true, + exclude_synonyms: true, + term: 'disc' + ).map(&:name) + expect(tags).to contain_exactly(base_tag.name) + end + + it 'excludes tags with synonyms with exclude_has_synonyms params' do + tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user), + for_input: true, + exclude_has_synonyms: true, + term: 'disc' + ).map(&:name) + expect(tags).to contain_exactly(synonym.name) + end + + it 'can exclude synonyms and tags with synonyms' do + expect(DiscourseTagging.filter_allowed_tags(Guardian.new(user), + for_input: true, + exclude_has_synonyms: true, + exclude_synonyms: true, + term: 'disc' + )).to be_empty + end + end end end @@ -357,6 +393,27 @@ describe DiscourseTagging do expect(valid).to eq(true) end end + + context 'tag synonyms' do + fab!(:topic) { Fabricate(:topic) } + + fab!(:syn1) { Fabricate(:tag, name: 'synonym1', target_tag: tag1) } + fab!(:syn2) { Fabricate(:tag, name: 'synonym2', target_tag: tag1) } + + it "uses the base tag when a synonym is given" do + valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [syn1.name]) + expect(valid).to eq(true) + expect(topic.errors[:base]).to be_empty + expect_same_tag_names(topic.reload.tags, [tag1]) + end + + it "handles multiple synonyms for the same tag" do + valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag1.name, syn1.name, syn2.name]) + expect(valid).to eq(true) + expect(topic.errors[:base]).to be_empty + expect_same_tag_names(topic.reload.tags, [tag1]) + end + end end describe '#tags_for_saving' do @@ -440,4 +497,67 @@ describe DiscourseTagging do expect(DiscourseTagging.staff_tag_names).to contain_exactly(other_staff_tag.name) end end + + describe '#add_or_create_synonyms_by_name' do + it "can add an existing tag" do + expect { + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name])).to eq(true) + }.to_not change { Tag.count } + expect_same_tag_names(tag1.reload.synonyms, [tag2]) + expect(tag2.reload.target_tag).to eq(tag1) + end + + it "can add existing tag with wrong case" do + expect { + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name.upcase])).to eq(true) + }.to_not change { Tag.count } + expect_same_tag_names(tag1.reload.synonyms, [tag2]) + expect(tag2.reload.target_tag).to eq(tag1) + end + + it "can create new tags" do + expect { + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, ['synonym1'])).to eq(true) + }.to change { Tag.count }.by(1) + s = Tag.where_name('synonym1').first + expect_same_tag_names(tag1.reload.synonyms, [s]) + expect(s.target_tag).to eq(tag1) + end + + it "can add existing and new tags" do + expect { + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name, 'synonym1'])).to eq(true) + }.to change { Tag.count }.by(1) + s = Tag.where_name('synonym1').first + expect_same_tag_names(tag1.reload.synonyms, [tag2, s]) + expect(s.target_tag).to eq(tag1) + expect(tag2.reload.target_tag).to eq(tag1) + end + + it "can change a synonym's target tag" do + synonym = Fabricate(:tag, name: 'synonym1', target_tag: tag1) + expect { + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag2, [synonym.name])).to eq(true) + }.to_not change { Tag.count } + expect_same_tag_names(tag2.reload.synonyms, [synonym]) + expect(tag1.reload.synonyms.count).to eq(0) + expect(synonym.reload.target_tag).to eq(tag2) + end + + it "doesn't allow tags that have synonyms to become synonyms" do + tag2.synonyms << Fabricate(:tag) + value = DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name]) + expect(value).to be_a(Array) + expect(value.size).to eq(1) + expect(value.first.errors[:target_tag_id]).to be_present + expect(tag1.reload.synonyms.count).to eq(0) + expect(tag2.reload.synonyms.count).to eq(1) + end + + it "changes tag of topics" do + topic = Fabricate(:topic, tags: [tag2]) + expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name])).to eq(true) + expect_same_tag_names(topic.reload.tags, [tag1]) + end + end end diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 122a2e67b8c..d094403797c 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -669,13 +669,15 @@ describe Search do let(:category) { Fabricate(:category_with_definition) } context 'post searching' do - it 'can find posts with tags' do + before do SiteSetting.tagging_enabled = true - - post = Fabricate(:post, raw: 'I am special post') DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(Fabricate.build(:admin)), [tag.name, uppercase_tag.name]) post.topic.save + end + let(:post) { Fabricate(:post, raw: 'I am special post') } + + it 'can find posts with tags' do # we got to make this index (it is deferred) Jobs::ReindexSearch.new.rebuild_problem_posts @@ -690,6 +692,13 @@ describe Search do result = Search.execute(tag.name) expect(result.posts.length).to eq(0) end + + it 'can find posts with tag synonyms' do + synonym = Fabricate(:tag, name: 'synonym', target_tag: tag) + Jobs::ReindexSearch.new.rebuild_problem_posts + result = Search.execute(synonym.name) + expect(result.posts.length).to eq(1) + end end context 'tagging is disabled' do diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 7e1530abedc..558d4175464 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -178,6 +178,7 @@ describe TopicQuery do fab!(:tagged_topic3) { Fabricate(:topic, tags: [tag, other_tag]) } fab!(:tagged_topic4) { Fabricate(:topic, tags: [uppercase_tag]) } fab!(:no_tags_topic) { Fabricate(:topic) } + let(:synonym) { Fabricate(:tag, target_tag: tag, name: 'synonym') } it "returns topics with the tag when filtered to it" do expect(TopicQuery.new(moderator, tags: tag.name).list_latest.topics) @@ -210,6 +211,26 @@ describe TopicQuery do it "can return topics with no tags" do expect(TopicQuery.new(moderator, no_tags: true).list_latest.topics.map(&:id)).to eq([no_tags_topic.id]) end + + it "can filter using a synonym" do + expect(TopicQuery.new(moderator, tags: synonym.name).list_latest.topics) + .to contain_exactly(tagged_topic1, tagged_topic3) + + expect(TopicQuery.new(moderator, tags: [synonym.id]).list_latest.topics) + .to contain_exactly(tagged_topic1, tagged_topic3) + + expect(TopicQuery.new( + moderator, tags: [synonym.name, other_tag.name] + ).list_latest.topics).to contain_exactly( + tagged_topic1, tagged_topic2, tagged_topic3 + ) + + expect(TopicQuery.new(moderator, tags: [synonym.id, other_tag.id]).list_latest.topics) + .to contain_exactly(tagged_topic1, tagged_topic2, tagged_topic3) + + expect(TopicQuery.new(moderator, tags: ["SYnonYM"]).list_latest.topics) + .to contain_exactly(tagged_topic1, tagged_topic3) + end end context "and categories too" do diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb index c917ca661f3..521a3897238 100644 --- a/spec/integration/category_tag_spec.rb +++ b/spec/integration/category_tag_spec.rb @@ -5,14 +5,6 @@ require 'rails_helper' describe "category tag restrictions" do - def sorted_tag_names(tag_records) - tag_records.map { |t| t.is_a?(String) ? t : t.name }.sort - end - - def expect_same_tag_names(a, b) - expect(sorted_tag_names(a)).to eq(sorted_tag_names(b)) - end - def filter_allowed_tags(opts = {}) DiscourseTagging.filter_allowed_tags(Guardian.new(user), opts) end diff --git a/spec/models/tag_group_spec.rb b/spec/models/tag_group_spec.rb index ef00c45e1c8..f1261943791 100644 --- a/spec/models/tag_group_spec.rb +++ b/spec/models/tag_group_spec.rb @@ -87,4 +87,35 @@ describe TagGroup do include_examples "correct visible tag groups" end end + + describe 'tag_names=' do + let(:tag_group) { Fabricate(:tag_group) } + fab!(:tag) { Fabricate(:tag) } + + before { SiteSetting.tagging_enabled = true } + + it "can use existing tags and create new ones" do + expect { + tag_group.tag_names = [tag.name, 'new-tag'] + }.to change { Tag.count }.by(1) + expect_same_tag_names(tag_group.reload.tags, [tag, 'new-tag']) + end + + context 'with synonyms' do + fab!(:synonym) { Fabricate(:tag, name: 'synonym', target_tag: tag) } + + it "adds synonyms from base tags too" do + expect { + tag_group.tag_names = [tag.name, 'new-tag'] + }.to change { Tag.count }.by(1) + expect_same_tag_names(tag_group.reload.tags, [tag, 'new-tag', synonym]) + end + + it "removes tags correctly" do + tag_group.update!(tag_names: [tag.name]) + tag_group.tag_names = ['new-tag'] + expect_same_tag_names(tag_group.reload.tags, ['new-tag']) + end + end + end end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 88ac08ded58..441bb734ac3 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -13,6 +13,7 @@ describe Tag do end let(:tag) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } let(:topic) { Fabricate(:topic, tags: [tag]) } before do @@ -46,6 +47,12 @@ describe Tag do expect(event[:event_name]).to eq(:tag_destroyed) expect(event[:params].first).to eq(subject) end + + it 'removes it from its tag group' do + tag_group = Fabricate(:tag_group, tags: [tag]) + expect { tag.destroy }.to change { TagGroupMembership.count }.by(-1) + expect(tag_group.reload.tags).to be_empty + end end it "can delete tags on deleted topics" do @@ -188,4 +195,56 @@ describe Tag do expect(Tag.unused.pluck(:name)).to contain_exactly("unused1", "unused2") end end + + context "synonyms" do + let(:synonym) { Fabricate(:tag, target_tag: tag) } + + it "can be a synonym for another tag" do + expect(synonym).to be_synonym + expect(synonym.target_tag).to eq(tag) + end + + it "cannot have a synonym of a synonym" do + synonym2 = Fabricate.build(:tag, target_tag: synonym) + expect(synonym2).to_not be_valid + expect(synonym2.errors[:target_tag_id]).to be_present + end + + it "a tag with synonyms cannot become a synonym" do + synonym + tag.target_tag = Fabricate(:tag) + expect(tag).to_not be_valid + expect(tag.errors[:target_tag_id]).to be_present + end + + it "can be added to a tag group" do + tag_group = Fabricate(:tag_group, tags: [tag]) + synonym + expect(tag_group.reload.tags).to include(synonym) + end + + it "can be added to a category" do + category = Fabricate(:category, tags: [tag]) + synonym + expect(category.reload.tags).to include(synonym) + end + + it "destroying a tag destroys its synonyms" do + synonym + expect { tag.destroy }.to change { Tag.count }.by(-2) + expect(Tag.find_by_id(synonym.id)).to be_nil + end + + it "can add a tag from the same tag group as a synonym" do + tag_group = Fabricate(:tag_group, tags: [tag, tag2]) + tag2.update!(target_tag: tag) + expect(tag_group.reload.tags).to include(tag2) + end + + it "can add a tag restricted to the same category as a synonym" do + category = Fabricate(:category, tags: [tag, tag2]) + tag2.update!(target_tag: tag) + expect(category.reload.tags).to include(tag2) + end + end end diff --git a/spec/models/tag_user_spec.rb b/spec/models/tag_user_spec.rb index 258aa19f66c..23a679ca5ab 100644 --- a/spec/models/tag_user_spec.rb +++ b/spec/models/tag_user_spec.rb @@ -40,6 +40,28 @@ describe TagUser do TagUser.change(user.id, tag.id, regular) expect(TopicUser.get(topic, user).notification_level).to eq tracking end + + it "watches or tracks on change using a synonym" do + user = Fabricate(:user) + tag = Fabricate(:tag) + synonym = Fabricate(:tag, target_tag: tag) + post = create_post(tags: [tag.name]) + topic = post.topic + + TopicUser.change(user.id, topic.id, total_msecs_viewed: 1) + + TagUser.change(user.id, synonym.id, tracking) + expect(TopicUser.get(topic, user).notification_level).to eq tracking + + TagUser.change(user.id, synonym.id, watching) + expect(TopicUser.get(topic, user).notification_level).to eq watching + + TagUser.change(user.id, synonym.id, regular) + expect(TopicUser.get(topic, user).notification_level).to eq tracking + + expect(TagUser.where(user_id: user.id, tag_id: synonym.id).first).to be_nil + expect(TagUser.where(user_id: user.id, tag_id: tag.id).first).to be_present + end end context "batch_set" do @@ -65,6 +87,30 @@ describe TagUser do expect(TopicUser.get(topic, user).notification_level).to eq tracking end + + it "watches and unwatches tags correctly using tag synonym" do + + user = Fabricate(:user) + tag = Fabricate(:tag) + synonym = Fabricate(:tag, target_tag: tag) + post = create_post(tags: [tag.name]) + topic = post.topic + + # we need topic user record to ensure watch picks up other wise it is implicit + TopicUser.change(user.id, topic.id, total_msecs_viewed: 1) + + TagUser.batch_set(user, :tracking, [synonym.name]) + + expect(TopicUser.get(topic, user).notification_level).to eq tracking + + TagUser.batch_set(user, :watching, [synonym.name]) + + expect(TopicUser.get(topic, user).notification_level).to eq watching + + TagUser.batch_set(user, :watching, []) + + expect(TopicUser.get(topic, user).notification_level).to eq tracking + end end context "integration" do diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index 500988cf19b..934812ad9ce 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -23,10 +23,9 @@ describe TagsController do describe '#index' do - before do - Fabricate(:tag, name: 'test') - Fabricate(:tag, name: 'topic-test', topic_count: 1) - end + fab!(:test_tag) { Fabricate(:tag, name: 'test') } + fab!(:topic_tag) { Fabricate(:tag, name: 'topic-test', topic_count: 1) } + fab!(:synonym) { Fabricate(:tag, name: 'synonym', target_tag: topic_tag) } shared_examples "successfully retrieve tags with topic_count > 0" do it "should return the right response" do @@ -43,6 +42,19 @@ describe TagsController do context "with tags_listed_by_group enabled" do before { SiteSetting.tags_listed_by_group = true } include_examples "successfully retrieve tags with topic_count > 0" + + it "works for tags in groups" do + tag_group = Fabricate(:tag_group, tags: [test_tag, topic_tag, synonym]) + get "/tags.json" + expect(response.status).to eq(200) + + tags = json["tags"] + expect(tags.length).to eq(0) + group = json.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) + end end context "with tags_listed_by_group disabled" do @@ -79,6 +91,12 @@ describe TagsController do expect(response.status).to eq(404) end + it "should handle synonyms" do + synonym = Fabricate(:tag, target_tag: tag) + get "/tags/#{synonym.name}" + expect(response.status).to eq(200) + end + it "does not show staff-only tags" do tag_group = Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["test"]) @@ -158,6 +176,79 @@ describe TagsController do end end + describe '#info' do + fab!(:tag) { Fabricate(:tag, name: 'test') } + let(:synonym) { Fabricate(:tag, name: 'synonym', target_tag: tag) } + + it "returns 404 if tag not found" do + get "/tags/nope/info.json" + expect(response.status).to eq(404) + end + + it "can handle tag with no synonyms" do + get "/tags/#{tag.name}/info.json" + expect(response.status).to eq(200) + expect(json.dig('tag_info', 'name')).to eq(tag.name) + expect(json.dig('tag_info', 'synonyms')).to be_empty + expect(json.dig('tag_info', 'category_ids')).to be_empty + end + + it "can handle a synonym" do + get "/tags/#{synonym.name}/info.json" + expect(response.status).to eq(200) + expect(json.dig('tag_info', 'name')).to eq(synonym.name) + expect(json.dig('tag_info', 'synonyms')).to be_empty + expect(json.dig('tag_info', 'category_ids')).to be_empty + end + + it "can return a tag's synonyms" do + synonym + get "/tags/#{tag.name}/info.json" + expect(response.status).to eq(200) + expect(json.dig('tag_info', 'synonyms').map { |t| t['text'] }).to eq([synonym.name]) + end + + it "returns 404 if tag is staff-only" do + tag_group = Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["test"]) + get "/tags/test/info.json" + expect(response.status).to eq(404) + end + + it "staff-only tags can be retrieved for staff user" do + sign_in(admin) + tag_group = Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["test"]) + get "/tags/test/info.json" + expect(response.status).to eq(200) + end + + it "can return category restrictions" do + category.update!(tags: [tag]) + category2 = Fabricate(:category) + tag_group = Fabricate(:tag_group, tags: [tag]) + category2.update!(tag_groups: [tag_group]) + staff_category = Fabricate(:private_category, group: Fabricate(:group), tags: [tag]) + get "/tags/#{tag.name}/info.json" + expect(json.dig('tag_info', 'category_ids')).to contain_exactly(category.id, category2.id) + expect(json['categories']).to be_present + end + + context 'tag belongs to a tag group' do + fab!(:tag_group) { Fabricate(:tag_group, tags: [tag]) } + + it "returns tag groups if tag groups are visible" do + SiteSetting.tags_listed_by_group = true + get "/tags/#{tag.name}/info.json" + expect(json.dig('tag_info', 'tag_group_names')).to eq([tag_group.name]) + end + + it "doesn't return tag groups if tag groups aren't visible" do + SiteSetting.tags_listed_by_group = false + get "/tags/#{tag.name}/info.json" + expect(json['tag_info'].has_key?('tag_group_names')).to eq(false) + end + end + end + describe '#check_hashtag' do fab!(:tag) { Fabricate(:tag) } @@ -472,6 +563,31 @@ describe TagsController do end end + context 'with synonyms' do + fab!(:tag) { Fabricate(:tag, name: 'plant') } + fab!(:synonym) { Fabricate(:tag, name: 'plants', target_tag: tag) } + + it "can return synonyms" do + get "/tags/filter/search.json", params: { q: 'plant' } + expect(response.status).to eq(200) + expect(json['results'].map { |j| j['id'] }).to contain_exactly('plant', 'plants') + end + + it "can omit synonyms" do + get "/tags/filter/search.json", params: { q: 'plant', excludeSynonyms: 'true' } + expect(response.status).to eq(200) + expect(json['results'].map { |j| j['id'] }).to contain_exactly('plant') + end + + it "can return a message about synonyms not being allowed" do + get "/tags/filter/search.json", params: { q: 'plants', excludeSynonyms: 'true' } + expect(response.status).to eq(200) + expect(json["results"].map { |j| j["id"] }.sort).to eq([]) + expect(json["forbidden"]).to be_present + expect(json["forbidden_message"]).to eq(I18n.t("tags.forbidden.synonym", tag_name: tag.name)) + end + end + it "matches tags after sanitizing input" do yup, nope = Fabricate(:tag, name: 'yup'), Fabricate(:tag, name: 'nope') get "/tags/filter/search.json", params: { q: 'N/ope' } @@ -612,4 +728,90 @@ describe TagsController do end end end + + describe '#create_synonyms' do + fab!(:tag) { Fabricate(:tag) } + + it 'fails if not logged in' do + post "/tags/#{tag.name}/synonyms.json", params: { synonyms: ['synonym1'] } + expect(response.status).to eq(403) + end + + it 'fails if not staff user' do + sign_in(user) + post "/tags/#{tag.name}/synonyms.json", params: { synonyms: ['synonym1'] } + expect(response.status).to eq(403) + end + + context 'signed in as admin' do + before { sign_in(admin) } + + it 'can make a tag a synonym of another tag' do + tag2 = Fabricate(:tag) + expect { + post "/tags/#{tag.name}/synonyms.json", params: { synonyms: [tag2.name] } + }.to_not change { Tag.count } + expect(response.status).to eq(200) + expect(tag2.reload.target_tag).to eq(tag) + end + + it 'can create new tags at the same time' do + expect { + post "/tags/#{tag.name}/synonyms.json", params: { synonyms: ['synonym'] } + }.to change { Tag.count }.by(1) + expect(response.status).to eq(200) + expect(Tag.find_by_name('synonym')&.target_tag).to eq(tag) + end + + it 'can return errors' do + tag2 = Fabricate(:tag, target_tag: tag) + tag3 = Fabricate(:tag) + post "/tags/#{tag3.name}/synonyms.json", params: { synonyms: [tag.name] } + expect(response.status).to eq(200) + json = JSON.parse(response.body) + expect(json['failed']).to be_present + expect(json.dig('failed_tags', tag.name)).to be_present + end + end + end + + describe '#destroy_synonym' do + fab!(:tag) { Fabricate(:tag) } + fab!(:synonym) { Fabricate(:tag, target_tag: tag, name: 'synonym') } + subject { delete("/tags/#{tag.name}/synonyms/#{synonym.name}.json") } + + it 'fails if not logged in' do + subject + expect(response.status).to eq(403) + end + + it 'fails if not staff user' do + sign_in(user) + subject + expect(response.status).to eq(403) + end + + context 'signed in as admin' do + before { sign_in(admin) } + + it "can remove a synonym from a tag" do + synonym2 = Fabricate(:tag, target_tag: tag, name: 'synonym2') + expect { subject }.to_not change { Tag.count } + expect_same_tag_names(tag.reload.synonyms, [synonym2]) + expect(synonym.reload).to_not be_synonym + end + + it "returns error if tag isn't a synonym" do + delete "/tags/#{Fabricate(:tag).name}/synonyms/#{synonym.name}.json" + expect(response.status).to eq(400) + expect_same_tag_names(tag.reload.synonyms, [synonym]) + end + + it "returns error if synonym not found" do + delete "/tags/#{Fabricate(:tag).name}/synonyms/nope.json" + expect(response.status).to eq(404) + expect_same_tag_names(tag.reload.synonyms, [synonym]) + end + end + end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 9a7323a1b4e..c0f54163f80 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1701,14 +1701,16 @@ describe UsersController do let!(:user) { sign_in(Fabricate(:user)) } it 'allows the update' do + SiteSetting.tagging_enabled = true user2 = Fabricate(:user) user3 = Fabricate(:user) tags = [Fabricate(:tag), Fabricate(:tag)] + tag_synonym = Fabricate(:tag, target_tag: tags[1]) put "/u/#{user.username}.json", params: { name: 'Jim Tom', muted_usernames: "#{user2.username},#{user3.username}", - watched_tags: "#{tags[0].name},#{tags[1].name}", + watched_tags: "#{tags[0].name},#{tag_synonym.name}", card_background_upload_url: upload.url, profile_background_upload_url: upload.url } diff --git a/spec/serializers/tag_group_serializer_spec.rb b/spec/serializers/tag_group_serializer_spec.rb index b9adf22187d..92cdf76c158 100644 --- a/spec/serializers/tag_group_serializer_spec.rb +++ b/spec/serializers/tag_group_serializer_spec.rb @@ -20,4 +20,12 @@ describe TagGroupSerializer do expect(serialized[:permissions].keys).to contain_exactly("staff") end + it "doesn't return tag synonyms" do + tag = Fabricate(:tag) + synonym = Fabricate(:tag, target_tag: tag) + tag_group = Fabricate(:tag_group, tags: [tag, synonym]) + serialized = TagGroupSerializer.new(tag_group, root: false).as_json + expect(serialized[:tag_names]).to eq([tag.name]) + end + end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index c08edfefede..efffb42785f 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -120,6 +120,14 @@ module Helpers end end + def sorted_tag_names(tag_records) + tag_records.map { |t| t.is_a?(String) ? t : t.name }.sort + end + + def expect_same_tag_names(a, b) + expect(sorted_tag_names(a)).to eq(sorted_tag_names(b)) + end + def capture_stdout old_stdout = $stdout io = StringIO.new diff --git a/test/javascripts/acceptance/tags-test.js.es6 b/test/javascripts/acceptance/tags-test.js.es6 index 7dad65494c2..bf18d97ed8a 100644 --- a/test/javascripts/acceptance/tags-test.js.es6 +++ b/test/javascripts/acceptance/tags-test.js.es6 @@ -180,3 +180,140 @@ test("new topic button is not available for staff-only tags", async assert => { await visit("/tags/staff-only-tag"); assert.ok(find("#create-topic:disabled").length === 0); }); + +acceptance("Tag info", { + loggedIn: true, + settings: { + tags_listed_by_group: true + }, + pretend(server, helper) { + server.get("/tags/planters/notifications", () => { + return helper.response({ + tag_notification: { id: "planters", notification_level: 1 } + }); + }); + + server.get("/tags/planters/l/latest.json", () => { + return helper.response({ + users: [], + primary_groups: [], + topic_list: { + can_create_topic: true, + draft: null, + draft_key: "new_topic", + draft_sequence: 1, + per_page: 30, + tags: [ + { + id: 1, + name: "planters", + topic_count: 1 + } + ], + topics: [] + } + }); + }); + + server.get("/tags/planters/info", () => { + return helper.response({ + tag_info: { + id: 12, + name: "planters", + topic_count: 1, + staff: false, + synonyms: [ + { + id: "containers", + text: "containers" + }, + { + id: "planter", + text: "planter" + } + ], + tag_group_names: ["Gardening"], + category_ids: [7] + }, + categories: [ + { + id: 7, + name: "Outdoors", + color: "000", + text_color: "FFFFFF", + slug: "outdoors", + topic_count: 701, + post_count: 5320, + description: "Talk about the outdoors.", + description_text: "Talk about the outdoors.", + topic_url: "/t/category-definition-for-outdoors/1026", + read_restricted: false, + permission: null, + notification_level: null + } + ] + }); + }); + } +}); + +test("tag info can show synonyms", async assert => { + updateCurrentUser({ moderator: false, admin: false }); + + await visit("/tags/planters"); + assert.ok(find("#show-tag-info").length === 1); + + await click("#show-tag-info"); + assert.ok(exists(".tag-info .tag-name"), "show tag"); + assert.ok( + find(".tag-info .tag-associations") + .text() + .indexOf("Gardening") >= 0, + "show tag group names" + ); + assert.ok( + find(".tag-info .synonyms-list .tag-box").length === 2, + "shows the synonyms" + ); + assert.ok( + find(".tag-info .badge-category").length === 1, + "show the category" + ); + assert.ok(!exists("#rename-tag"), "can't rename tag"); + assert.ok(!exists("#edit-synonyms"), "can't edit synonyms"); + assert.ok(!exists("#delete-tag"), "can't delete tag"); +}); + +test("admin can manage tags", async assert => { + server.delete("/tags/planters/synonyms/containers", () => [ + 200, + { "Content-Type": "application/json" }, + { success: true } + ]); + + updateCurrentUser({ moderator: false, admin: true }); + + await visit("/tags/planters"); + assert.ok(find("#show-tag-info").length === 1); + + await click("#show-tag-info"); + assert.ok(exists("#rename-tag"), "can rename tag"); + assert.ok(exists("#edit-synonyms"), "can edit synonyms"); + assert.ok(exists("#delete-tag"), "can delete tag"); + + await click("#edit-synonyms"); + assert.ok( + find(".unlink-synonym:visible").length === 2, + "unlink UI is visible" + ); + assert.ok( + find(".delete-synonym:visible").length === 2, + "delete UI is visible" + ); + + await click(".unlink-synonym:first"); + assert.ok( + find(".tag-info .synonyms-list .tag-box").length === 1, + "removed a synonym" + ); +}); diff --git a/test/javascripts/components/tag-drop-test.js.es6 b/test/javascripts/components/tag-drop-test.js.es6 index 2cfab99b487..c457ec71f44 100644 --- a/test/javascripts/components/tag-drop-test.js.es6 +++ b/test/javascripts/components/tag-drop-test.js.es6 @@ -25,13 +25,13 @@ componentTest("default", { if (params.queryParams.q === "rég") { return response({ "results": [ - { "id": "régis", "text": "régis", "count": 2, "pm_count": 0 } + { "id": "régis", "text": "régis", "count": 2, "pm_count": 0, target_tag: null } ] }); - }else if (params.queryParams.q === "dav") { + } else if (params.queryParams.q === "dav") { return response({ "results": [ - { "id": "David", "text": "David", "count": 2, "pm_count": 0 } + { "id": "David", "text": "David", "count": 2, "pm_count": 0, target_tag: null } ] }); } @@ -77,6 +77,42 @@ componentTest("default", { } }); +componentTest("synonym", { + template: "{{tag-drop}}", + + beforeEach() { + this.site.set("can_create_tag", true); + this.set("site.top_tags", ["jeff", "neil", "arpit", "régis"]); + + const response = object => { + return [200, { "Content-Type": "application/json" }, object]; + }; + + // prettier-ignore + server.get("/tags/filter/search", (params) => { //eslint-disable-line + if (params.queryParams.q === "robin") { + return response({ + "results": [ + { "id": "Robin", "text": "Robin", "count": 2, "pm_count": 0, target_tag: 'EvilTrout' } + ] + }); + } + }); + }, + + async test(assert) { + await this.subject.expand(); + + sandbox.stub(DiscourseURL, "routeTo"); + await this.subject.fillInFilter("robin"); + await this.subject.keyboard("enter"); + assert.ok( + DiscourseURL.routeTo.calledWith("/tags/eviltrout"), + "it routes to the target tag" + ); + } +}); + componentTest("no tags", { template: "{{tag-drop}}",