FEATURE: Return subcategories on categories endpoint (#14492)

FEATURE: Return subcategories on categories endpoint (#14492)

  • FEATURE: Return subcategories on categories endpoint

When using the API subcategories will now be returned nested inside of each category response under the subcategory_list param. We already return all the subcategory ids under the subcategory_ids param, but you then would have to make multiple separate API calls to fetch each of those subcategories. This way you can get ALL of the categories along with their subcategories in a single API response.

The UI will not be affected by this change because you need to pass in the include_subcategories=true param in order for subcategories to be returned.

In a follow up PR I’ll add the API scoping for fetching categories so that a readonly API key can be used for the /categories.json endpoint. This endpoint should be used instead of the /site.json endpoint for fetching a sites categories and subcategories.

  • Update PR based on feedback
  • Have spec check for specific subcategory
  • Move comparison check out of loop
  • Only populate subcategory list if option present
  • Remove empty array initialization
  • Update api spec to allow null response
  • More PR updates based on feedback
  • Use a category serializer for the subcategory_list
  • Don’t include the subcategory_list param if empty
  • For the spec check for the subcategory by id
  • Fix spec to account for param not present when empty
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 2931dbc..4e7337e 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -26,7 +26,8 @@ class CategoriesController < ApplicationController
     category_options = {
       is_homepage: current_homepage == "categories",
       parent_category_id: params[:parent_category_id],
-      include_topics: include_topics(parent_category)
+      include_topics: include_topics(parent_category),
+      include_subcategories: params[:include_subcategories] == "true"
     }
 
     @category_list = CategoryList.new(guardian, category_options)
diff --git a/app/models/category.rb b/app/models/category.rb
index 07b890e..630a74c 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -139,7 +139,7 @@ class Category < ActiveRecord::Base
 
   # permission is just used by serialization
   # we may consider wrapping this in another spot
-  attr_accessor :displayable_topics, :permission, :subcategory_ids, :notification_level, :has_children
+  attr_accessor :displayable_topics, :permission, :subcategory_ids, :subcategory_list, :notification_level, :has_children
 
   # Allows us to skip creating the category definition topic in tests.
   attr_accessor :skip_category_definition
diff --git a/app/models/category_list.rb b/app/models/category_list.rb
index 4144f79..655edfa 100644
--- a/app/models/category_list.rb
+++ b/app/models/category_list.rb
@@ -100,6 +100,8 @@ class CategoryList
 
     @categories = @categories.to_a
 
+    include_subcategories = @options[:include_subcategories] == true
+
     notification_levels = CategoryUser.notification_levels_for(@guardian.user)
     default_notification_level = CategoryUser.default_notification_level
 
@@ -111,16 +113,26 @@ class CategoryList
     end
 
     if @options[:parent_category_id].blank?
-      subcategories = {}
+      subcategory_ids = {}
+      subcategory_list = {}
       to_delete = Set.new
       @categories.each do |c|
         if c.parent_category_id.present?
-          subcategories[c.parent_category_id] ||= []
-          subcategories[c.parent_category_id] << c.id
+          subcategory_ids[c.parent_category_id] ||= []
+          subcategory_ids[c.parent_category_id] << c.id
+          if include_subcategories
+            subcategory_list[c.parent_category_id] ||= []
+            subcategory_list[c.parent_category_id] << c
+          end
           to_delete << c
         end
       end
-      @categories.each { |c| c.subcategory_ids = subcategories[c.id] || [] }
+      @categories.each do |c|
+        c.subcategory_ids = subcategory_ids[c.id] || []
+        if include_subcategories
+          c.subcategory_list = subcategory_list[c.id] || []
+        end
+      end
       @categories.delete_if { |c| to_delete.include?(c) }
     end
 
diff --git a/app/serializers/category_detailed_serializer.rb b/app/serializers/category_detailed_serializer.rb
index 6b1e508..1349cd4 100644
--- a/app/serializers/category_detailed_serializer.rb
+++ b/app/serializers/category_detailed_serializer.rb
@@ -14,10 +14,16 @@ class CategoryDetailedSerializer < BasicCategorySerializer
 
   has_many :displayable_topics, serializer: ListableTopicSerializer, embed: :objects, key: :topics
 
+  has_many :subcategory_list, serializer: CategoryDetailedSerializer, embed: :objects, key: :subcategory_list
+
   def include_displayable_topics?
     displayable_topics.present?
   end
 
+  def include_subcategory_list?
+    subcategory_list.present?
+  end
+
   def is_uncategorized
     object.id == SiteSetting.uncategorized_category_id
   end
diff --git a/spec/requests/api/schemas/json/category_list_response.json b/spec/requests/api/schemas/json/category_list_response.json
index 1aa3c8f..fc3c424 100644
--- a/spec/requests/api/schemas/json/category_list_response.json
+++ b/spec/requests/api/schemas/json/category_list_response.json
@@ -141,6 +141,15 @@
 
                   ]
                 },
+                "subcategory_list": {
+                  "type": [
+                    "array",
+                    "null"
+                  ],
+                  "items": [
+
+                  ]
+                },
                 "uploaded_logo": {
                   "type": [
                     "string",
diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb
index 2cfed06..2067279 100644
--- a/spec/requests/categories_controller_spec.rb
+++ b/spec/requests/categories_controller_spec.rb
@@ -68,6 +68,55 @@ describe CategoriesController do
       )
     end
 
+    it 'does not returns subcatgories without permission' do
+      subcategory = Fabricate(:category,  user: admin, parent_category: category)
+      subcategory.set_permissions(admins: :full)
+      subcategory.save!
+
+      sign_in(user)
+
+      get "/categories.json?include_subcategories=true"
+
+      expect(response.status).to eq(200)
+
+      category_list = response.parsed_body["category_list"]
+
+      subcategories_for_category = category_list["categories"][1]["subcategory_list"]
+      expect(subcategories_for_category).to eq(nil)
+    end
+
+    it 'returns the right subcategory response with permission' do
+      subcategory = Fabricate(:category, user: admin, parent_category: category)
+
+      sign_in(user)
+
+      get "/categories.json?include_subcategories=true"
+
+      expect(response.status).to eq(200)
+
+      category_list = response.parsed_body["category_list"]
+
+      subcategories_for_category = category_list["categories"][1]["subcategory_list"]
+      expect(subcategories_for_category.count).to eq(1)
+      expect(subcategories_for_category.first["parent_category_id"]).to eq(category.id)
+      expect(subcategories_for_category.first["id"]).to eq(subcategory.id)
+    end
+
+    it 'does not return subcategories without query param' do
+      subcategory = Fabricate(:category, user: admin, parent_category: category)
+
+      sign_in(user)
+
+      get "/categories.json"
+
+      expect(response.status).to eq(200)
+
+      category_list = response.parsed_body["category_list"]
+
+      subcategories_for_category = category_list["categories"][1]["subcategory_list"]
+      expect(subcategories_for_category).to eq(nil)
+    end
+
     it 'does not show uncategorized unless allow_uncategorized_topics' do
       SiteSetting.desktop_category_page_style = "categories_boxes_with_topics"
 

GitHub sha: fe676f334aa07489092b4d6af61d643ecb95aa27

This commit appears in #14492 which was approved by CvX and eviltrout. It was merged by blake.