FIX: Make category slug validation less strict (#8915)

This was changed recently and caused issues saving old categories which
already had digits at the beginning of the slug (for example, '30-days').

app/assets/javascripts/discourse/models/category.js.es6

@@ -274,8 +274,12 @@ Category.reopenClass({
       result = Category.slugFor(parentCategory) + separator;
     }
 
-    const slug = get(category, "slug");
+    const id = get(category, "id"),
-    return !slug || slug.trim().length === 0 ? `${result}-` : result + slug;
+      slug = get(category, "slug");
+
+    return !slug || slug.trim().length === 0
+      ? `${result}${id}-category`
+      : result + slug;
   },
 
   list() {
@@ -360,7 +364,7 @@ Category.reopenClass({
       if (
         !category &&
         parts.length > 0 &&
-        parts[parts.length - 1].match(/^\d+-/)
+        parts[parts.length - 1].match(/^\d+-category/)
       ) {
         const id = parseInt(parts.pop(), 10);
 

app/controllers/list_controller.rb

@@ -324,7 +324,7 @@ class ListController < ApplicationController
       end
 
       # Legacy paths
-      if @category.nil? && parts.last =~ /\A\d+-/
+      if @category.nil? && parts.last =~ /\A\d+-category/
         @category = Category.find_by_id(parts.last.to_i)
       end
     end

app/controllers/tags_controller.rb

@@ -364,7 +364,7 @@ class TagsController < ::ApplicationController
         end
 
         # Legacy paths
-        if @filter_on_category.nil? && parts.last =~ /\A\d+-/
+        if @filter_on_category.nil? && parts.last =~ /\A\d+-category/
           @filter_on_category = Category.find_by_id(parts.last.to_i)
         end
       end

app/models/category.rb

@@ -316,7 +316,7 @@ class Category < ActiveRecord::Base
     end
 
     # only allow to use category itself id.
-    match_id = /^(\d+)-/.match(self.slug)
+    match_id = /^(\d+)-category/.match(self.slug)
     if match_id.present?
       errors.add(:slug, :invalid) if new_record? || (match_id[1] != self.id.to_s)
     end

spec/requests/list_controller_spec.rb

@@ -350,7 +350,7 @@ RSpec.describe ListController do
 
       context 'with a link that includes an id' do
         it "succeeds" do
-          get "/c/#{category.id}-#{category.slug}/l/latest"
+          get "/c/#{category.id}-category/l/latest"
           expect(response.status).to eq(200)
         end
       end

test/javascripts/models/category-test.js.es6

@@ -17,13 +17,13 @@ QUnit.test("slugFor", assert => {
   );
   slugFor(
     store.createRecord("category", { id: 123, slug: "" }),
-    "-",
+    "123-category",
-    "It returns - for empty slugs"
+    "It returns id-category for empty strings"
   );
   slugFor(
     store.createRecord("category", { id: 456 }),
-    "-",
+    "456-category",
-    "It returns - for undefined slugs"
+    "It returns id-category for undefined slugs"
   );
   slugFor(
     store.createRecord("category", { slug: "熱帶風暴畫眉" }),
@@ -46,14 +46,14 @@ QUnit.test("slugFor", assert => {
 
   slugFor(
     store.createRecord("category", { id: 555, parentCategory: parentCategory }),
-    "darth/-",
+    "darth/555-category",
     "it uses the parent slug before the child and then uses id"
   );
 
   parentCategory.set("slug", null);
   slugFor(
     store.createRecord("category", { id: 555, parentCategory: parentCategory }),
-    "-/-",
+    "345-category/555-category",
     "it uses the parent before the child and uses ids for both"
   );
 });