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
This commit is contained in:
parent
766d337d42
commit
fe676f334a
|
@ -26,7 +26,8 @@ class CategoriesController < ApplicationController
|
||||||
category_options = {
|
category_options = {
|
||||||
is_homepage: current_homepage == "categories",
|
is_homepage: current_homepage == "categories",
|
||||||
parent_category_id: params[:parent_category_id],
|
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)
|
@category_list = CategoryList.new(guardian, category_options)
|
||||||
|
|
|
@ -139,7 +139,7 @@ class Category < ActiveRecord::Base
|
||||||
|
|
||||||
# permission is just used by serialization
|
# permission is just used by serialization
|
||||||
# we may consider wrapping this in another spot
|
# 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.
|
# Allows us to skip creating the category definition topic in tests.
|
||||||
attr_accessor :skip_category_definition
|
attr_accessor :skip_category_definition
|
||||||
|
|
|
@ -100,6 +100,8 @@ class CategoryList
|
||||||
|
|
||||||
@categories = @categories.to_a
|
@categories = @categories.to_a
|
||||||
|
|
||||||
|
include_subcategories = @options[:include_subcategories] == true
|
||||||
|
|
||||||
notification_levels = CategoryUser.notification_levels_for(@guardian.user)
|
notification_levels = CategoryUser.notification_levels_for(@guardian.user)
|
||||||
default_notification_level = CategoryUser.default_notification_level
|
default_notification_level = CategoryUser.default_notification_level
|
||||||
|
|
||||||
|
@ -111,16 +113,26 @@ class CategoryList
|
||||||
end
|
end
|
||||||
|
|
||||||
if @options[:parent_category_id].blank?
|
if @options[:parent_category_id].blank?
|
||||||
subcategories = {}
|
subcategory_ids = {}
|
||||||
|
subcategory_list = {}
|
||||||
to_delete = Set.new
|
to_delete = Set.new
|
||||||
@categories.each do |c|
|
@categories.each do |c|
|
||||||
if c.parent_category_id.present?
|
if c.parent_category_id.present?
|
||||||
subcategories[c.parent_category_id] ||= []
|
subcategory_ids[c.parent_category_id] ||= []
|
||||||
subcategories[c.parent_category_id] << c.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
|
to_delete << c
|
||||||
end
|
end
|
||||||
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) }
|
@categories.delete_if { |c| to_delete.include?(c) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -14,10 +14,16 @@ class CategoryDetailedSerializer < BasicCategorySerializer
|
||||||
|
|
||||||
has_many :displayable_topics, serializer: ListableTopicSerializer, embed: :objects, key: :topics
|
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?
|
def include_displayable_topics?
|
||||||
displayable_topics.present?
|
displayable_topics.present?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def include_subcategory_list?
|
||||||
|
subcategory_list.present?
|
||||||
|
end
|
||||||
|
|
||||||
def is_uncategorized
|
def is_uncategorized
|
||||||
object.id == SiteSetting.uncategorized_category_id
|
object.id == SiteSetting.uncategorized_category_id
|
||||||
end
|
end
|
||||||
|
|
|
@ -141,6 +141,15 @@
|
||||||
|
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
|
"subcategory_list": {
|
||||||
|
"type": [
|
||||||
|
"array",
|
||||||
|
"null"
|
||||||
|
],
|
||||||
|
"items": [
|
||||||
|
|
||||||
|
]
|
||||||
|
},
|
||||||
"uploaded_logo": {
|
"uploaded_logo": {
|
||||||
"type": [
|
"type": [
|
||||||
"string",
|
"string",
|
||||||
|
|
|
@ -68,6 +68,55 @@ describe CategoriesController do
|
||||||
)
|
)
|
||||||
end
|
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
|
it 'does not show uncategorized unless allow_uncategorized_topics' do
|
||||||
SiteSetting.desktop_category_page_style = "categories_boxes_with_topics"
|
SiteSetting.desktop_category_page_style = "categories_boxes_with_topics"
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue