DEV: Deprecate Category#url_with_id in favor of Category#url (#9972)

app/controllers/category_hashtags_controller.rb

@@ -9,7 +9,7 @@ class CategoryHashtagsController < ApplicationController
     ids = category_slugs.map { |category_slug| Category.query_from_hashtag_slug(category_slug).try(:id) }
 
     valid_categories = Category.secured(guardian).where(id: ids).map do |category|
-      { slug: category.hashtag_slug, url: category.url_with_id }
+      { slug: category.hashtag_slug, url: category.url }
     end.compact
 
     render json: { valid: valid_categories }

app/models/category.rb

@@ -721,11 +721,13 @@ class Category < ActiveRecord::Base
   end
 
   def url
-    @@url_cache[self.id] ||= "#{Discourse.base_uri}/c/#{slug_path.join('/')}"
+    @@url_cache[self.id] ||= "#{Discourse.base_uri}/c/#{slug_path.join('/')}/#{self.id}"
   end
 
   def url_with_id
-    self.parent_category ? "#{url}/#{self.id}" : "#{Discourse.base_uri}/c/#{self.slug}/#{self.id}"
+    Discourse.deprecate("Category#url_with_id is deprecated. Use `Category#url` instead.", output_in_test: true)
+
+    url
   end
 
   # If the name changes, try and update the category definition topic too if it's an exact match
@@ -739,9 +741,10 @@ class Category < ActiveRecord::Base
 
   def create_category_permalink
     old_slug = saved_changes.transform_values(&:first)["slug"]
+
     url = +"#{Discourse.base_uri}/c"
     url << "/#{parent_category.slug_path.join('/')}" if parent_category_id
-    url << "/#{old_slug}"
+    url << "/#{old_slug}/#{id}"
     url = Permalink.normalize_url(url)
 
     if Permalink.where(url: url).exists?

app/models/permalink.rb

@@ -80,7 +80,7 @@ class Permalink < ActiveRecord::Base
     return external_url if external_url
     return "#{Discourse::base_uri}#{post.url}" if post
     return topic.relative_url if topic
-    return "#{category.url}/#{category.id}" if category
+    return category.url if category
     return tag.full_url if tag
     nil
   end

app/models/topic_list.rb

@@ -67,7 +67,7 @@ class TopicList
 
   def preload_key
     if @category
-      "topic_list_#{@category.url.sub(/^\//, '')}/#{@category.id}/l/#{@filter}"
+      "topic_list_#{@category.url.sub(/^\//, '')}/l/#{@filter}"
     else
       "topic_list_#{@filter}"
     end

lib/pretty_text/helpers.rb

@@ -41,7 +41,7 @@ module PrettyText
 
     def category_hashtag_lookup(category_slug)
       if category = Category.query_from_hashtag_slug(category_slug)
-        [category.url_with_id, category_slug]
+        [category.url, category_slug]
       else
         nil
       end
@@ -106,7 +106,7 @@ module PrettyText
       is_tag = text =~ /#{tag_postfix}$/
 
       if !is_tag && category = Category.query_from_hashtag_slug(text)
-        [category.url_with_id, text]
+        [category.url, text]
       elsif (!is_tag && tag = Tag.find_by(name: text)) ||
             (is_tag && tag = Tag.find_by(name: text.gsub!("#{tag_postfix}", '')))
         ["#{Discourse.base_url}/tag/#{tag.name}", text]

spec/components/pretty_text_spec.rb

@@ -1106,9 +1106,9 @@ describe PrettyText do
 
     [
       "<span class=\"hashtag\">#unknown::tag</span>",
-      "<a class=\"hashtag\" href=\"#{category2.url_with_id}\">#<span>known</span></a>",
+      "<a class=\"hashtag\" href=\"#{category2.url}\">#<span>known</span></a>",
       "<a class=\"hashtag\" href=\"http://test.localhost/tag/known\">#<span>known</span></a>",
-      "<a class=\"hashtag\" href=\"#{category.url_with_id}\">#<span>testing</span></a>"
+      "<a class=\"hashtag\" href=\"#{category.url}\">#<span>testing</span></a>"
     ].each do |element|
 
       expect(cooked).to include(element)

