From c60e360c900ddc08a68fc91ace5ee03404cfa0d2 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 12 Jan 2016 16:40:36 +0800 Subject: [PATCH] FIX: Clashing category slug. --- .../discourse/components/d-editor.js.es6 | 2 +- .../dialects/category_hashtag_dialect.js | 2 +- .../discourse/models/category.js.es6 | 4 +-- .../category_hashtags_controller.rb | 6 +++-- app/models/category.rb | 5 ++-- app/models/concerns/category_hashtag.rb | 23 ++++++++++++++++ lib/pretty_text.rb | 5 ++-- .../concern/category_hashtag_spec.rb | 26 +++++++++++++++++++ spec/components/pretty_text_spec.rb | 2 +- .../category_hashtags_controller_spec.rb | 4 +-- 10 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 app/models/concerns/category_hashtag.rb create mode 100644 spec/components/concern/category_hashtag_spec.rb diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 631429f37..6cf612370 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -255,7 +255,7 @@ export default Ember.Component.extend({ template: template, key: '#', transformComplete(category) { - return category.get('slug'); + return Category.slugFor(category, ":"); }, dataSource(term) { return Category.search(term); diff --git a/app/assets/javascripts/discourse/dialects/category_hashtag_dialect.js b/app/assets/javascripts/discourse/dialects/category_hashtag_dialect.js index 4beb7ab53..3242e9667 100644 --- a/app/assets/javascripts/discourse/dialects/category_hashtag_dialect.js +++ b/app/assets/javascripts/discourse/dialects/category_hashtag_dialect.js @@ -4,7 +4,7 @@ **/ Discourse.Dialect.inlineRegexp({ start: '#', - matcher: /^#([\w-]{1,50})/i, + matcher: /^#([\w-:]{1,50})/i, spaceOrTagBoundary: true, emitter: function(matches) { diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index e507419c0..91d5c60df 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -205,14 +205,14 @@ Category.reopenClass({ return _uncategorized; }, - slugFor(category) { + slugFor(category, separator = "/") { if (!category) return ""; const parentCategory = Em.get(category, 'parentCategory'); let result = ""; if (parentCategory) { - result = Category.slugFor(parentCategory) + "/"; + result = Category.slugFor(parentCategory) + separator; } const id = Em.get(category, 'id'), diff --git a/app/controllers/category_hashtags_controller.rb b/app/controllers/category_hashtags_controller.rb index 6e78a92ec..ca4a1f700 100644 --- a/app/controllers/category_hashtags_controller.rb +++ b/app/controllers/category_hashtags_controller.rb @@ -5,8 +5,10 @@ class CategoryHashtagsController < ApplicationController category_slugs = params[:category_slugs] category_slugs.each(&:downcase!) - valid_categories = Category.secured(guardian).where(slug: category_slugs).map do |category| - { slug: category.slug, url: category.url_with_id } + 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_with_id } end.compact render json: { valid: valid_categories } diff --git a/app/models/category.rb b/app/models/category.rb index 7ada616b8..961276780 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -5,6 +5,7 @@ class Category < ActiveRecord::Base include Positionable include HasCustomFields + include CategoryHashtag belongs_to :topic, dependent: :destroy belongs_to :topic_only_relative_url, @@ -398,8 +399,8 @@ SQL @@url_cache.clear end - def full_slug - url[3..-1].gsub("/", "-") + def full_slug(separator = "-") + url[3..-1].gsub("/", separator) end def url diff --git a/app/models/concerns/category_hashtag.rb b/app/models/concerns/category_hashtag.rb new file mode 100644 index 000000000..fe97b0252 --- /dev/null +++ b/app/models/concerns/category_hashtag.rb @@ -0,0 +1,23 @@ +module CategoryHashtag + extend ActiveSupport::Concern + + SEPARATOR = ":".freeze + + class_methods do + def query_from_hashtag_slug(category_slug) + parent_slug, child_slug = category_slug.split(":", 2) + + category = Category.where(slug: parent_slug, parent_category_id: nil) + + if child_slug + Category.where(slug: child_slug, parent_category_id: category.pluck(:id).first).first + else + category.first + end + end + end + + def hashtag_slug + full_slug(SEPARATOR) + end +end diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 126f3358c..927f8ecd7 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -49,9 +49,8 @@ module PrettyText end def category_hashtag_lookup(category_slug) - if category_slug - category = Category.find_by_slug(category_slug) - return ['category', category.url_with_id] if category + if category = Category.query_from_hashtag_slug(category_slug) + ['category', category.url_with_id] else nil end diff --git a/spec/components/concern/category_hashtag_spec.rb b/spec/components/concern/category_hashtag_spec.rb new file mode 100644 index 000000000..3fe10ba84 --- /dev/null +++ b/spec/components/concern/category_hashtag_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +describe CategoryHashtag do + describe '#query_from_hashtag_slug' do + let(:parent_category) { Fabricate(:category) } + let(:child_category) { Fabricate(:category, parent_category: parent_category) } + + it "should return the right result for a parent category slug" do + expect(Category.query_from_hashtag_slug(parent_category.slug)) + .to eq(parent_category) + end + + it "should return the right result for a parent and child category slug" do + expect(Category.query_from_hashtag_slug("#{parent_category.slug}:#{child_category.slug}")) + .to eq(child_category) + end + + it "should return nil for incorrect parent category slug" do + expect(Category.query_from_hashtag_slug("random-slug")).to eq(nil) + end + + it "should return nil for incorrect parent and child category slug" do + expect(Category.query_from_hashtag_slug("random-slug:random-slug")).to eq(nil) + end + end +end diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index f9cd29081..dabe28405 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -390,7 +390,7 @@ HTML end it "doesn't replace unicode emoji if emoji is disabled" do - SiteSetting.enable_emoji = false + SiteSetting.enable_emoji = false expect(PrettyText.cook("💣")).not_to match(/\:bomb\:/) end end diff --git a/spec/controllers/category_hashtags_controller_spec.rb b/spec/controllers/category_hashtags_controller_spec.rb index 01d5eeb74..e5f151561 100644 --- a/spec/controllers/category_hashtags_controller_spec.rb +++ b/spec/controllers/category_hashtags_controller_spec.rb @@ -12,7 +12,7 @@ describe CategoryHashtagsController do xhr :get, :check, category_slugs: [category.slug, 'none'] expect(JSON.parse(response.body)).to eq( - { "valid" => [{ "slug" => category.slug, "url" => category.url_with_id }] } + { "valid" => [{ "slug" => category.hashtag_slug, "url" => category.url_with_id }] } ) end @@ -32,7 +32,7 @@ describe CategoryHashtagsController do xhr :get, :check, category_slugs: [private_category.slug] expect(JSON.parse(response.body)).to eq( - { "valid" => [{ "slug" => private_category.slug, "url" => private_category.url_with_id }] } + { "valid" => [{ "slug" => private_category.hashtag_slug, "url" => private_category.url_with_id }] } ) end end