FIX: Improve category hashtag lookup (#10133)

* FIX: Improve category hashtag lookup

This commit improves support for sub-sub-categories and does not include
the ID of the category in the slug, which fixes the composer preview.

* FIX: Sub-sub-categories can be mentioned using only two levels

* FIX: Remove support for three-level hashtags

* DEV: Simplify code

app/controllers/category_hashtags_controller.rb

@@ -8,10 +8,15 @@ class CategoryHashtagsController < ApplicationController
 
     ids = category_slugs.map { |category_slug| Category.query_from_hashtag_slug(category_slug).try(:id) }
 
-    valid_categories = Category.secured(guardian).where(id: ids).map do |category|
-      { slug: category.hashtag_slug, url: category.url }
-    end.compact
+    slugs_and_urls = {}
 
-    render json: { valid: valid_categories }
+    Category.secured(guardian).where(id: ids).each do |category|
+      slugs_and_urls[category.slug] ||= category.url
+      slugs_and_urls[category.slug_path.last(2).join(':')] ||= category.url
+    end
+
+    render json: {
+      valid: slugs_and_urls.map { |slug, url| { slug: slug, url: url } }
+    }
   end
 end

app/models/concerns/category_hashtag.rb

@@ -7,10 +7,15 @@ module CategoryHashtag
 
   class_methods do
     def query_from_hashtag_slug(category_slug)
-      parent_slug, child_slug = category_slug.split(SEPARATOR, 2)
+      slug_path = category_slug.split(SEPARATOR)
+      return nil if slug_path.empty? || slug_path.size > 2
 
+      if SiteSetting.slug_generation_method == "encoded"
+        slug_path.map! { |slug| CGI.escape(slug) }
+      end
+
+      parent_slug, child_slug = slug_path.last(2)
       categories = Category.where(slug: parent_slug)
-
       if child_slug
         Category.where(slug: child_slug, parent_category_id: categories.select(:id)).first
       else
@@ -18,8 +23,4 @@ module CategoryHashtag
       end
     end
   end
-
-  def hashtag_slug
-    full_slug.split("-").last(2).join(SEPARATOR)
-  end
 end

spec/requests/category_hashtags_controller_spec.rb

@@ -1,31 +1,30 @@
 # frozen_string_literal: true
 
-require 'rails_helper'
+require "rails_helper"
 
 describe CategoryHashtagsController do
-  describe "check" do
-    describe "logged in" do
+  fab!(:category) { Fabricate(:category) }
 
-      describe "regular user" do
+  let(:group) { Fabricate(:group) }
+  let(:private_category) { Fabricate(:private_category, group: group) }
+
+  describe "#check" do
+    context "when logged in" do
+      context "as regular user" do
         before do
           sign_in(Fabricate(:user))
         end
 
-        it 'only returns the categories that are valid' do
-          category = Fabricate(:category)
-
-          get "/category_hashtags/check.json", params: { category_slugs: [category.slug, 'none'] }
+        it "returns only valid categories" do
+          get "/category_hashtags/check.json", params: { category_slugs: [category.slug, private_category.slug, "none"] }
 
           expect(response.status).to eq(200)
           expect(response.parsed_body).to eq(
-            "valid" => [{ "slug" => category.hashtag_slug, "url" => category.url }]
+            "valid" => [{ "slug" => category.slug, "url" => category.url }]
           )
         end
 
-        it 'does not return restricted categories for a normal user' do
-          group = Fabricate(:group)
-          private_category = Fabricate(:private_category, group: group)
-
+        it "does not return restricted categories" do
           get "/category_hashtags/check.json", params: { category_slugs: [private_category.slug] }
 
           expect(response.status).to eq(200)
@@ -33,26 +32,71 @@ describe CategoryHashtagsController do
         end
       end
 
-      describe "admin user" do
-        it 'returns restricted categories for an admin' do
-          admin = sign_in(Fabricate(:admin))
-          group = Fabricate(:group)
-          group.add(admin)
-          private_category = Fabricate(:private_category, group: group)
+      context "as admin" do
+        fab!(:admin) { Fabricate(:admin) }
 
-          get "/category_hashtags/check.json",
-            params: { category_slugs: [private_category.slug] }
+        before do
+          sign_in(admin)
+        end
+
+        it "returns restricted categories" do
+          group.add(admin)
+
+          get "/category_hashtags/check.json", params: { category_slugs: [private_category.slug] }
 
           expect(response.status).to eq(200)
           expect(response.parsed_body).to eq(
-            "valid" => [{ "slug" => private_category.hashtag_slug, "url" => private_category.url }]
+            "valid" => [{ "slug" => private_category.slug, "url" => private_category.url }]
+          )
+        end
+      end
+
+      context "with sub-sub-categories" do
+        before do
+          SiteSetting.max_category_nesting = 3
+          sign_in(Fabricate(:user))
+        end
+
+        it "works" do
+          foo = Fabricate(:category_with_definition, slug: "foo")
+          foobar = Fabricate(:category_with_definition, slug: "bar", parent_category_id: foo.id)
+          foobarbaz = Fabricate(:category_with_definition, slug: "baz", parent_category_id: foobar.id)
+
+          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)
+
+          get "/category_hashtags/check.json", params: {
+            category_slugs: [
+              ":",
+              "foo",
+              "bar",
+              "baz",
+              "foo:bar",
+              "bar:baz",
+              "foo:bar:baz", # should not work
+              "qux",
+              "qux:bar",
+              "qux:bar:baz" # should not work
+            ]
+          }
+
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["valid"]).to contain_exactly(
+            { "slug" => "foo",     "url" => foo.url },
+            { "slug" => "bar",     "url" => foobar.url },
+            { "slug" => "foo:bar", "url" => foobar.url },
+            { "slug" => "baz",     "url" => foobarbaz.url },
+            { "slug" => "bar:baz", "url" => foobarbaz.url },
+            { "slug" => "qux",     "url" => qux.url },
+            { "slug" => "qux:bar", "url" => quxbar.url }
           )
         end
       end
     end
 
-    describe "not logged in" do
-      it 'raises an exception' do
+    context "when not logged in" do
+      it "returns invalid access" do
         get "/category_hashtags/check.json", params: { category_slugs: [] }
         expect(response.status).to eq(403)
       end