DEV: Added modifier hooks to allow plugins to tweak how categories and groups are fetched (#21837)

This commit adds modifiers that allow plugins to change how categories and groups are prefetched into the application and listed in the respective controllers.

Possible use cases:

- prevent some categories/groups from being prefetched when the application loads for performance reasons.
- prevent some categories/groups from being listed in their respective index pages.
This commit is contained in:
Sérgio Saquetim 2023-05-30 18:41:50 -03:00 committed by GitHub
parent 1e3a5124da
commit 908117e270
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 196 additions and 26 deletions

View File

@ -59,7 +59,8 @@ class GroupsController < ApplicationController
if !guardian.is_staff?
# hide automatic groups from all non stuff to de-clutter page
groups = groups.where("automatic IS FALSE OR groups.id = ?", Group::AUTO_GROUPS[:moderators])
groups =
groups.where("groups.automatic IS FALSE OR groups.id = ?", Group::AUTO_GROUPS[:moderators])
type_filters.delete(:automatic)
end
@ -80,6 +81,8 @@ class GroupsController < ApplicationController
type_filters = type_filters - %i[my owner]
end
groups = DiscoursePluginRegistry.apply_modifier(:groups_index_query, groups, self)
type_filters.delete(:non_automatic)
# count the total before doing pagination
@ -122,7 +125,10 @@ class GroupsController < ApplicationController
groups = Group.visible_groups(current_user)
if !guardian.is_staff?
groups =
groups.where("automatic IS FALSE OR groups.id = ?", Group::AUTO_GROUPS[:moderators])
groups.where(
"groups.automatic IS FALSE OR groups.id = ?",
Group::AUTO_GROUPS[:moderators],
)
end
render_json_dump(
@ -613,10 +619,12 @@ class GroupsController < ApplicationController
.order(:name)
if (term = params[:term]).present?
groups = groups.where("name ILIKE :term OR full_name ILIKE :term", term: "%#{term}%")
groups =
groups.where("groups.name ILIKE :term OR groups.full_name ILIKE :term", term: "%#{term}%")
end
groups = groups.where(automatic: false) if params[:ignore_automatic].to_s == "true"
groups = DiscoursePluginRegistry.apply_modifier(:groups_search_query, groups, self)
if Group.preloaded_custom_field_names.present?
Group.preload_custom_fields(groups, Group.preloaded_custom_field_names)

View File

@ -119,17 +119,19 @@ class CategoryList
end
def find_categories
@categories = Category.includes(CategoryList.included_associations).secured(@guardian)
query = Category.includes(CategoryList.included_associations).secured(@guardian)
@categories =
@categories.where(
query =
query.where(
"categories.parent_category_id = ?",
@options[:parent_category_id].to_i,
) if @options[:parent_category_id].present?
@categories = self.class.order_categories(@categories)
query = self.class.order_categories(query)
query =
DiscoursePluginRegistry.apply_modifier(:category_list_find_categories_query, query, self)
@categories = @categories.to_a
@categories = query.to_a
include_subcategories = @options[:include_subcategories] == true

View File

@ -657,14 +657,17 @@ class Group < ActiveRecord::Base
groups ||= Group
relation =
groups.where("name ILIKE :term_like OR full_name ILIKE :term_like", term_like: "%#{name}%")
groups.where(
"groups.name ILIKE :term_like OR groups.full_name ILIKE :term_like",
term_like: "%#{name}%",
)
if sort == :auto
prefix = "#{name.gsub("_", "\\_")}%"
relation =
relation.reorder(
DB.sql_fragment(
"CASE WHEN name ILIKE :like OR full_name ILIKE :like THEN 0 ELSE 1 END ASC, name ASC",
"CASE WHEN groups.name ILIKE :like OR groups.full_name ILIKE :like THEN 0 ELSE 1 END ASC, groups.name ASC",
like: prefix,
),
)

View File

@ -71,20 +71,27 @@ class Site
.cache
.fetch(categories_cache_key, expires_in: 30.minutes) do
categories =
Category
.includes(
:uploaded_logo,
:uploaded_logo_dark,
:uploaded_background,
:tags,
:tag_groups,
:form_templates,
category_required_tag_groups: :tag_group,
)
.joins("LEFT JOIN topics t on t.id = categories.topic_id")
.select("categories.*, t.slug topic_slug")
.order(:position)
.to_a
begin
query =
Category
.includes(
:uploaded_logo,
:uploaded_logo_dark,
:uploaded_background,
:tags,
:tag_groups,
:form_templates,
category_required_tag_groups: :tag_group,
)
.joins("LEFT JOIN topics t on t.id = categories.topic_id")
.select("categories.*, t.slug topic_slug")
.order(:position)
query =
DiscoursePluginRegistry.apply_modifier(:site_all_categories_cache_query, query, self)
query.to_a
end
if preloaded_category_custom_fields.present?
Category.preload_custom_fields(categories, preloaded_category_custom_fields)
@ -151,7 +158,13 @@ class Site
end
def groups
Group.visible_groups(@guardian.user, "name ASC", include_everyone: true).includes(:flair_upload)
query =
Group.visible_groups(@guardian.user, "groups.name ASC", include_everyone: true).includes(
:flair_upload,
)
query = DiscoursePluginRegistry.apply_modifier(:site_groups_query, query, self)
query
end
def archetypes

View File

@ -630,7 +630,7 @@ class Search
Group
.visible_groups(@guardian.user)
.members_visible_groups(@guardian.user)
.where("groups.name ILIKE ? OR (id = ? AND id > 0)", match, match.to_i)
.where("groups.name ILIKE ? OR (groups.id = ? AND groups.id > 0)", match, match.to_i)
DiscoursePluginRegistry.search_groups_set_query_callbacks.each do |cb|
group_query = cb.call(group_query, @term, @guardian)

View File

@ -327,4 +327,27 @@ RSpec.describe CategoryList do
end
end
end
describe "category_list_find_categories_query modifier" do
fab!(:cool_category) { Fabricate(:category, name: "Cool category") }
fab!(:boring_category) { Fabricate(:category, name: "Boring category") }
it "allows changing the query" do
prefetched_categories = CategoryList.new(Guardian.new(user)).categories.map { |c| c[:id] }
expect(prefetched_categories).to include(cool_category.id, boring_category.id)
Plugin::Instance
.new
.register_modifier(:category_list_find_categories_query) do |query|
query.where("categories.name LIKE 'Cool%'")
end
prefetched_categories = CategoryList.new(Guardian.new(user)).categories.map { |c| c[:id] }
expect(prefetched_categories).to include(cool_category.id)
expect(prefetched_categories).not_to include(boring_category.id)
ensure
DiscoursePluginRegistry.clear_modifiers!
end
end
end

View File

@ -157,6 +157,32 @@ RSpec.describe Site do
expect(site.categories.map { |c| c[:can_edit] }).to contain_exactly(true, true)
end
describe "site_all_categories_cache_query modifier" do
fab!(:cool_category) { Fabricate(:category, name: "Cool category") }
fab!(:boring_category) { Fabricate(:category, name: "Boring category") }
it "allows changing the query" do
prefetched_categories = Site.new(Guardian.new(user)).categories.map { |c| c[:id] }
expect(prefetched_categories).to include(cool_category.id, boring_category.id)
# we need to clear the cache to ensure that the categories list will be updated
Site.clear_cache
Plugin::Instance
.new
.register_modifier(:site_all_categories_cache_query) do |query|
query.where("categories.name LIKE 'Cool%'")
end
prefetched_categories = Site.new(Guardian.new(user)).categories.map { |c| c[:id] }
expect(prefetched_categories).to include(cool_category.id)
expect(prefetched_categories).not_to include(boring_category.id)
ensure
DiscoursePluginRegistry.clear_modifiers!
end
end
end
it "omits groups user can not see" do
@ -174,6 +200,31 @@ RSpec.describe Site do
expect(site.groups.pluck(:name)).to include(staff_group.name, public_group.name, "everyone")
end
describe "site_groups_query modifier" do
fab!(:user) { Fabricate(:user) }
fab!(:cool_group) { Fabricate(:group, name: "cool-group") }
fab!(:boring_group) { Fabricate(:group, name: "boring-group") }
it "allows changing the query" do
prefetched_groups = Site.new(Guardian.new(user)).groups.map { |c| c[:id] }
expect(prefetched_groups).to include(cool_group.id, boring_group.id)
# we need to clear the cache to ensure that the groups list will be updated
Site.clear_cache
Plugin::Instance
.new
.register_modifier(:site_groups_query) { |query| query.where("groups.name LIKE 'cool%'") }
prefetched_groups = Site.new(Guardian.new(user)).groups.map { |c| c[:id] }
expect(prefetched_groups).to include(cool_group.id)
expect(prefetched_groups).not_to include(boring_group.id)
ensure
DiscoursePluginRegistry.clear_modifiers!
end
end
it "includes all enabled authentication providers" do
SiteSetting.enable_twitter_logins = true
SiteSetting.enable_facebook_logins = true

View File

@ -397,6 +397,46 @@ RSpec.describe GroupsController do
end
end
end
describe "groups_index_query modifier" do
fab!(:user) { Fabricate(:user) }
fab!(:cool_group) { Fabricate(:group, name: "cool-group") }
fab!(:boring_group) { Fabricate(:group, name: "boring-group") }
it "allows changing the query" do
get "/groups.json"
expect(response.status).to eq(200)
expect(response.parsed_body["groups"].map { |g| g["id"] }).to include(
cool_group.id,
boring_group.id,
)
get "/groups.json", params: { filter: "cool" }
expect(response.status).to eq(200)
expect(response.parsed_body["groups"].map { |g| g["id"] }).to include(cool_group.id)
expect(response.parsed_body["groups"].map { |g| g["id"] }).not_to include(boring_group.id)
Plugin::Instance
.new
.register_modifier(:groups_index_query) do |query|
query.where("groups.name LIKE 'cool%'")
end
get "/groups.json"
expect(response.status).to eq(200)
expect(response.parsed_body["groups"].map { |g| g["id"] }).to include(cool_group.id)
expect(response.parsed_body["groups"].map { |g| g["id"] }).not_to include(boring_group.id)
get "/groups.json", params: { filter: "boring" }
expect(response.status).to eq(200)
expect(response.parsed_body["groups"].map { |g| g["id"] }).not_to include(
cool_group.id,
boring_group.id,
)
ensure
DiscoursePluginRegistry.clear_modifiers!
end
end
end
describe "#show" do
@ -2328,6 +2368,36 @@ RSpec.describe GroupsController do
expect(groups.map { |group| group["id"] }).to contain_exactly(group.id, hidden_group.id)
end
end
describe "groups_search_query modifier" do
fab!(:user) { Fabricate(:user) }
fab!(:cool_group) { Fabricate(:group, name: "cool-group") }
fab!(:boring_group) { Fabricate(:group, name: "boring-group") }
before { sign_in(user) }
it "allows changing the query" do
get "/groups/search.json", params: { term: "cool" }
expect(response.status).to eq(200)
expect(response.parsed_body.map { |g| g["id"] }).to include(cool_group.id)
expect(response.parsed_body.map { |g| g["id"] }).not_to include(boring_group.id)
Plugin::Instance
.new
.register_modifier(:groups_search_query) do |query|
query.where("groups.name LIKE 'boring%'")
end
get "/groups/search.json", params: { term: "cool" }
expect(response.status).to eq(200)
expect(response.parsed_body.map { |g| g["id"] }).not_to include(
cool_group.id,
boring_group.id,
)
ensure
DiscoursePluginRegistry.clear_modifiers!
end
end
end
describe "#new" do