From 9bf522f2278f6e6d65d4a25803d1e4b0f15395d4 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 5 Oct 2018 10:23:52 +0100 Subject: [PATCH] FEATURE: Mixed case tagging (#6454) - By default, behaviour is not changed: tags are made lowercase upon creation and edit. - If force_lowercase_tags is disabled, then mixed case tags are allowed. - Tags must remain case-insensitively unique. This is enforced by ActiveRecord and Postgres. - A migration is added to provide a `UNIQUE` index on `lower(name)`. Migration includes a safety to correct any current tags that do not meet the criteria. - A `where_name` scope is added to `models/tag.rb`, to allow easy case-insensitive lookups. This is used instead of `Tag.where(name: "blah")`. - URLs remain lowercase. Mixed case URLs are functional, but have the lowercase equivalent as the canonical. --- .../discourse/lib/render-tag.js.es6 | 5 +++-- .../discourse/routes/tags-show.js.es6 | 13 ++++++++----- .../components/mini-tag-chooser.js.es6 | 1 - .../select-kit/components/tag-drop.js.es6 | 2 +- .../javascripts/select-kit/mixins/tags.js.es6 | 8 ++++++-- app/controllers/tags_controller.rb | 10 ++++------ app/models/tag.rb | 11 ++++++++++- app/models/tag_user.rb | 2 +- app/services/search_indexer.rb | 2 +- config/locales/server.en.yml | 1 + config/site_settings.yml | 2 ++ db/migrate/20180928105835_add_index_to_tags.rb | 17 +++++++++++++++++ lib/discourse_tagging.rb | 18 +++++++++--------- lib/search.rb | 8 ++++---- lib/topic_query.rb | 5 +++-- script/bulk_import/discourse_merger.rb | 2 +- spec/components/discourse_tagging_spec.rb | 13 +++++++++++-- spec/components/search_spec.rb | 9 +++++++-- spec/components/topic_query_spec.rb | 5 +++++ spec/models/tag_spec.rb | 6 ++++++ test/javascripts/acceptance/tags-test.js.es6 | 13 +++++++++++-- .../components/mini-tag-chooser-test.js.es6 | 12 +++++++++++- .../components/tag-drop-test.js.es6 | 15 +++++++++++++++ 23 files changed, 137 insertions(+), 43 deletions(-) create mode 100644 db/migrate/20180928105835_add_index_to_tags.rb diff --git a/app/assets/javascripts/discourse/lib/render-tag.js.es6 b/app/assets/javascripts/discourse/lib/render-tag.js.es6 index 533058adf08..8420638b3b5 100644 --- a/app/assets/javascripts/discourse/lib/render-tag.js.es6 +++ b/app/assets/javascripts/discourse/lib/render-tag.js.es6 @@ -1,6 +1,7 @@ export default function renderTag(tag, params) { params = params || {}; - tag = Handlebars.Utils.escapeExpression(tag); + const visibleName = Handlebars.Utils.escapeExpression(tag); + tag = visibleName.toLowerCase(); const classes = ["discourse-tag"]; const tagName = params.tagName || "a"; let path; @@ -29,7 +30,7 @@ export default function renderTag(tag, params) { " class='" + classes.join(" ") + "'>" + - tag + + visibleName + ""; diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index 437f3e4e821..2bd25671571 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -49,10 +49,12 @@ export default Discourse.Route.extend({ if (tag && tag.get("id") !== "none" && this.get("currentUser")) { // If logged in, we should get the tag's user settings - return this.store.find("tagNotification", tag.get("id")).then(tn => { - this.set("tagNotification", tn); - return tag; - }); + return this.store + .find("tagNotification", tag.get("id").toLowerCase()) + .then(tn => { + this.set("tagNotification", tn); + return tag; + }); } return tag; @@ -67,7 +69,7 @@ export default Discourse.Route.extend({ const categorySlug = this.get("categorySlug"); const parentCategorySlug = this.get("parentCategorySlug"); const filter = this.get("navMode"); - const tag_id = tag ? tag.id : "none"; + const tag_id = tag ? tag.id.toLowerCase() : "none"; if (categorySlug) { var category = Discourse.Category.findBySlug( @@ -100,6 +102,7 @@ export default Discourse.Route.extend({ params, {} ).then(list => { + tag.set("id", list.topic_list.tags[0].name); // Update name of tag (case might be different) controller.setProperties({ list: list, canCreateTopic: list.get("can_create_topic"), diff --git a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 index f44fdb33e22..765aae62dde 100644 --- a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 @@ -225,7 +225,6 @@ export default ComboBox.extend(TagsMixin, { case "string": // See lib/discourse_tagging#clean_tag. return content - .toLowerCase() .trim() .replace(/\s+/, "-") .replace(/[\/\?#\[\]@!\$&'\(\)\*\+,;=\.%\\`^\s|\{\}"<>]+/, "") 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 b1bb9799602..c99806b8481 100644 --- a/app/assets/javascripts/select-kit/components/tag-drop.js.es6 +++ b/app/assets/javascripts/select-kit/components/tag-drop.js.es6 @@ -145,7 +145,7 @@ export default ComboBoxComponent.extend(TagsMixin, { if (this.get("currentCategory")) { url += this.get("currentCategory.url"); } - url = `${url}/${tagId}`; + url = `${url}/${tagId.toLowerCase()}`; DiscourseURL.routeTo(url); }, diff --git a/app/assets/javascripts/select-kit/mixins/tags.js.es6 b/app/assets/javascripts/select-kit/mixins/tags.js.es6 index 70da1641811..0f0bef09ad4 100644 --- a/app/assets/javascripts/select-kit/mixins/tags.js.es6 +++ b/app/assets/javascripts/select-kit/mixins/tags.js.es6 @@ -52,12 +52,16 @@ export default Ember.Mixin.create({ return false; } + const toLowerCaseOrUndefined = string => { + return string === undefined ? undefined : string.toLowerCase(); + }; + const inCollection = this.get("collectionComputedContent") - .map(c => get(c, "id")) + .map(c => toLowerCaseOrUndefined(get(c, "id"))) .includes(term); const inSelection = this.get("selection") - .map(s => get(s, "value").toLowerCase()) + .map(s => toLowerCaseOrUndefined(get(s, "value"))) .includes(term); if (inCollection || inSelection) { return false; diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index fccddd0e4ab..8c7c854e7c6 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -89,7 +89,7 @@ class TagsController < ::ApplicationController path_name = url_method(params.slice(:category, :parent_category)) canonical_url "#{Discourse.base_url_no_prefix}#{public_send(path_name, *(params.slice(:parent_category, :category, :tag_id).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_id] != 'none' && !Tag.where_name(@tag_id).exists? raise Discourse::NotFound.new("tag not found", check_permalinks: true) else respond_with_list(@list) @@ -162,7 +162,7 @@ class TagsController < ::ApplicationController json_response = { results: tags } - if Tag.where(name: params[:q]).exists? && !tags.find { |h| h[:id] == params[:q] } + if Tag.where_name(params[:q]).exists? && !tags.find { |h| h[:id] == params[:q] } # filter_allowed_tags determined that the tag entered is not allowed json_response[:forbidden] = params[:q] end @@ -171,7 +171,7 @@ class TagsController < ::ApplicationController end def notifications - tag = Tag.find_by_name(params[:tag_id]) + tag = Tag.where_name(params[:tag_id]).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 } } @@ -186,9 +186,7 @@ class TagsController < ::ApplicationController end def check_hashtag - tag_values = params[:tag_values].each(&:downcase!) - - valid_tags = Tag.where(name: tag_values).map do |tag| + valid_tags = Tag.where_name(params[:tag_values]).map do |tag| { value: tag.name, url: tag.full_url } end.compact diff --git a/app/models/tag.rb b/app/models/tag.rb index 11002e25417..f2ca7f067e3 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -2,7 +2,12 @@ class Tag < ActiveRecord::Base include Searchable include HasDestroyedWebHook - validates :name, presence: true, uniqueness: true + validates :name, presence: true, uniqueness: { case_sensitive: false } + + scope :where_name, ->(name) do + name = Array(name).map(&:downcase) + where("lower(name) IN (?)", name) + end has_many :tag_users # notification settings @@ -59,6 +64,10 @@ class Tag < ActiveRecord::Base SQL end + def self.find_by_name(name) + self.find_by('lower(name) = ?', name.downcase) + end + def self.top_tags(limit_arg: nil, category: nil, guardian: nil) limit = limit_arg || SiteSetting.max_tags_in_filter_list scope_category_ids = (guardian || Guardian.new).allowed_category_ids diff --git a/app/models/tag_user.rb b/app/models/tag_user.rb index 87eafe5404d..bee895bfe0e 100644 --- a/app/models/tag_user.rb +++ b/app/models/tag_user.rb @@ -19,7 +19,7 @@ class TagUser < ActiveRecord::Base records = TagUser.where(user: user, notification_level: notification_levels[level]) old_ids = records.pluck(:tag_id) - tag_ids = tags.empty? ? [] : Tag.where('name in (?)', tags).pluck(:id) + tag_ids = tags.empty? ? [] : Tag.where_name(tags).pluck(:id) remove = (old_ids - tag_ids) if remove.present? diff --git a/app/services/search_indexer.rb b/app/services/search_indexer.rb index 72240873685..e31bf8e4914 100644 --- a/app/services/search_indexer.rb +++ b/app/services/search_indexer.rb @@ -105,7 +105,7 @@ class SearchIndexer end def self.update_tags_index(tag_id, name) - update_index(table: 'tag', id: tag_id, raw_data: [name]) + update_index(table: 'tag', id: tag_id, raw_data: [name.downcase]) end def self.queue_post_reindex(topic_id) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1ea6b56ab1b..e5236174f00 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1798,6 +1798,7 @@ en: min_trust_level_to_tag_topics: "Minimum trust level required to tag topics" suppress_overlapping_tags_in_list: "If tags match exact words in topic titles, don't show the tag" remove_muted_tags_from_latest: "Don't show topics tagged with muted tags in the latest topic list." + force_lowercase_tags: "Force all new tags to be entirely lowercase." company_short_name: "Company Name (short)" company_full_name: "Company Name (full)" diff --git a/config/site_settings.yml b/config/site_settings.yml index f82095adb28..6b932a524c6 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1788,3 +1788,5 @@ tags: client: true remove_muted_tags_from_latest: default: false + force_lowercase_tags: + default: true \ No newline at end of file diff --git a/db/migrate/20180928105835_add_index_to_tags.rb b/db/migrate/20180928105835_add_index_to_tags.rb new file mode 100644 index 00000000000..50a45870668 --- /dev/null +++ b/db/migrate/20180928105835_add_index_to_tags.rb @@ -0,0 +1,17 @@ +class AddIndexToTags < ActiveRecord::Migration[5.2] + def up + # Append ID to any tags that already have duplicate names + # Super rare case, as this is not possible to do via the UI + # Might affect some imports + execute <<~SQL + UPDATE tags + SET name = name || id + WHERE EXISTS(SELECT * FROM tags t WHERE lower(t.name) = lower(tags.name) AND t.id < tags.id) + SQL + + add_index :tags, 'lower(name)', unique: true + end + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 030927df29e..2c0519cd199 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -39,7 +39,7 @@ module DiscourseTagging # guardian is explicitly nil cause we don't want to strip all # staff tags that already passed validation tags = filter_allowed_tags( - Tag.where(name: tag_names), + Tag.where_name(tag_names), nil, # guardian for_topic: true, category: category, @@ -48,7 +48,7 @@ module DiscourseTagging if tags.size < tag_names.size && (category.nil? || (category.tags.count == 0 && category.tag_groups.count == 0)) tag_names.each do |name| - unless Tag.where(name: name).exists? + unless Tag.where_name(name).exists? tags << Tag.create(name: name) end end @@ -82,8 +82,7 @@ module DiscourseTagging # for_topic: results are for tagging a topic # selected_tags: an array of tag names that are in the current selection def self.filter_allowed_tags(query, guardian, opts = {}) - - selected_tag_ids = opts[:selected_tags] ? Tag.where(name: opts[:selected_tags]).pluck(:id) : [] + selected_tag_ids = opts[:selected_tags] ? Tag.where_name(opts[:selected_tags]).pluck(:id) : [] if !opts[:for_topic] && !selected_tag_ids.empty? query = query.where('tags.id NOT IN (?)', selected_tag_ids) @@ -92,8 +91,8 @@ module DiscourseTagging term = opts[:term] if term.present? term.gsub!("_", "\\_") - term = clean_tag(term) - query = query.where('tags.name like ?', "%#{term}%") + term = clean_tag(term).downcase + query = query.where('lower(tags.name) like ?', "%#{term}%") end # Filters for category-specific tags: @@ -203,7 +202,8 @@ module DiscourseTagging end def self.clean_tag(tag) - tag.downcase.strip + tag.downcase! if SiteSetting.force_lowercase_tags + tag.strip .gsub(/\s+/, '-').squeeze('-') .gsub(TAGS_FILTER_REGEXP, '')[0...SiteSetting.max_tag_length] end @@ -212,7 +212,7 @@ module DiscourseTagging return [] unless guardian.can_tag_topics? && tags_arg.present? - tag_names = Tag.where(name: tags_arg).pluck(:name) + tag_names = Tag.where_name(tags_arg).pluck(:name) if guardian.can_create_tag? tag_names += (tags_arg - tag_names).map { |t| clean_tag(t) } @@ -226,7 +226,7 @@ module DiscourseTagging def self.add_or_create_tags_by_name(taggable, tag_names_arg, opts = {}) 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 + 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| diff --git a/lib/search.rb b/lib/search.rb index 80c6df50532..d61833e9996 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -407,7 +407,7 @@ class Search posts.where("topics.category_id IN (?)", category_ids) else # try a possible tag match - tag_id = Tag.where(name: slug[0]).pluck(:id).first + tag_id = Tag.where_name(slug[0]).pluck(:id).first if (tag_id) posts.where("topics.id IN ( SELECT DISTINCT(tt.topic_id) @@ -496,7 +496,7 @@ class Search def search_tags(posts, match, positive:) return if match.nil? - + match.downcase! modifier = positive ? "" : "NOT" if match.include?('+') @@ -507,7 +507,7 @@ class Search FROM topic_tags tt, tags WHERE tt.tag_id = tags.id GROUP BY tt.topic_id - HAVING to_tsvector(#{default_ts_config}, array_to_string(array_agg(tags.name), ' ')) @@ to_tsquery(#{default_ts_config}, ?) + HAVING to_tsvector(#{default_ts_config}, array_to_string(array_agg(lower(tags.name)), ' ')) @@ to_tsquery(#{default_ts_config}, ?) )", tags.join('&')) else tags = match.split(",") @@ -515,7 +515,7 @@ class Search posts.where("topics.id #{modifier} IN ( SELECT DISTINCT(tt.topic_id) FROM topic_tags tt, tags - WHERE tt.tag_id = tags.id AND tags.name IN (?) + WHERE tt.tag_id = tags.id AND lower(tags.name) IN (?) )", tags) end end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 202d3444704..6e299e73ba6 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -634,11 +634,12 @@ class TopicQuery result = result.preload(:tags) if @options[:tags] && @options[:tags].size > 0 + @options[:tags].each { |t| t.downcase! if t.is_a? String } if @options[:match_all_tags] # ALL of the given tags: tags_count = @options[:tags].length - @options[:tags] = Tag.where(name: @options[:tags]).pluck(:id) unless @options[:tags][0].is_a?(Integer) + @options[:tags] = Tag.where_name(@options[:tags]).pluck(:id) unless @options[:tags][0].is_a?(Integer) if tags_count == @options[:tags].length @options[:tags].each_with_index do |tag, index| @@ -654,7 +655,7 @@ class TopicQuery if @options[:tags][0].is_a?(Integer) result = result.where("tags.id in (?)", @options[:tags]) else - result = result.where("tags.name in (?)", @options[:tags]) + result = result.where("lower(tags.name) in (?)", @options[:tags]) end end elsif @options[:no_tags] diff --git a/script/bulk_import/discourse_merger.rb b/script/bulk_import/discourse_merger.rb index 3d0f4131c7e..2a44a343fa5 100644 --- a/script/bulk_import/discourse_merger.rb +++ b/script/bulk_import/discourse_merger.rb @@ -271,7 +271,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base @raw_connection.copy_data(sql, @encoder) do source_raw_connection.exec("SELECT #{columns.map { |c| "\"#{c}\"" }.join(', ')} FROM tags").each do |row| - if existing = Tag.where(name: row['name']).first + if existing = Tag.where_name(row['name']).first @tags[row['id']] = existing.id next end diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb index c378699e893..5d52375406b 100644 --- a/spec/components/discourse_tagging_spec.rb +++ b/spec/components/discourse_tagging_spec.rb @@ -13,7 +13,7 @@ describe DiscourseTagging do let!(:tag1) { Fabricate(:tag, name: "fun") } let!(:tag2) { Fabricate(:tag, name: "fun2") } - let!(:tag3) { Fabricate(:tag, name: "fun3") } + let!(:tag3) { Fabricate(:tag, name: "Fun3") } before do SiteSetting.tagging_enabled = true @@ -186,7 +186,8 @@ describe DiscourseTagging do it "returns only existing tag names" do Fabricate(:tag, name: 'oldtag') - expect(described_class.tags_for_saving(['newtag', 'oldtag'], guardian).try(:sort)).to eq(['oldtag']) + Fabricate(:tag, name: 'oldTag2') + expect(described_class.tags_for_saving(['newtag', 'oldtag', 'oldtag2'], guardian)).to contain_exactly('oldtag', 'oldTag2') end end @@ -205,5 +206,13 @@ describe DiscourseTagging do expect(described_class.tags_for_saving(['math=fun', 'fun*2@gmail.com'], guardian).try(:sort)).to eq(['math=fun', 'fun2gmailcom'].sort) end end + + describe "clean_tag" do + it "downcases new tags if setting enabled" do + expect(DiscourseTagging.clean_tag("HeLlO")).to eq("hello") + SiteSetting.force_lowercase_tags = false + expect(DiscourseTagging.clean_tag("HeLlO")).to eq("HeLlO") + end + end end end diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 912415a4a12..d987571be33 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -407,6 +407,7 @@ describe Search do end let!(:tag) { Fabricate(:tag) } + let!(:uppercase_tag) { Fabricate(:tag, name: "HeLlO") } let(:tag_group) { Fabricate(:tag_group) } let(:category) { Fabricate(:category) } @@ -415,7 +416,7 @@ describe Search 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]) + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(Fabricate.build(:admin)), [tag.name, uppercase_tag.name]) post.topic.save # we got to make this index (it is deferred) @@ -424,6 +425,9 @@ describe Search do result = Search.execute(tag.name) expect(result.posts.length).to eq(1) + result = Search.execute("hElLo") + expect(result.posts.length).to eq(1) + SiteSetting.tagging_enabled = false result = Search.execute(tag.name) @@ -822,9 +826,10 @@ describe Search do expect(Search.execute("sams post #sub-category").posts.length).to eq(1) # tags - topic.tags = [Fabricate(:tag, name: 'alpha'), Fabricate(:tag, name: 'привет')] + topic.tags = [Fabricate(:tag, name: 'alpha'), Fabricate(:tag, name: 'привет'), Fabricate(:tag, name: 'HeLlO')] expect(Search.execute('this is a test #alpha').posts.map(&:id)).to eq([post.id]) expect(Search.execute('this is a test #привет').posts.map(&:id)).to eq([post.id]) + expect(Search.execute('this is a test #hElLo').posts.map(&:id)).to eq([post.id]) expect(Search.execute('this is a test #beta').posts.size).to eq(0) end diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 08515a3e9da..7eac899b2ba 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -160,6 +160,7 @@ describe TopicQuery do context 'tag filter' do let(:tag) { Fabricate(:tag) } let(:other_tag) { Fabricate(:tag) } + let(:uppercase_tag) { Fabricate(:tag, name: "HeLlO") } before do SiteSetting.tagging_enabled = true @@ -169,6 +170,7 @@ describe TopicQuery do let!(:tagged_topic1) { Fabricate(:topic, tags: [tag]) } let!(:tagged_topic2) { Fabricate(:topic, tags: [other_tag]) } let!(:tagged_topic3) { Fabricate(:topic, tags: [tag, other_tag]) } + let!(:tagged_topic4) { Fabricate(:topic, tags: [uppercase_tag]) } let!(:no_tags_topic) { Fabricate(:topic) } it "returns topics with the tag when filtered to it" do @@ -186,6 +188,9 @@ describe TopicQuery do expect(TopicQuery.new(moderator, tags: [tag.id, other_tag.id]).list_latest.topics) .to contain_exactly(tagged_topic1, tagged_topic2, tagged_topic3) + + expect(TopicQuery.new(moderator, tags: ["hElLo"]).list_latest.topics) + .to contain_exactly(tagged_topic4) end it "can return topics with all specified tags" do diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 69dd546e8da..0fcb28e42a9 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -27,6 +27,12 @@ describe Tag do expect(event[:event_name]).to eq(:tag_created) expect(event[:params].first).to eq(subject) end + + it 'prevents case-insensitive duplicates' do + Fabricate.build(:tag, name: "hello").save! + expect { Fabricate.build(:tag, name: "hElLo").save! }.to raise_error(ActiveRecord::RecordInvalid) + end + end describe 'destroy' do diff --git a/test/javascripts/acceptance/tags-test.js.es6 b/test/javascripts/acceptance/tags-test.js.es6 index ced627cce31..b32fabca8aa 100644 --- a/test/javascripts/acceptance/tags-test.js.es6 +++ b/test/javascripts/acceptance/tags-test.js.es6 @@ -32,7 +32,7 @@ QUnit.test("list the tags in groups", async assert => { id: 2, name: "Ford Cars", tags: [ - { id: "escort", text: "escort", count: 1, pm_count: 0 }, + { id: "Escort", text: "Escort", count: 1, pm_count: 0 }, { id: "focus", text: "focus", count: 3, pm_count: 0 } ] }, @@ -79,7 +79,16 @@ QUnit.test("list the tags in groups", async assert => { .map(i => { return $(i).text(); }), - ["focus", "escort"], + ["focus", "Escort"], "shows the tags in default sort (by count)" ); + assert.deepEqual( + $(".tag-list:first .discourse-tag") + .toArray() + .map(i => { + return $(i).attr("href"); + }), + ["/tags/focus", "/tags/escort"], + "always uses lowercase URLs for mixed case tags" + ); }); diff --git a/test/javascripts/components/mini-tag-chooser-test.js.es6 b/test/javascripts/components/mini-tag-chooser-test.js.es6 index 69eae80f9ef..d08379be6c1 100644 --- a/test/javascripts/components/mini-tag-chooser-test.js.es6 +++ b/test/javascripts/components/mini-tag-chooser-test.js.es6 @@ -28,7 +28,7 @@ componentTest("default", { }); } - if (params.queryParams.q === "joffrey" || params.queryParams.q === "invalid'tag" || params.queryParams.q === "01234567890123456789012345") { + if (params.queryParams.q.toLowerCase() === "joffrey" || params.queryParams.q === "invalid'tag" || params.queryParams.q === "01234567890123456789012345") { return response({results: []}); } @@ -74,6 +74,16 @@ componentTest("default", { "it creates the tag" ); + await this.get("subject").expand(); + await this.get("subject").fillInFilter("Joffrey"); + await this.get("subject").keyboard("enter"); + await this.get("subject").collapse(); + assert.deepEqual( + this.get("tags"), + ["jeff", "neil", "arpit", "régis", "joffrey"], + "it does not allow case insensitive duplicate tags" + ); + await this.get("subject").expand(); await this.get("subject").fillInFilter("invalid'tag"); await this.get("subject").keyboard("enter"); diff --git a/test/javascripts/components/tag-drop-test.js.es6 b/test/javascripts/components/tag-drop-test.js.es6 index e3e8926a1f6..5859f845c73 100644 --- a/test/javascripts/components/tag-drop-test.js.es6 +++ b/test/javascripts/components/tag-drop-test.js.es6 @@ -1,4 +1,5 @@ import componentTest from "helpers/component-test"; +import DiscourseURL from "discourse/lib/url"; moduleForComponent("tag-drop", { integration: true, @@ -26,6 +27,12 @@ componentTest("default", { { "id": "régis", "name": "régis", "count": 2, "pm_count": 0 } ] }); + }else if (params.queryParams.q === "dav") { + return response({ + "results": [ + { "id": "David", "name": "David", "count": 2, "pm_count": 0 } + ] + }); } }); }, @@ -66,5 +73,13 @@ componentTest("default", { "jeff", "it returns top tags for an empty search" ); + + sandbox.stub(DiscourseURL, "routeTo"); + await this.get("subject").fillInFilter("dav"); + await this.get("subject").keyboard("enter"); + assert.ok( + DiscourseURL.routeTo.calledWith("/tags/david"), + "it uses lowercase URLs for tags" + ); } });