From b772ff6e13101315e15e1b235dee6346bda49fee Mon Sep 17 00:00:00 2001 From: Erick Guan Date: Mon, 13 Apr 2015 22:50:41 +0800 Subject: [PATCH] FEATURE: add slug geneartion options --- Gemfile | 2 - Gemfile.lock | 2 - .../javascripts/discourse/models/category.js | 6 +- app/controllers/topics_controller.rb | 3 +- app/models/category.rb | 12 +- app/models/slug_setting.rb | 17 ++ app/models/topic.rb | 4 +- app/serializers/notification_serializer.rb | 2 +- app/serializers/user_action_serializer.rb | 2 +- config/locales/server.en.yml | 2 + config/site_settings.yml | 4 + .../20140120155706_add_lounge_category.rb | 3 +- .../20140122043508_add_meta_category.rb | 3 +- .../20140227201005_add_staff_category.rb | 3 +- lib/onebox/engine/discourse_local_onebox.rb | 2 +- lib/slug.rb | 41 ++-- lib/tasks/build_test_topic.rake | 2 +- spec/components/slug_spec.rb | 175 +++++++++++++----- spec/models/category_spec.rb | 80 +++++--- spec/models/topic_spec.rb | 64 +++++-- test/javascripts/models/category-test.js.es6 | 42 ++++- 21 files changed, 349 insertions(+), 122 deletions(-) create mode 100644 app/models/slug_setting.rb diff --git a/Gemfile b/Gemfile index cdc98d5e041..b6d77cf94ac 100644 --- a/Gemfile +++ b/Gemfile @@ -185,8 +185,6 @@ end gem 'rmmseg-cpp', require: false -gem 'stringex', require: false - gem 'logster' # perftools only works on 1.9 atm diff --git a/Gemfile.lock b/Gemfile.lock index a68db9f9faf..f012589f523 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -423,7 +423,6 @@ GEM activesupport (>= 3.0) sprockets (~> 2.8) stackprof (0.2.7) - stringex (2.5.2) therubyracer (0.12.2) libv8 (~> 3.16.14.0) ref @@ -544,7 +543,6 @@ DEPENDENCIES sinatra spork-rails stackprof - stringex therubyracer thin timecop diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index d01e93110dc..2a731e30cb0 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -224,7 +224,7 @@ Discourse.Category.reopenClass({ findSingleBySlug: function(slug) { return Discourse.Category.list().find(function(c) { - return Discourse.Category.slugFor(c) === slug; + return Discourse.Category.slugFor(c) === slug || Discourse.Category.slugFor(c) === encodeURI(slug); }); }, @@ -254,7 +254,9 @@ Discourse.Category.reopenClass({ if (slug === 'none') { return parentCategory; } category = categories.find(function(item) { - return item && item.get('parentCategory') === parentCategory && Discourse.Category.slugFor(item) === (parentSlug + "/" + slug); + return item && item.get('parentCategory') === parentCategory && + (Discourse.Category.slugFor(item) === (parentSlug + "/" + slug) || + Discourse.Category.slugFor(item) === encodeURIComponent(parentSlug) + "/" + encodeURIComponent(slug)); }); } } else { diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 523813c108e..358fdc5c529 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -427,7 +427,8 @@ class TopicsController < ApplicationController end def slugs_do_not_match - params[:slug] && @topic_view.topic.slug != params[:slug] + params[:slug] && (@topic_view.topic.slug != params[:slug] && + @topic_view.topic.slug != Rack::Utils.escape_path(params[:slug])) # maybe percent encoded end def redirect_to_correct_topic(topic, post_number=nil) diff --git a/app/models/category.rb b/app/models/category.rb index bfaf2f001d6..ea895cb025f 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -207,14 +207,18 @@ SQL if slug.present? # santized custom slug - self.slug = Slug.for(slug) + self.slug = Slug.for(Rack::Utils.unescape(slug), '') errors.add(:slug, 'is already in use') if duplicate_slug? else # auto slug - self.slug = Slug.for(name) - return if self.slug.blank? + self.slug = Slug.for(name, '') self.slug = '' if duplicate_slug? end + # only allow to use category itself id. new_record doesn't have a id. + unless new_record? + match_id = /(\d+)-category/.match(self.slug) + errors.add(:slug, :invalid) if match_id && match_id[1] && match_id[1] != self.id.to_s + end end def slug_for_url @@ -338,11 +342,13 @@ SQL def self.query_parent_category(parent_slug) self.where(slug: parent_slug, parent_category_id: nil).pluck(:id).first || + self.where(slug: Rack::Utils.escape_path(parent_slug), parent_category_id: nil).pluck(:id).first || self.where(id: parent_slug.to_i).pluck(:id).first end def self.query_category(slug_or_id, parent_category_id) self.where(slug: slug_or_id, parent_category_id: parent_category_id).includes(:featured_users).first || + self.where(slug: Rack::Utils.escape_path(slug_or_id), parent_category_id: parent_category_id).includes(:featured_users).first || self.where(id: slug_or_id.to_i, parent_category_id: parent_category_id).includes(:featured_users).first end diff --git a/app/models/slug_setting.rb b/app/models/slug_setting.rb new file mode 100644 index 00000000000..b2be27fcbd3 --- /dev/null +++ b/app/models/slug_setting.rb @@ -0,0 +1,17 @@ +require_dependency 'enum_site_setting' + +class SlugSetting < EnumSiteSetting + + VALUES = %w(ascii encoded none) + + def self.valid_value?(val) + VALUES.include?(val) + end + + def self.values + VALUES.map do |l| + {name: l, value: l} + end + end + +end diff --git a/app/models/topic.rb b/app/models/topic.rb index 8dc8df8bb64..ef8d90d6573 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -665,7 +665,7 @@ class Topic < ActiveRecord::Base def slug unless slug = read_attribute(:slug) return '' unless title.present? - slug = Slug.for(title).presence || "topic" + slug = Slug.for(title, 'topic') if new_record? write_attribute(:slug, slug) else @@ -677,7 +677,7 @@ class Topic < ActiveRecord::Base end def title=(t) - slug = (Slug.for(t.to_s).presence || "topic") + slug = Slug.for(t.to_s, 'topic') write_attribute(:slug, slug) write_attribute(:title,t) end diff --git a/app/serializers/notification_serializer.rb b/app/serializers/notification_serializer.rb index 50d0b1ef7b7..86ede10b919 100644 --- a/app/serializers/notification_serializer.rb +++ b/app/serializers/notification_serializer.rb @@ -10,7 +10,7 @@ class NotificationSerializer < ApplicationSerializer :is_warning def slug - Slug.for(object.topic.title) if object.topic.present? + Slug.for(object.topic.title, 'topic') if object.topic.present? end def is_warning diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb index 503aabe626d..9fa5d292dd5 100644 --- a/app/serializers/user_action_serializer.rb +++ b/app/serializers/user_action_serializer.rb @@ -58,7 +58,7 @@ class UserActionSerializer < ApplicationSerializer end def slug - Slug.for(object.title) + Slug.for(object.title, 'topic') end def include_slug? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0db4cbc5a3f..8fba30e700a 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1142,6 +1142,8 @@ en: prevent_anons_from_downloading_files: "Prevent anonymous users from downloading attachments. WARNING: this will prevent any non-image site assets posted as attachments from working." + slug_generation_method: "Choose a slug generation method. 'encoded' will generate percent encoding string. 'none' will disable slug at all." + enable_emoji: "Enable emoji" emoji_set: "How would you like your emoji?" enforce_square_emoji: "Force a square aspect ratio to all emojis." diff --git a/config/site_settings.yml b/config/site_settings.yml index c43ea748050..30270d02011 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -779,6 +779,10 @@ uncategorized: client: true default: true + slug_generation_method: + default: 'ascii' + enum: 'SlugSetting' + # Search min_search_term_length: client: true diff --git a/db/migrate/20140120155706_add_lounge_category.rb b/db/migrate/20140120155706_add_lounge_category.rb index 95ac270a6ea..e67ae7de7a7 100644 --- a/db/migrate/20140120155706_add_lounge_category.rb +++ b/db/migrate/20140120155706_add_lounge_category.rb @@ -14,10 +14,11 @@ class AddLoungeCategory < ActiveRecord::Migration result = execute "INSERT INTO categories (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position) - VALUES ('#{name}', 'EEEEEE', '652D90', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true, 3) + VALUES ('#{name}', 'EEEEEE', '652D90', now(), now(), -1, '', '#{description}', true, 3) RETURNING id" category_id = result[0]["id"].to_i + execute "UPDATE categories SET slug='#{Slug.for(name, "#{category_id}-category")}' WHERE id=#{category_id}" execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at) VALUES ('lounge_category_id', 3, #{category_id}, now(), now())" end diff --git a/db/migrate/20140122043508_add_meta_category.rb b/db/migrate/20140122043508_add_meta_category.rb index fe50ec0bc2d..aa128bd7d83 100644 --- a/db/migrate/20140122043508_add_meta_category.rb +++ b/db/migrate/20140122043508_add_meta_category.rb @@ -10,10 +10,11 @@ class AddMetaCategory < ActiveRecord::Migration result = Category.exec_sql "INSERT INTO categories (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position) VALUES (:name, '808281', 'FFFFFF', now(), now(), -1, :slug, :description, true, 1) - RETURNING id", name: name, slug: Slug.for(name), description: description + RETURNING id", name: name, slug: '', description: description category_id = result[0]["id"].to_i + execute "UPDATE categories SET slug='#{Slug.for(name, "#{category_id}-category")}' WHERE id=#{category_id}" execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at) VALUES ('meta_category_id', 3, #{category_id}, now(), now())" end diff --git a/db/migrate/20140227201005_add_staff_category.rb b/db/migrate/20140227201005_add_staff_category.rb index 441dbaef3af..6771fd978ef 100644 --- a/db/migrate/20140227201005_add_staff_category.rb +++ b/db/migrate/20140227201005_add_staff_category.rb @@ -8,10 +8,11 @@ class AddStaffCategory < ActiveRecord::Migration if Category.exec_sql("SELECT 1 FROM categories where name ilike '#{name}'").count == 0 result = execute "INSERT INTO categories (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position) - VALUES ('#{name}', '283890', 'FFFFFF', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true, 2) + VALUES ('#{name}', '283890', 'FFFFFF', now(), now(), -1, '', '#{description}', true, 2) RETURNING id" category_id = result[0]["id"].to_i + execute "UPDATE categories SET slug='#{Slug.for(name, "#{category_id}-category")}' WHERE id=#{category_id}" execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at) VALUES ('staff_category_id', 3, #{category_id}, now(), now())" end diff --git a/lib/onebox/engine/discourse_local_onebox.rb b/lib/onebox/engine/discourse_local_onebox.rb index db4075b43e2..91d39e2f670 100644 --- a/lib/onebox/engine/discourse_local_onebox.rb +++ b/lib/onebox/engine/discourse_local_onebox.rb @@ -48,7 +48,7 @@ module Onebox return linked unless Guardian.new.can_see?(post) topic = post.topic - slug = Slug.for(topic.title) + slug = Slug.for(topic.title, 'topic') excerpt = post.excerpt(SiteSetting.post_onebox_maxlength) excerpt.gsub!("\n"," ") diff --git a/lib/slug.rb b/lib/slug.rb index 61520772352..afde72162d4 100644 --- a/lib/slug.rb +++ b/lib/slug.rb @@ -1,21 +1,34 @@ # encoding: utf-8 - module Slug - def self.for(string) - # TODO review if ja should use this - # ko asked for it to be removed - if ['zh_CN', 'ja'].include?(SiteSetting.default_locale) - unless defined? Stringex - require 'stringex_lite' - end - slug = string.to_url - else - slug = string.gsub("'", "").parameterize - slug.gsub!("_", "-") - end - slug =~ /[^\d]/ ? slug : '' # Reject slugs that only contain numbers, because they would be indistinguishable from id's. + def self.for(string, default = 'topic') + slug = case SiteSetting.slug_generation_method.to_sym + when :ascii then self.ascii_generator(string) + when :encoded then self.encoded_generator(string) + when :none then self.none_generator(string) + else raise Discourse::SiteSettingMissing + end + # Reject slugs that only contain numbers, because they would be indistinguishable from id's. + slug = (slug =~ /[^\d]/ ? slug : '') + slug.blank? ? default : slug end + private + + def self.ascii_generator(string) + slug = string.gsub("'", "").parameterize + slug.gsub("_", "-") + end + + def self.encoded_generator(string) + # strip and sanitized RFC 3986 reserved character and blank + string = string.strip.gsub(/\s+/, '-').gsub(/[:\/\?#\[\]@!\$\s&'\(\)\*\+,;=]+/, '') + string = Rack::Utils.escape_path(string) + string =~ /^-+$/ ? '' : string + end + + def self.none_generator(string) + '' + end end diff --git a/lib/tasks/build_test_topic.rake b/lib/tasks/build_test_topic.rake index e135cabc928..40d801d7c8f 100644 --- a/lib/tasks/build_test_topic.rake +++ b/lib/tasks/build_test_topic.rake @@ -25,7 +25,7 @@ task 'build_test_topic' => :environment do first_post = PostCreator.new(evil_trout, raw: "This is the original post.", title: "pushState/replaceState test topic").create topic = first_post.topic - topic_url = "#{Discourse.base_url}/t/#{Slug.for(topic.title)}/#{topic.id}" + topic_url = "#{Discourse.base_url}/t/#{Slug.for(topic.title, 'topic')}/#{topic.id}" 99.times do |i| post_number = (i + 2) diff --git a/spec/components/slug_spec.rb b/spec/components/slug_spec.rb index 166f6b3cb04..827adf82353 100644 --- a/spec/components/slug_spec.rb +++ b/spec/components/slug_spec.rb @@ -5,60 +5,141 @@ require 'slug' describe Slug do - it 'replaces spaces with hyphens' do - expect(Slug.for("hello world")).to eq('hello-world') + describe '#for' do + context 'ascii generator' do + before { SiteSetting.slug_generation_method = 'ascii' } + + it 'generates the slug' do + expect(Slug.for("hello world", 'topic')).to eq('hello-world') + end + + it 'generates default slug when nothing' do + expect(Slug.for('', 'topic')).to eq('topic') + end + + it "doesn't generate slugs that are just numbers" do + expect(Slug.for('123', 'topic')).to eq('topic') + end + end + + context 'encoded generator' do + before { SiteSetting.slug_generation_method = 'encoded' } + after { SiteSetting.slug_generation_method = 'ascii' } + + it 'generates the slug' do + expect(Slug.for("熱帶風暴畫眉", 'topic')).to eq('%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89') + end + + it 'generates default slug when nothing' do + expect(Slug.for('', 'topic')).to eq('topic') + end + + it "doesn't generate slugs that are just numbers" do + expect(Slug.for('123', 'topic')).to eq('topic') + end + end + + context 'none generator' do + before { SiteSetting.slug_generation_method = 'none' } + after { SiteSetting.slug_generation_method = 'ascii' } + + it 'generates the slug' do + expect(Slug.for("hello world", 'category')).to eq('category') + expect(Slug.for("hello world", 'topic')).to eq('topic') + expect(Slug.for('', 'topic')).to eq('topic') + expect(Slug.for('123', 'topic')).to eq('topic') + end + end end - it 'changes accented characters' do - expect(Slug.for('àllo')).to eq('allo') + describe '#ascii_generator' do + before { SiteSetting.slug_generation_method = 'ascii' } + + it 'replaces spaces with hyphens' do + expect(Slug.ascii_generator("hello world")).to eq('hello-world') + end + + it 'changes accented characters' do + expect(Slug.ascii_generator('àllo')).to eq('allo') + end + + it 'replaces symbols' do + expect(Slug.ascii_generator('evil#trout')).to eq('evil-trout') + end + + it 'handles a.b.c properly' do + expect(Slug.ascii_generator("a.b.c")).to eq("a-b-c") + end + + it 'handles double dots right' do + expect(Slug.ascii_generator("a....b.....c")).to eq("a-b-c") + end + + it 'strips trailing punctuation' do + expect(Slug.ascii_generator("hello...")).to eq("hello") + end + + it 'strips leading punctuation' do + expect(Slug.ascii_generator("...hello")).to eq("hello") + end + + it 'handles our initial transliteration' do + from = "àáäâčďèéëěêìíïîľĺňòóöôŕřšťůùúüûýžñç" + to = "aaaacdeeeeeiiiillnoooorrstuuuuuyznc" + expect(Slug.ascii_generator(from)).to eq(to) + end + + it 'replaces underscores' do + expect(Slug.ascii_generator("o_o_o")).to eq("o-o-o") + end + + it "doesn't keep single quotes within word" do + expect(Slug.ascii_generator("Jeff hate's this")).to eq("jeff-hates-this") + end + + it 'generates null when nothing' do + expect(Slug.ascii_generator('')).to eq('') + end + + it "keeps number unchanged" do + expect(Slug.ascii_generator('123')).to eq('123') + end end - it 'replaces symbols' do - expect(Slug.for('evil#trout')).to eq('evil-trout') + describe '#encoded_generator' do + before { SiteSetting.slug_generation_method = 'encoded' } + after { SiteSetting.slug_generation_method = 'ascii' } + + it 'generates precentage encoded string' do + expect(Slug.encoded_generator("Jeff hate's !~-_|,=#this")).to eq("Jeff-hates-%7E-_%7Cthis") + expect(Slug.encoded_generator("뉴스피드")).to eq("%EB%89%B4%EC%8A%A4%ED%94%BC%EB%93%9C") + expect(Slug.encoded_generator("آموزش اضافه کردن لینک اختیاری به هدر")).to eq("%D8%A2%D9%85%D9%88%D8%B2%D8%B4-%D8%A7%D8%B6%D8%A7%D9%81%D9%87-%DA%A9%D8%B1%D8%AF%D9%86-%D9%84%DB%8C%D9%86%DA%A9-%D8%A7%D8%AE%D8%AA%DB%8C%D8%A7%D8%B1%DB%8C-%D8%A8%D9%87-%D9%87%D8%AF%D8%B1") + expect(Slug.encoded_generator("熱帶風暴畫眉")).to eq("%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89") + end + + it 'reject RFC 3986 reserved character and blank' do + expect(Slug.encoded_generator(":/?#[]@!$ &'()*+,;=")).to eq("") + end + + it 'generates null when nothing' do + expect(Slug.encoded_generator('')).to eq('') + end + + it "keeps number unchanged" do + expect(Slug.encoded_generator('123')).to eq('123') + end end - it 'handles a.b.c properly' do - expect(Slug.for("a.b.c")).to eq("a-b-c") - end + describe '#none_generator' do + before { SiteSetting.slug_generation_method = 'none' } + after { SiteSetting.slug_generation_method = 'ascii' } - it 'handles double dots right' do - expect(Slug.for("a....b.....c")).to eq("a-b-c") + it 'generates nothing' do + expect(Slug.none_generator("Jeff hate's this")).to eq('') + expect(Slug.none_generator(nil)).to eq('') + expect(Slug.none_generator('')).to eq('') + expect(Slug.none_generator('31')).to eq('') + end end - - it 'strips trailing punctuation' do - expect(Slug.for("hello...")).to eq("hello") - end - - it 'strips leading punctuation' do - expect(Slug.for("...hello")).to eq("hello") - end - - it 'handles our initial transliteration' do - from = "àáäâčďèéëěêìíïîľĺňòóöôŕřšťůùúüûýžñç" - to = "aaaacdeeeeeiiiillnoooorrstuuuuuyznc" - expect(Slug.for(from)).to eq(to) - end - - it 'replaces underscores' do - expect(Slug.for("o_o_o")).to eq("o-o-o") - end - - it "doesn't generate slugs that are just numbers" do - expect(Slug.for('123')).to be_blank - end - - it "doesn't generate slugs that are just numbers" do - expect(Slug.for('2')).to be_blank - end - - it "doesn't keep single quotes within word" do - expect(Slug.for("Jeff hate's this")).to eq("jeff-hates-this") - end - - it "translate the chineses" do - SiteSetting.default_locale = 'zh_CN' - expect(Slug.for("习近平:中企承建港口电站等助斯里兰卡发展")).to eq("xi-jin-ping-zhong-qi-cheng-jian-gang-kou-dian-zhan-deng-zhu-si-li-lan-qia-fa-zhan") - end - end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index f8d5f39c7dc..33a56e2015b 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -169,22 +169,54 @@ describe Category do end describe 'non-english characters' do - let(:category) { Fabricate(:category, name: "测试") } + context 'uses ascii slug generator' do + before do + SiteSetting.slug_generation_method = 'ascii' + @category = Fabricate(:category, name: "测试") + end + after { @category.destroy } - it "creates a blank slug, this is OK." do - expect(category.slug).to be_blank - expect(category.slug_for_url).to eq("#{category.id}-category") + it "creates a blank slug" do + expect(@category.slug).to be_blank + expect(@category.slug_for_url).to eq("#{@category.id}-category") + end end - it "creates a localized slug if default locale is zh_CN" do - SiteSetting.default_locale = 'zh_CN' - expect(category.slug).to_not be_blank - expect(category.slug_for_url).to eq("ce-shi") + context 'uses none slug generator' do + before do + SiteSetting.slug_generation_method = 'none' + @category = Fabricate(:category, name: "测试") + end + after do + SiteSetting.slug_generation_method = 'ascii' + @category.destroy + end + + it "creates a blank slug" do + expect(@category.slug).to be_blank + expect(@category.slug_for_url).to eq("#{@category.id}-category") + end + end + + context 'uses encoded slug generator' do + before do + SiteSetting.slug_generation_method = 'encoded' + @category = Fabricate(:category, name: "测试") + end + after do + SiteSetting.slug_generation_method = 'ascii' + @category.destroy + end + + it "creates a slug" do + expect(@category.slug).to eq("%E6%B5%8B%E8%AF%95") + expect(@category.slug_for_url).to eq("%E6%B5%8B%E8%AF%95") + end end end describe 'slug would be a number' do - let(:category) { Fabricate(:category, name: "2") } + let(:category) { Fabricate.build(:category, name: "2") } it 'creates a blank slug' do expect(category.slug).to be_blank @@ -193,21 +225,25 @@ describe Category do end describe 'custom slug can be provided' do - it 'has the custom value' do - c = Fabricate(:category, name: "Cats", slug: "cats-category") - expect(c.slug).to eq("cats-category") - end + it 'can be sanitized' do + @c = Fabricate(:category, name: "Fun Cats", slug: "fun-cats") + @cat = Fabricate(:category, name: "love cats", slug: "love-cats") - it 'and be sanitized' do - c = Fabricate(:category, name: 'Cats', slug: ' invalid slug') - expect(c.slug).to eq('invalid-slug') - end + @c.slug = ' invalid slug' + @c.save + expect(@c.slug).to eq('invalid-slug') - it 'fails if custom slug is duplicate with existing' do - c1 = Fabricate(:category, name: "Cats", slug: "cats") - c2 = Fabricate.build(:category, name: "More Cats", slug: "cats") - expect(c2).to_not be_valid - expect(c2.errors[:slug]).to be_present + c = Fabricate.build(:category, name: "More Fun Cats", slug: "love-cats") + expect(c).not_to be_valid + expect(c.errors[:slug]).to be_present + + @cat.slug = "#{@c.id}-category" + expect(@cat).not_to be_valid + expect(@cat.errors[:slug]).to be_present + + @cat.slug = "#{@cat.id}-category" + expect(@cat).to be_valid + expect(@cat.errors[:slug]).not_to be_present end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 66ce8e17b3f..105894c57bb 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -12,28 +12,64 @@ describe Topic do it { is_expected.to rate_limit } context 'slug' do - let(:title) { "hello world topic" } - let(:slug) { "hello-world-slug" } + let(:slug) { "hello-world-topic" } + context 'encoded generator' do + before { SiteSetting.slug_generation_method = 'encoded' } + after { SiteSetting.slug_generation_method = 'ascii' } - it "returns a Slug for a title" do - Slug.expects(:for).with(title).returns(slug) - expect(Fabricate.build(:topic, title: title).slug).to eq(slug) + it "returns a Slug for a title" do + Slug.expects(:for).with(title, 'topic').returns(slug) + expect(Fabricate.build(:topic, title: title).slug).to eq(slug) + end + + context 'for cjk characters' do + let(:title) { "熱帶風暴畫眉" } + let(:slug) { "%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89" } + it "returns encoded Slug for a title" do + Slug.expects(:for).with(title, 'topic').returns(slug) + expect(Fabricate.build(:topic, title: title).slug).to eq(slug) + end + end + + context 'for numbers' do + let(:title) { "123456789" } + let(:slug) { "topic" } + it 'generates default slug' do + Slug.expects(:for).with(title, 'topic').returns("topic") + expect(Fabricate.build(:topic, title: title).slug).to eq("topic") + end + end end - let(:chinese_title) { "习近平:中企承建港口电站等助斯里兰卡发展" } - let(:chinese_slug) { "xi-jin-ping-zhong-qi-cheng-jian-gang-kou-dian-zhan-deng-zhu-si-li-lan-qia-fa-zhan" } + context 'none generator' do + before { SiteSetting.slug_generation_method = 'none' } + after { SiteSetting.slug_generation_method = 'ascii' } + let(:title) { "熱帶風暴畫眉" } + let(:slug) { "topic" } - it "returns a symbolized slug for a chinese title" do - SiteSetting.default_locale = 'zh_CN' - expect(Fabricate.build(:topic, title: chinese_title).slug).to eq(chinese_slug) + it "returns a Slug for a title" do + Slug.expects(:for).with(title, 'topic').returns('topic') + expect(Fabricate.build(:topic, title: title).slug).to eq(slug) + end end - it "returns 'topic' when the slug is empty (say, non-english chars)" do - Slug.expects(:for).with(title).returns("") - expect(Fabricate.build(:topic, title: title).slug).to eq("topic") - end + context '#ascii_generator' do + before { SiteSetting.slug_generation_method = 'ascii' } + it "returns a Slug for a title" do + Slug.expects(:for).with(title, 'topic').returns(slug) + expect(Fabricate.build(:topic, title: title).slug).to eq(slug) + end + context 'for cjk characters' do + let(:title) { "熱帶風暴畫眉" } + let(:slug) { 'topic' } + it "returns 'topic' when the slug is empty (say, non-latin characters)" do + Slug.expects(:for).with(title, 'topic').returns("topic") + expect(Fabricate.build(:topic, title: title).slug).to eq("topic") + end + end + end end context "updating a title to be shorter" do diff --git a/test/javascripts/models/category-test.js.es6 b/test/javascripts/models/category-test.js.es6 index 5334b67dd46..b005ec1167c 100644 --- a/test/javascripts/models/category-test.js.es6 +++ b/test/javascripts/models/category-test.js.es6 @@ -9,6 +9,7 @@ test('slugFor', function(){ slugFor(Discourse.Category.create({slug: 'hello'}), "hello", "It calculates the proper slug for hello"); slugFor(Discourse.Category.create({id: 123, slug: ''}), "123-category", "It returns id-category for empty strings"); slugFor(Discourse.Category.create({id: 456}), "456-category", "It returns id-category for undefined slugs"); + slugFor(Discourse.Category.create({slug: '熱帶風暴畫眉'}), "熱帶風暴畫眉", "It can be non english characters"); var parentCategory = Discourse.Category.create({id: 345, slug: 'darth'}); slugFor(Discourse.Category.create({slug: 'luke', parentCategory: parentCategory}), @@ -27,16 +28,45 @@ test('slugFor', function(){ test('findBySlug', function() { + expect(6); + var darth = Discourse.Category.create({id: 1, slug: 'darth'}), - luke = Discourse.Category.create({id: 2, slug: 'luke', parentCategory: darth}), - categoryList = [darth, luke]; + luke = Discourse.Category.create({id: 2, slug: 'luke', parentCategory: darth}), + hurricane = Discourse.Category.create({id: 3, slug: '熱帶風暴畫眉'}), + newsFeed = Discourse.Category.create({id: 4, slug: '뉴스피드', parentCategory: hurricane}), + time = Discourse.Category.create({id: 5, slug: '时间', parentCategory: darth}), + bah = Discourse.Category.create({id: 6, slug: 'bah', parentCategory: hurricane}), + categoryList = [darth, luke, hurricane, newsFeed, time, bah]; sandbox.stub(Discourse.Category, 'list').returns(categoryList); - equal(Discourse.Category.findBySlug('darth'), darth, 'we can find a parent category'); - equal(Discourse.Category.findBySlug('luke', 'darth'), luke, 'we can find a child with parent'); - blank(Discourse.Category.findBySlug('luke'), 'luke is blank without the parent'); - blank(Discourse.Category.findBySlug('luke', 'leia'), 'luke is blank with an incorrect parent'); + deepEqual(Discourse.Category.findBySlug('darth'), darth, 'we can find a category'); + deepEqual(Discourse.Category.findBySlug('luke', 'darth'), luke, 'we can find the other category with parent category'); + deepEqual(Discourse.Category.findBySlug('熱帶風暴畫眉'), hurricane, 'we can find a category with CJK slug'); + deepEqual(Discourse.Category.findBySlug('뉴스피드', '熱帶風暴畫眉'), newsFeed, 'we can find a category with CJK slug whose parent slug is also CJK'); + deepEqual(Discourse.Category.findBySlug('时间', 'darth'), time, 'we can find a category with CJK slug whose parent slug is english'); + deepEqual(Discourse.Category.findBySlug('bah', '熱帶風暴畫眉'), bah, 'we can find a category with english slug whose parent slug is CJK'); +}); + +test('findSingleBySlug', function() { + expect(6); + + var darth = Discourse.Category.create({id: 1, slug: 'darth'}), + luke = Discourse.Category.create({id: 2, slug: 'luke', parentCategory: darth}), + hurricane = Discourse.Category.create({id: 3, slug: '熱帶風暴畫眉'}), + newsFeed = Discourse.Category.create({id: 4, slug: '뉴스피드', parentCategory: hurricane}), + time = Discourse.Category.create({id: 5, slug: '时间', parentCategory: darth}), + bah = Discourse.Category.create({id: 6, slug: 'bah', parentCategory: hurricane}), + categoryList = [darth, luke, hurricane, newsFeed, time, bah]; + + sandbox.stub(Discourse.Category, 'list').returns(categoryList); + + deepEqual(Discourse.Category.findSingleBySlug('darth'), darth, 'we can find a category'); + deepEqual(Discourse.Category.findSingleBySlug('darth/luke'), luke, 'we can find the other category with parent category'); + deepEqual(Discourse.Category.findSingleBySlug('熱帶風暴畫眉'), hurricane, 'we can find a category with CJK slug'); + deepEqual(Discourse.Category.findSingleBySlug('熱帶風暴畫眉/뉴스피드'), newsFeed, 'we can find a category with CJK slug whose parent slug is also CJK'); + deepEqual(Discourse.Category.findSingleBySlug('darth/时间'), time, 'we can find a category with CJK slug whose parent slug is english'); + deepEqual(Discourse.Category.findSingleBySlug('熱帶風暴畫眉/bah'), bah, 'we can find a category with english slug whose parent slug is CJK'); }); test('findByIds', function() {