this is a category hashtag #bug
this is a tag hashtag #monkey
-` + +` ); }); }); diff --git a/app/models/category.rb b/app/models/category.rb index 09cf67d47f..7085aa3084 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -343,8 +343,7 @@ class Category < ActiveRecord::Base if slug.present? # if we don't unescape it first we strip the % from the encoded version slug = SiteSetting.slug_generation_method == 'encoded' ? CGI.unescape(self.slug) : self.slug - # sanitize the custom slug - self.slug = Slug.sanitize(slug) + self.slug = Slug.for(slug, '', method: :encoded) if self.slug.blank? errors.add(:slug, :invalid) @@ -795,8 +794,12 @@ class Category < ActiveRecord::Base return nil if slug_path.empty? return nil if slug_path.size > SiteSetting.max_category_nesting - if SiteSetting.slug_generation_method == "encoded" - slug_path.map! { |slug| CGI.escape(slug) } + slug_path.map! do |slug| + if SiteSetting.slug_generation_method == "encoded" + CGI.escape(slug.downcase) + else + slug.downcase + end end query = diff --git a/db/migrate/20201117212328_set_category_slug_to_lower.rb b/db/migrate/20201117212328_set_category_slug_to_lower.rb new file mode 100644 index 0000000000..8d7d275645 --- /dev/null +++ b/db/migrate/20201117212328_set_category_slug_to_lower.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +class SetCategorySlugToLower < ActiveRecord::Migration[6.0] + def up + remove_index(:categories, name: 'unique_index_categories_on_slug') + + categories = DB.query("SELECT id, name, slug, parent_category_id FROM categories") + old_slugs = categories.map { |c| [c.id, c.slug] }.to_h + updates = {} + + # Resolve duplicate tags by replacing mixed case slugs with new ones + # extracted from category names + slugs = categories + .filter { |category| category.slug.present? } + .group_by { |category| [category.parent_category_id, category.slug.downcase] } + .map { |slug, cats| [slug, cats.size] } + .to_h + + categories.each do |category| + old_parent_and_slug = [category.parent_category_id, category.slug.downcase] + next if category.slug.blank? || + category.slug == category.slug.downcase || + slugs[old_parent_and_slug] <= 1 + + new_slug = category.name.parameterize.tr("_", "-").squeeze('-').gsub(/\A-+|-+\z/, '')[0..255] + new_slug = '' if (new_slug =~ /[^\d]/).blank? + new_parent_and_slug = [category.parent_category_id, new_slug] + next if new_slug.blank? || + (slugs[new_parent_and_slug].present? && slugs[new_parent_and_slug] > 0) + + updates[category.id] = category.slug = new_slug + slugs[old_parent_and_slug] -= 1 + slugs[new_parent_and_slug] = 1 + end + + # Reset left conflicting slugs + slugs = categories + .filter { |category| category.slug.present? } + .group_by { |category| [category.parent_category_id, category.slug.downcase] } + .map { |slug, cats| [slug, cats.size] } + .to_h + + categories.each do |category| + old_parent_and_slug = [category.parent_category_id, category.slug.downcase] + next if category.slug.blank? || + category.slug == category.slug.downcase || + slugs[old_parent_and_slug] <= 1 + + updates[category.id] = category.slug = '' + slugs[old_parent_and_slug] -= 1 + end + + # Update all category slugs + updates.each do |id, slug| + execute <<~SQL + UPDATE categories + SET slug = '#{PG::Connection.escape_string(slug)}' + WHERE id = #{id} -- #{PG::Connection.escape_string(old_slugs[id])} + SQL + end + + # Ensure all slugs are lowercase + execute "UPDATE categories SET slug = LOWER(slug)" + + add_index( + :categories, + 'COALESCE(parent_category_id, -1), LOWER(slug)', + name: 'unique_index_categories_on_slug', + where: "slug != ''", + unique: true + ) + end + + def down + remove_index(:categories, name: 'unique_index_categories_on_slug') + + add_index( + :categories, + 'COALESCE(parent_category_id, -1), slug', + name: 'unique_index_categories_on_slug', + where: "slug != ''", + unique: true + ) + end +end diff --git a/lib/slug.rb b/lib/slug.rb index 9f846cddb7..ebb177347d 100644 --- a/lib/slug.rb +++ b/lib/slug.rb @@ -6,28 +6,22 @@ module Slug CHAR_FILTER_REGEXP = /[:\/\?#\[\]@!\$&'\(\)\*\+,;=_\.~%\\`^\s|\{\}"<>]+/ # :/?#[]@!$&'()*+,;=_.~%\`^|{}"<> MAX_LENGTH = 255 - def self.for(string, default = 'topic', max_length = MAX_LENGTH) + def self.for(string, default = 'topic', max_length = MAX_LENGTH, method: nil) string = string.gsub(/:([\w\-+]+(?::t\d)?):/, '') if string.present? # strip emoji strings - - if SiteSetting.slug_generation_method == 'encoded' - max_length = 9999 # do not truncate encoded slugs - end + method = (method || SiteSetting.slug_generation_method || :ascii).to_sym + max_length = 9999 if method == :encoded # do not truncate encoded slugs slug = - case (SiteSetting.slug_generation_method || :ascii).to_sym + case method when :ascii then self.ascii_generator(string) when :encoded then self.encoded_generator(string) when :none then self.none_generator(string) end + slug = self.prettify_slug(slug, max_length: max_length) (slug.blank? || slug_is_only_numbers?(slug)) ? default : slug end - def self.sanitize(string, downcase: false, max_length: MAX_LENGTH) - slug = self.encoded_generator(string, downcase: downcase) - self.prettify_slug(slug, max_length: max_length) - end - private def self.slug_is_only_numbers?(slug) diff --git a/spec/components/concern/category_hashtag_spec.rb b/spec/components/concern/category_hashtag_spec.rb index 81f437efae..c89dbb575b 100644 --- a/spec/components/concern/category_hashtag_spec.rb +++ b/spec/components/concern/category_hashtag_spec.rb @@ -29,14 +29,6 @@ describe CategoryHashtag do expect(Category.query_from_hashtag_slug("non-existent#{CategoryHashtag::SEPARATOR}#{parent_category.slug}")).to eq(nil) end - it "should be case sensitive" do - parent_category.update!(slug: "ApPlE") - child_category.update!(slug: "OraNGE") - - expect(Category.query_from_hashtag_slug("apple")).to eq(nil) - expect(Category.query_from_hashtag_slug("apple#{CategoryHashtag::SEPARATOR}orange")).to eq(nil) - end - context "multi-level categories" do before do SiteSetting.max_category_nesting = 3 diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 29c8d554bb..2454364d76 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -32,6 +32,13 @@ describe Category do expect(cats.errors[:name]).to be_present end + describe "slug" do + it "converts to lower" do + category = Category.create!(name: "Hello World", slug: "Hello-World", user: user) + expect(category.slug).to eq("hello-world") + end + end + describe "resolve_permissions" do it "can determine read_restricted" do read_restricted, resolved = Category.resolve_permissions(everyone: :full)