spec/models/category_spec.rb

@@ -430,7 +430,7 @@ describe Category do
     end
 
     it "reuses existing permalink when category slug is changed" do
-      permalink = Permalink.create!(url: "c/#{@category.slug}", category_id: 42)
+      permalink = Permalink.create!(url: "c/#{@category.slug}/#{@category.id}", category_id: 42)
 
       expect { @category.update(slug: 'new-slug') }.to_not change { Permalink.count }
       expect(permalink.reload.category_id).to eq(@category.id)
@@ -695,47 +695,20 @@ describe Category do
 
     describe "for normal categories" do
       it "builds a url" do
-        expect(category.url).to eq("/c/root")
+        expect(category.url).to eq("/c/root/#{category.id}")
       end
     end
 
     describe "for subcategories" do
       it "builds a url" do
-        expect(sub_category.url).to eq("/c/root/child")
+        expect(sub_category.url).to eq("/c/root/child/#{sub_category.id}")
       end
     end
 
     describe "for sub-sub-categories" do
       it "builds a url" do
         expect(sub_sub_category.url)
-          .to eq("/c/root/child/child-of-child")
-      end
-    end
-  end
-
-  describe "#url_with_id" do
-    fab!(:category) do
-      Fabricate(
-        :category_with_definition,
-        name: 'cats',
-      )
-    end
-
-    it "includes the id in the URL" do
-      expect(category.url_with_id).to eq("/c/cats/#{category.id}")
-    end
-
-    context "child category" do
-      fab!(:child_category) do
-        Fabricate(
-          :category,
-          parent_category_id: category.id,
-          name: 'dogs',
-        )
-      end
-
-      it "includes the id in the URL" do
-        expect(child_category.url_with_id).to eq("/c/cats/dogs/#{child_category.id}")
+          .to eq("/c/root/child/child-of-child/#{sub_sub_category.id}")
       end
     end
   end

spec/models/permalink_spec.rb

@@ -63,7 +63,7 @@ describe Permalink do
 
     it "returns a category url when category_id is set" do
       permalink.category_id = category.id
-      expect(target_url).to eq("#{category.url}/#{category.id}")
+      expect(target_url).to eq("#{category.url}")
     end
 
     it "returns nil when category_id is set but category is not found" do

spec/requests/categories_controller_spec.rb

@@ -13,7 +13,7 @@ describe CategoriesController do
       get '/categories', headers: { 'HTTP_USER_AGENT' => 'Googlebot' }
       html = Nokogiri::HTML5(response.body)
       expect(html.css('body.crawler')).to be_present
-      expect(html.css("a[href=\"/forum/c/#{category.slug}\"]")).to be_present
+      expect(html.css("a[href=\"/forum/c/#{category.slug}/#{category.id}\"]")).to be_present
     end
 
     it "properly preloads topic list" do

spec/requests/category_hashtags_controller_spec.rb

@@ -18,7 +18,7 @@ describe CategoryHashtagsController do
 
           expect(response.status).to eq(200)
           expect(response.parsed_body).to eq(
-            "valid" => [{ "slug" => category.hashtag_slug, "url" => category.url_with_id }]
+            "valid" => [{ "slug" => category.hashtag_slug, "url" => category.url }]
           )
         end
 
@@ -45,7 +45,7 @@ describe CategoryHashtagsController do
 
           expect(response.status).to eq(200)
           expect(response.parsed_body).to eq(
-            "valid" => [{ "slug" => private_category.hashtag_slug, "url" => private_category.url_with_id }]
+            "valid" => [{ "slug" => private_category.hashtag_slug, "url" => private_category.url }]
           )
         end
       end