Merge pull request #3370 from fantasticfears/slug

FEATURE: add slug geneartion options
This commit is contained in:
Régis Hanol 2015-05-05 14:26:40 +02:00
commit eb244bac34
15 changed files with 342 additions and 115 deletions

View File

@ -185,8 +185,6 @@ end
gem 'rmmseg-cpp', require: false
gem 'stringex', require: false
gem 'logster'
# perftools only works on 1.9 atm

View File

@ -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

View File

@ -207,14 +207,18 @@ SQL
if slug.present?
# santized custom slug
self.slug = Slug.for(slug)
self.slug = Slug.for(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

View File

@ -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

View File

@ -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)
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)
write_attribute(:slug, slug)
write_attribute(:title,t)
end

View File

@ -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."

View File

@ -779,6 +779,10 @@ uncategorized:
client: true
default: true
slug_generation_method:
default: 'ascii'
enum: 'SlugSetting'
# Search
min_search_term_length:
client: true

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -1,21 +1,38 @@
# 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'
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)
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.
# 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)
string.gsub("'", "")
.parameterize
.gsub("_", "-")
end
def self.encoded_generator(string)
# This generator will sanitize almost all special characters,
# including reserved characters from RFC3986.
# See also URI::REGEXP::PATTERN.
string.strip
.gsub(/\s+/, '-')
.gsub(/[:\/\?#\[\]@!\$&'\(\)\*\+,;=_\.~%\\`^\s|\{\}"<>]+/, '') # :/?#[]@!$&'()*+,;=_.~%\`^|{}"<>
.gsub(/\A-+|-+\z/, '') # remove possible trailing and preceding dashes
.squeeze('-') # squeeze continuous dashes to prettify slug
end
def self.none_generator(string)
''
end
end

View File

@ -5,60 +5,142 @@ require 'slug'
describe Slug do
it 'replaces spaces with hyphens' do
describe '#for' do
context 'ascii generator' do
before { SiteSetting.slug_generation_method = 'ascii' }
it 'generates the slug' do
expect(Slug.for("hello world")).to eq('hello-world')
end
it 'generates default slug when nothing' do
expect(Slug.for('')).to eq('topic')
end
it "doesn't generate slugs that are just numbers" do
expect(Slug.for('123')).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("熱帶風暴畫眉")).to eq('熱帶風暴畫眉')
end
it 'generates default slug when nothing' do
expect(Slug.for('')).to eq('topic')
end
it "doesn't generate slugs that are just numbers" do
expect(Slug.for('123')).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")).to eq('topic')
expect(Slug.for('')).to eq('topic')
expect(Slug.for('123')).to eq('topic')
end
end
end
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.for('àllo')).to eq('allo')
expect(Slug.ascii_generator('àllo')).to eq('allo')
end
it 'replaces symbols' do
expect(Slug.for('evil#trout')).to eq('evil-trout')
expect(Slug.ascii_generator('evil#trout')).to eq('evil-trout')
end
it 'handles a.b.c properly' do
expect(Slug.for("a.b.c")).to eq("a-b-c")
expect(Slug.ascii_generator("a.b.c")).to eq("a-b-c")
end
it 'handles double dots right' do
expect(Slug.for("a....b.....c")).to eq("a-b-c")
expect(Slug.ascii_generator("a....b.....c")).to eq("a-b-c")
end
it 'strips trailing punctuation' do
expect(Slug.for("hello...")).to eq("hello")
expect(Slug.ascii_generator("hello...")).to eq("hello")
end
it 'strips leading punctuation' do
expect(Slug.for("...hello")).to eq("hello")
expect(Slug.ascii_generator("...hello")).to eq("hello")
end
it 'handles our initial transliteration' do
from = "àáäâčďèéëěêìíïîľĺňòóöôŕřšťůùúüûýžñç"
to = "aaaacdeeeeeiiiillnoooorrstuuuuuyznc"
expect(Slug.for(from)).to eq(to)
expect(Slug.ascii_generator(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
expect(Slug.ascii_generator("o_o_o")).to eq("o-o-o")
end
it "doesn't keep single quotes within word" do
expect(Slug.for("Jeff hate's this")).to eq("jeff-hates-this")
expect(Slug.ascii_generator("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")
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
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-this")
expect(Slug.encoded_generator("뉴스피드")).to eq("뉴스피드")
expect(Slug.encoded_generator("آموزش اضافه کردن لینک اختیاری به هدر")).to eq("آموزش-اضافه-کردن-لینک-اختیاری-به-هدر")
expect(Slug.encoded_generator("熱帶風暴畫眉")).to eq("熱帶風暴畫眉")
end
it 'reject RFC 3986 reserved character and blank' do
expect(Slug.encoded_generator(":/?#[]@!$ &'()*+,;=% -_`~.")).to eq("")
expect(Slug.encoded_generator(" - English and Chinese title with special characters / 中文标题 !@:?\\:'`#^& $%&*()` -- ")).to eq("English-and-Chinese-title-with-special-characters-中文标题")
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
describe '#none_generator' do
before { SiteSetting.slug_generation_method = 'none' }
after { SiteSetting.slug_generation_method = 'ascii' }
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
end

View File

@ -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("测试")
expect(@category.slug_for_url).to eq("测试")
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

View File

@ -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)
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" }
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)
context 'for cjk characters' do
let(:title) { "熱帶風暴畫眉" }
let(:slug) { "熱帶風暴畫眉" }
it "returns encoded Slug for a title" do
Slug.expects(:for).with(title).returns(slug)
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("")
context 'for numbers' do
let(:title) { "123456789" }
let(:slug) { "topic" }
it 'generates default slug' do
Slug.expects(:for).with(title).returns("topic")
expect(Fabricate.build(:topic, title: title).slug).to eq("topic")
end
end
end
context 'none generator' do
before { SiteSetting.slug_generation_method = 'none' }
after { SiteSetting.slug_generation_method = 'ascii' }
let(:title) { "熱帶風暴畫眉" }
let(:slug) { "topic" }
it "returns a Slug for a title" do
Slug.expects(:for).with(title).returns('topic')
expect(Fabricate.build(:topic, title: title).slug).to eq(slug)
end
end
context '#ascii_generator' do
before { 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)
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).returns("topic")
expect(Fabricate.build(:topic, title: title).slug).to eq("topic")
end
end
end
end
context "updating a title to be shorter" do

View File

@ -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];
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() {