FIX: Redirect user to the URL with the correct category slug (#10167)

This commit implements a category redirect similar to the one for topic
URLs with wrong slug.

app/controllers/list_controller.rb

@@ -338,6 +338,21 @@ class ListController < ApplicationController
 
     raise Discourse::NotFound.new("category not found", check_permalinks: true) if @category.nil?
 
+    if !guardian.can_see?(@category)
+      if SiteSetting.detailed_404
+        raise Discourse::InvalidAccess
+      else
+        raise Discourse::NotFound
+      end
+    end
+
+    current_slug = params.require(:category_slug_path_with_id)
+    real_slug = @category.full_slug("/")
+    if current_slug != real_slug
+      url = request.fullpath.gsub(current_slug, real_slug)
+      return redirect_to path(url), status: 301
+    end
+
     params[:category] = @category.id.to_s
 
     @description_meta = if @category.uncategorized?
@@ -347,14 +362,6 @@ class ListController < ApplicationController
     end
     @description_meta = SiteSetting.site_description if @description_meta.blank?
 
-    if !guardian.can_see?(@category)
-      if SiteSetting.detailed_404
-        raise Discourse::InvalidAccess
-      else
-        raise Discourse::NotFound
-      end
-    end
-
     if use_crawler_layout?
       @subcategories = @category.subcategories.select { |c| guardian.can_see?(c) }
     end

spec/requests/api/categories_spec.rb

@@ -243,12 +243,13 @@ describe 'categories' do
     end
   end
 
-  path '/c/{category_id}.json' do
+  path '/c/{category_slug}/{category_id}.json' do
 
     get 'List topics' do
       tags 'Categories'
       produces 'application/json'
-      parameter name: :category_id, in: :path, schema: { type: :string }
+      parameter name: :category_slug, in: :path, schema: { type: :string }
+      parameter name: :category_id, in: :path, schema: { type: :integer }
 
       response '200', 'response' do
         schema type: :object, properties: {
@@ -329,6 +330,7 @@ describe 'categories' do
           }
         }
         let(:category_id) { category.id }
+        let(:category_slug) { category.slug_path.join("/") }
         run_test!
       end
     end

spec/requests/list_controller_spec.rb

@@ -363,14 +363,7 @@ RSpec.describe ListController do
 
       context 'with access to see the category' do
         it "succeeds" do
-          get "/c/#{category.slug}/l/latest"
+          get "/c/#{category.slug}/#{category.id}/l/latest"
-          expect(response.status).to eq(200)
-        end
-      end
-
-      context 'with a link that includes an id' do
-        it "succeeds" do
-          get "/c/#{category.id}-category/l/latest"
           expect(response.status).to eq(200)
         end
       end
@@ -403,7 +396,7 @@ RSpec.describe ListController do
         }
 
         it 'uses the correct category' do
-          get "/c/#{other_category.slug}/l/latest.json"
+          get "/c/#{other_category.slug}/#{other_category.id}/l/latest.json"
           expect(response.status).to eq(200)
           body = response.parsed_body
           expect(body["topic_list"]["topics"].first["category_id"])
@@ -416,7 +409,7 @@ RSpec.describe ListController do
 
         context 'when parent and child are requested' do
           it "succeeds" do
-            get "/c/#{category.slug}/#{sub_category.slug}/l/latest"
+            get "/c/#{category.slug}/#{sub_category.slug}/#{sub_category.id}/l/latest"
             expect(response.status).to eq(200)
           end
         end
@@ -431,14 +424,14 @@ RSpec.describe ListController do
 
       describe 'feed' do
         it 'renders RSS' do
-          get "/c/#{category.slug}.rss"
+          get "/c/#{category.slug}/#{category.id}.rss"
           expect(response.status).to eq(200)
           expect(response.media_type).to eq('application/rss+xml')
         end
 
         it "renders RSS in subfolder correctly" do
           set_subfolder "/forum"
-          get "/c/#{category.slug}.rss"
+          get "/c/#{category.slug}/#{category.id}.rss"
           expect(response.status).to eq(200)
           expect(response.body).to_not include("/forum/forum")
           expect(response.body).to include("http://test.localhost/forum/c/#{category.slug}")
