DEV: Server-side category routing changes

The routes for categories are changing. The scheme that I intend to move
us to is:

/c/*slug_path/(:id)/ENDPOINT
/c/*slug_path/(:id)

This commit adds support for the new scheme to the server side without
dropping support for existing URLs. It is necessary to support existing
URLs for two reasons:

 * This commit does not change any client side routing code,
 * Posts that contain category hashtags that refer to a root category
   are baked into URLs that do not fit this new scheme, (/c/[id]-[slug])

app/controllers/list_controller.rb

@@ -11,16 +11,12 @@ class ListController < ApplicationController
     # filtered topics lists
     Discourse.filters.map { |f| :"category_#{f}" },
     Discourse.filters.map { |f| :"category_none_#{f}" },
-    Discourse.filters.map { |f| :"parent_category_category_#{f}" },
-    Discourse.filters.map { |f| :"parent_category_category_none_#{f}" },
     # top summaries
     :category_top,
     :category_none_top,
-    :parent_category_category_top,
     # top pages (ie. with a period)
     TopTopic.periods.map { |p| :"category_top_#{p}" },
     TopTopic.periods.map { |p| :"category_none_top_#{p}" },
-    TopTopic.periods.map { |p| :"parent_category_category_top_#{p}" },
     # category feeds
     :category_feed,
   ].flatten
@@ -34,8 +30,6 @@ class ListController < ApplicationController
     :category_default,
     Discourse.anonymous_filters.map { |f| :"category_#{f}" },
     Discourse.anonymous_filters.map { |f| :"category_none_#{f}" },
-    Discourse.anonymous_filters.map { |f| :"parent_category_category_#{f}" },
-    Discourse.anonymous_filters.map { |f| :"parent_category_category_none_#{f}" },
     # category feeds
     :category_feed,
     # user topics feed
@@ -44,13 +38,11 @@ class ListController < ApplicationController
     :top,
     :category_top,
     :category_none_top,
-    :parent_category_category_top,
     # top pages (ie. with a period)
     TopTopic.periods.map { |p| :"top_#{p}" },
     TopTopic.periods.map { |p| :"top_#{p}_feed" },
     TopTopic.periods.map { |p| :"category_top_#{p}" },
     TopTopic.periods.map { |p| :"category_none_top_#{p}" },
-    TopTopic.periods.map { |p| :"parent_category_category_top_#{p}" },
     :group_topics
   ].flatten
 
@@ -124,15 +116,6 @@ class ListController < ApplicationController
     define_method("category_none_#{filter}") do
       self.public_send(filter, category: @category.id, no_subcategories: true)
     end
-
-    define_method("parent_category_category_#{filter}") do
-      canonical_url "#{Discourse.base_url_no_prefix}#{@category.url}"
-      self.public_send(filter, category: @category.id)
-    end
-
-    define_method("parent_category_category_none_#{filter}") do
-      self.public_send(filter, category: @category.id)
-    end
   end
 
   def category_default
@@ -257,10 +240,6 @@ class ListController < ApplicationController
     top(category: @category.id, no_subcategories: true)
   end
 
-  def parent_category_category_top
-    top(category: @category.id)
-  end
-
   TopTopic.periods.each do |period|
     define_method("top_#{period}") do |options = nil|
       top_options = build_topic_list_options
@@ -296,10 +275,6 @@ class ListController < ApplicationController
       )
     end
 
-    define_method("parent_category_category_top_#{period}") do
-      self.public_send("top_#{period}", category: @category.id)
-    end
-
     # rss feed
     define_method("top_#{period}_feed") do |options = nil|
       discourse_expires_in 1.minute
@@ -333,32 +308,42 @@ class ListController < ApplicationController
 
   def page_params
     route_params = { format: 'json' }
-    route_params[:category]        = @category.slug_for_url                  if @category
+
-    route_params[:parent_category] = @category.parent_category.slug_for_url  if @category && @category.parent_category
+    if @category.present?
-    route_params[:username]        = UrlHelper.escape_uri(params[:username]) if params[:username].present?
+      slug_path = @category.slug_path
+
+      route_params[:category_slug_path_with_id] =
+        (slug_path + [@category.id.to_s]).join("/")
+    end
+
+    route_params[:username] = UrlHelper.escape_uri(params[:username]) if params[:username].present?
     route_params
   end
 
   def set_category
-    slug_or_id = params.fetch(:category)
+    parts = params.require(:category_slug_path_with_id).split('/')
-    parent_slug_or_id = params[:parent_category]
-    id = params[:id].to_i
 
-    parent_category_id = nil
+    if !parts.empty? && parts.last =~ /\A\d+\Z/
-    if parent_slug_or_id.present?
+      id = parts.pop.to_i
-      parent_category_id = Category.query_parent_category(parent_slug_or_id)
+    end
-      raise Discourse::NotFound.new("category not found", check_permalinks: true) if parent_category_id.blank? && !id
+    slug_path = parts unless parts.empty?
+
+    if id.present?
+      @category = Category.find_by_id(id)
+    elsif slug_path.present?
+      if (1..2).include?(slug_path.size)
+        @category = Category.find_by_slug(*slug_path.reverse)
+      end
+
+      # Legacy paths
+      if @category.nil? && parts.last =~ /\A\d+-/
+        @category = Category.find_by_id(parts.last.to_i)
+      end
     end
 
-    @category = Category.query_category(slug_or_id, parent_category_id)
+    raise Discourse::NotFound.new("category not found", check_permalinks: true) if @category.nil?
 
-    # Redirect if we have `/c/:parent_category/:category/:id`
+    params[:category] = @category.id.to_s
-    if params.include?(:id)
-      category = Category.find_by_id(id)
-      (redirect_to category.url, status: 301) && return if category
-    end
-
-    raise Discourse::NotFound.new("category not found", check_permalinks: true) if !@category
 
     @description_meta = @category.description_text
     if !guardian.can_see?(@category)
@@ -388,12 +373,21 @@ class ListController < ApplicationController
 
   def construct_url_with(action, opts, url_prefix = nil)
     method = url_prefix.blank? ? "#{action_name}_path" : "#{url_prefix}_#{action_name}_path"
-    url = if action == :prev
+
-      public_send(method, opts.merge(prev_page_params))
+    page_params =
-    else # :next
+      case action
-      public_send(method, opts.merge(next_page_params))
+      when :prev
-    end
+        prev_page_params
-    url.sub('.json?', '?')
+      when :next
+        next_page_params
+      else
+        raise "unreachable"
+      end
+
+    opts = opts.dup
+    opts.delete(:category) if page_params.include?(:category_slug_path_with_id)
+
+    public_send(method, opts.merge(page_params)).sub('.json?', '?')
   end
 
   def get_excluded_category_ids(current_category = nil)

app/models/category.rb

@@ -808,6 +808,16 @@ class Category < ActiveRecord::Base
     end
   end
 
+  def slug_path
+    if self.parent_category_id.present?
+      slug_path = self.parent_category.slug_path
+      slug_path.push(self.slug_for_url)
+      slug_path
+    else
+      [self.slug_for_url]
+    end
+  end
+
   private
 
   def check_permissions_compatibility(parent_permissions, child_permissions)

config/routes.rb

@@ -620,35 +620,49 @@ Discourse::Application.routes.draw do
 
   get '/c', to: redirect(relative_url_root + 'categories')
 
-  resources :categories, except: :show
+  resources :categories, except: [:show, :new, :edit]
-  post "category/:category_id/move" => "categories#move"
   post "categories/reorder" => "categories#reorder"
-  post "category/:category_id/notifications" => "categories#set_notifications"
+
-  put "category/:category_id/slug" => "categories#update_slug"
+  scope path: 'category/:category_id' do
+    post "/move" => "categories#move"
+    post "/notifications" => "categories#set_notifications"
+    put "/slug" => "categories#update_slug"
+  end
+
+  get "category/*path" => "categories#redirect"
 
   get "categories_and_latest" => "categories#categories_and_latest"
   get "categories_and_top" => "categories#categories_and_top"
 
   get "c/:id/show" => "categories#show"
+
   get "c/:category_slug/find_by_slug" => "categories#find_by_slug"
   get "c/:parent_category_slug/:category_slug/find_by_slug" => "categories#find_by_slug"
-  get "c/:category.rss" => "list#category_feed", format: :rss
+
-  get "c/:parent_category/:category.rss" => "list#category_feed", format: :rss
+  get "c/*category_slug_path_with_id.rss" => "list#category_feed", format: :rss
-  get "c/:category" => "list#category_default", as: "category_default"
+  scope path: 'c/*category_slug_path_with_id' do
-  get "c/:category/none" => "list#category_none_latest"
+    get "/none" => "list#category_none_latest"
-  get "c/:parent_category/:category/(:id)" => "list#parent_category_category_latest", constraints: { id: /\d+/ }
+    get "/none/l/top" => "list#category_none_top", as: "category_none_top"
-  get "c/:category/l/top" => "list#category_top", as: "category_top"
+    get "/l/top" => "list#category_top", as: "category_top"
-  get "c/:category/none/l/top" => "list#category_none_top", as: "category_none_top"
+
-  get "c/:parent_category/:category/l/top" => "list#parent_category_category_top", as: "parent_category_category_top"
+    TopTopic.periods.each do |period|
+      get "/none/l/top/#{period}" => "list#category_none_top_#{period}", as: "category_none_top_#{period}"
+      get "/l/top/#{period}" => "list#category_top_#{period}", as: "category_top_#{period}"
+    end
+
+    Discourse.filters.each do |filter|
+      get "/none/l/#{filter}" => "list#category_none_#{filter}", as: "category_none_#{filter}"
+      get "/l/#{filter}" => "list#category_#{filter}", as: "category_#{filter}"
+    end
+
+    get "/" => "list#category_default", as: "category_default"
+  end
 
   get "category_hashtags/check" => "category_hashtags#check"
 
   TopTopic.periods.each do |period|
     get "top/#{period}.rss" => "list#top_#{period}_feed", format: :rss
     get "top/#{period}" => "list#top_#{period}"
-    get "c/:category/l/top/#{period}" => "list#category_top_#{period}", as: "category_top_#{period}"
-    get "c/:category/none/l/top/#{period}" => "list#category_none_top_#{period}", as: "category_none_top_#{period}"
-    get "c/:parent_category/:category/l/top/#{period}" => "list#parent_category_category_top_#{period}", as: "parent_category_category_top_#{period}"
   end
 
   Discourse.anonymous_filters.each do |filter|
@@ -657,13 +671,8 @@ Discourse::Application.routes.draw do
 
   Discourse.filters.each do |filter|
     get "#{filter}" => "list##{filter}"
-    get "c/:category/l/#{filter}" => "list#category_#{filter}", as: "category_#{filter}"
-    get "c/:category/none/l/#{filter}" => "list#category_none_#{filter}", as: "category_none_#{filter}"
-    get "c/:parent_category/:category/l/#{filter}" => "list#parent_category_category_#{filter}", as: "parent_category_category_#{filter}"
   end
 
-  get "category/*path" => "categories#redirect"
-
   get "top" => "list#top"
   get "search/query" => "search#query"
   get "search" => "search#show"

spec/requests/list_controller_spec.rb

@@ -380,19 +380,15 @@ RSpec.describe ListController do
         let(:child_category) { Fabricate(:category_with_definition, parent_category: category) }
 
         context "with valid slug" do
-          it "redirects to the child category" do
+          it "succeeds" do
-            get "/c/#{category.slug}/#{child_category.slug}/l/latest", params: {
+            get "/c/#{category.slug}/#{child_category.slug}/#{child_category.id}/l/latest"
-              id: child_category.id
+            expect(response.status).to eq(200)
-            }
-            expect(response).to redirect_to(child_category.url)
           end
         end
 
         context "with invalid slug" do
-          it "redirects to child category" do
+          xit "redirects" do
-            get "/c/random_slug/another_random_slug/l/latest", params: {
+            get "/c/random_slug/another_random_slug/#{child_category.id}/l/latest"
-              id: child_category.id
-            }
             expect(response).to redirect_to(child_category.url)
           end
         end