diff --git a/app/controllers/category_hashtags_controller.rb b/app/controllers/category_hashtags_controller.rb index b5a971fa805..f0638510cbd 100644 --- a/app/controllers/category_hashtags_controller.rb +++ b/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 diff --git a/app/models/concerns/category_hashtag.rb b/app/models/concerns/category_hashtag.rb index c6745b01166..216c87a7f67 100644 --- a/app/models/concerns/category_hashtag.rb +++ b/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 diff --git a/spec/requests/category_hashtags_controller_spec.rb b/spec/requests/category_hashtags_controller_spec.rb index 7676ba56423..f4135ae1ec5 100644 --- a/spec/requests/category_hashtags_controller_spec.rb +++ b/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