@@ -448,7 +441,7 @@ RSpec.describe ListController do
       describe "category default views" do
         it "has a top default view" do
           category.update!(default_view: 'top', default_top_period: 'monthly')
-          get "/c/#{category.slug}.json"
+          get "/c/#{category.slug}/#{category.id}.json"
           expect(response.status).to eq(200)
           json = response.parsed_body
           expect(json["topic_list"]["for_period"]).to eq("monthly")
@@ -456,7 +449,7 @@ RSpec.describe ListController do
 
         it "has a default view of nil" do
           category.update!(default_view: nil)
-          get "/c/#{category.slug}.json"
+          get "/c/#{category.slug}/#{category.id}.json"
           expect(response.status).to eq(200)
           json = response.parsed_body
           expect(json["topic_list"]["for_period"]).to be_blank
@@ -464,7 +457,7 @@ RSpec.describe ListController do
 
         it "has a default view of ''" do
           category.update!(default_view: '')
-          get "/c/#{category.slug}.json"
+          get "/c/#{category.slug}/#{category.id}.json"
           expect(response.status).to eq(200)
           json = response.parsed_body
           expect(json["topic_list"]["for_period"]).to be_blank
@@ -472,7 +465,7 @@ RSpec.describe ListController do
 
         it "has a default view of latest" do
           category.update!(default_view: 'latest')
-          get "/c/#{category.slug}.json"
+          get "/c/#{category.slug}/#{category.id}.json"
           expect(response.status).to eq(200)
           json = response.parsed_body
           expect(json["topic_list"]["for_period"]).to be_blank
@@ -481,13 +474,13 @@ RSpec.describe ListController do
 
       describe "renders canonical tag" do
         it 'for category default view' do
-          get "/c/#{category.slug}"
+          get "/c/#{category.slug}/#{category.id}"
           expect(response.status).to eq(200)
           expect(css_select("link[rel=canonical]").length).to eq(1)
         end
 
         it 'for category latest view' do
-          get "/c/#{category.slug}/l/latest"
+          get "/c/#{category.slug}/#{category.id}/l/latest"
           expect(response.status).to eq(200)
           expect(css_select("link[rel=canonical]").length).to eq(1)
         end
@@ -497,14 +490,14 @@ RSpec.describe ListController do
         let!(:amazing_category) { Fabricate(:category_with_definition, name: "Amazing Category") }
 
         it 'for category default view' do
-          get "/c/#{amazing_category.slug}"
+          get "/c/#{amazing_category.slug}/#{amazing_category.id}"
 
           expect(response.body).to have_tag "title", text: "Amazing Category - Discourse"
         end
 
         it 'for category latest view' do
           SiteSetting.short_site_description = "Best community"
-          get "/c/#{amazing_category.slug}/l/latest"
+          get "/c/#{amazing_category.slug}/#{amazing_category.id}/l/latest"
 
           expect(response.body).to have_tag "title", text: "Amazing Category - Discourse"
         end
@@ -646,4 +639,30 @@ RSpec.describe ListController do
       expect(response.status).to eq(404)
     end
   end
+
+  describe "set_category" do
+    let(:category) { Fabricate(:category_with_definition) }
+    let(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
+    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id) }
+
+    before do
+      SiteSetting.max_category_nesting = 3
+    end
+
+    it "redirects to URL with the updated slug" do
+      get "/c/hello/world/bye/#{subsubcategory.id}"
+      expect(response.status).to eq(301)
+      expect(response).to redirect_to("/c/#{category.slug}/#{subcategory.slug}/#{subsubcategory.slug}/#{subsubcategory.id}")
+    end
+
+    context "with subfolder" do
+      it "redirects to URL containing the updated slug" do
+        set_subfolder "/forum"
+        get "/c/hello/world/bye/#{subsubcategory.id}"
+
+        expect(response.status).to eq(301)
+        expect(response).to redirect_to("/forum/c/#{category.slug}/#{subcategory.slug}/#{subsubcategory.slug}/#{subsubcategory.id}")
+      end
+    end
+  end
 end