diff --git a/app/models/concerns/category_hashtag.rb b/app/models/concerns/category_hashtag.rb index 135a903aee9..00f4e3e74f3 100644 --- a/app/models/concerns/category_hashtag.rb +++ b/app/models/concerns/category_hashtag.rb @@ -46,18 +46,20 @@ module CategoryHashtag # is no child then the "parent" part of the slug is just the # entire slug we look for. # - # Otherwise if the child slug is present, we find the parent - # by its slug then find the child by its slug and its parent's - # ID to make sure they match. + # Otherwise if the child slug is present, we find the child + # by its slug then find the parent by its slug and the child's + # parent ID to make sure they match. if child_slug.present? - parent_category = categories.find { |cat| cat.slug.casecmp?(parent_slug) } - if parent_category.present? - categories.find do |cat| - cat.slug.downcase == child_slug && cat.parent_category_id == parent_category.id + categories.find do |cat| + if cat.slug.casecmp?(child_slug) && cat.parent_category_id + categories.find do |parent_category| + parent_category.id == cat.parent_category_id && + parent_category.slug.casecmp?(parent_slug) + end end end else - categories.find { |cat| cat.slug.downcase == parent_slug && cat.top_level? } + categories.find { |cat| cat.slug.casecmp?(parent_slug) && cat.top_level? } end end .compact diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index fe8f63caa8e..85403cf5b16 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/app/services/category_hashtag_data_source.rb @@ -32,7 +32,11 @@ class CategoryHashtagDataSource end def self.lookup(guardian, slugs) - user_categories = Category.secured(guardian).includes(:parent_category) + user_categories = + Category + .secured(guardian) + .includes(:parent_category) + .order("parent_category_id ASC NULLS FIRST, id ASC") Category .query_loaded_from_slugs(slugs, user_categories) .map { |category| category_to_hashtag_item(category) } diff --git a/spec/requests/hashtags_controller_spec.rb b/spec/requests/hashtags_controller_spec.rb index 565e2268d14..968465bd0a0 100644 --- a/spec/requests/hashtags_controller_spec.rb +++ b/spec/requests/hashtags_controller_spec.rb @@ -275,6 +275,8 @@ RSpec.describe HashtagsController do qux = Fabricate(:category_with_definition, slug: "qux") quxbar = Fabricate(:category_with_definition, slug: "bar", parent_category_id: qux.id) + quxbarbaz = + Fabricate(:category_with_definition, slug: "baz", parent_category_id: quxbar.id) invalid_slugs = [":"] child_slugs = %w[bar baz] diff --git a/spec/services/category_hashtag_data_source_spec.rb b/spec/services/category_hashtag_data_source_spec.rb index 26098e69f8d..5140855158e 100644 --- a/spec/services/category_hashtag_data_source_spec.rb +++ b/spec/services/category_hashtag_data_source_spec.rb @@ -40,6 +40,47 @@ RSpec.describe CategoryHashtagDataSource do group.add(user) expect(described_class.lookup(Guardian.new(user), ["secret"]).first).not_to eq(nil) end + + context "with sub-sub-categories" do + before { SiteSetting.max_category_nesting = 3 } + + it "returns the first matching grandchild category (ordered by IDs) when there are multiple categories with the same slug" do + parent1 = Fabricate(:category, slug: "parent1") + parent2 = Fabricate(:category, slug: "parent2") + + parent1_child = Fabricate(:category, slug: "child", parent_category_id: parent1.id) + parent1_child_grandchild = + Fabricate(:category, slug: "grandchild", parent_category_id: parent1_child.id) + + parent2_child = Fabricate(:category, slug: "child", parent_category_id: parent2.id) + parent2_child_grandchild = + Fabricate(:category, slug: "grandchild", parent_category_id: parent2_child.id) + + result = described_class.lookup(guardian, ["child:grandchild"]) + expect(result.map(&:relative_url)).to eq([parent1_child_grandchild.url]) + + parent1_child.destroy + parent1_child = Fabricate(:category, slug: "child", parent_category_id: parent1.id) + + result = described_class.lookup(guardian, ["child:grandchild"]) + expect(result.map(&:relative_url)).to eq([parent2_child_grandchild.url]) + end + + it "returns the correct grandchild category when there are multiple children with the same slug and only one of them has the correct grandchild" do + parent1 = Fabricate(:category, slug: "parent1") + parent1_child = Fabricate(:category, slug: "child", parent_category_id: parent1.id) + parent1_child_grandchild = + Fabricate(:category, slug: "another-grandchild", parent_category_id: parent1_child.id) + + parent2 = Fabricate(:category, slug: "parent2") + parent2_child = Fabricate(:category, slug: "child", parent_category_id: parent2.id) + parent2_child_grandchild = + Fabricate(:category, slug: "grandchild", parent_category_id: parent2_child.id) + + result = described_class.lookup(guardian, ["child:grandchild"]) + expect(result.map(&:relative_url)).to eq([parent2_child_grandchild.url]) + end + end end describe "#search" do