FIX: Always preload admin plugin list for admin in sidebar (#25606)

When we show the links to installed plugins in the admin
sidebar (for plugins that have custom admin routes) we were
previously only doing this if you opened /admin, not if you
navigated there from the main forum. We should just always
preload this data if the user is admin.

This commit also changes `admin_sidebar_enabled_groups` to
not be sent to the client as part of ongoing efforts to
not check groups on the client, since not all a user's groups
may be serialized.
This commit is contained in:
Martin Brennan 2024-02-09 12:52:22 +10:00 committed by GitHub
parent 110d544225
commit 3cc73cfd1e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 73 additions and 67 deletions

View File

@ -7,12 +7,9 @@ export default class AdminController extends Controller {
@service router; @service router;
@service currentUser; @service currentUser;
@discourseComputed("siteSettings.admin_sidebar_enabled_groups") @discourseComputed("currentUser.use_admin_sidebar")
showAdminSidebar() { showAdminSidebar() {
return this.siteSettings.userInAnyGroups( return this.currentUser.use_admin_sidebar;
"admin_sidebar_enabled_groups",
this.currentUser
);
} }
@discourseComputed("siteSettings.enable_group_directory") @discourseComputed("siteSettings.enable_group_directory")

View File

@ -14,12 +14,7 @@ export default class AdminRoute extends DiscourseRoute {
} }
activate() { activate() {
if ( if (!this.currentUser.use_admin_sidebar) {
!this.siteSettings.userInAnyGroups(
"admin_sidebar_enabled_groups",
this.currentUser
)
) {
return DiscourseURL.redirectTo("/admin"); return DiscourseURL.redirectTo("/admin");
} }

View File

@ -13,12 +13,7 @@ export default class AdminRoute extends DiscourseRoute {
} }
activate() { activate() {
if ( if (this.currentUser.use_admin_sidebar) {
this.siteSettings.userInAnyGroups(
"admin_sidebar_enabled_groups",
this.currentUser
)
) {
this.sidebarState.setPanel(ADMIN_PANEL); this.sidebarState.setPanel(ADMIN_PANEL);
this.sidebarState.setSeparatedMode(); this.sidebarState.setSeparatedMode();
this.sidebarState.hideSwitchPanelButtons(); this.sidebarState.hideSwitchPanelButtons();
@ -32,12 +27,7 @@ export default class AdminRoute extends DiscourseRoute {
deactivate(transition) { deactivate(transition) {
this.controllerFor("application").set("showTop", true); this.controllerFor("application").set("showTop", true);
if ( if (this.currentUser.use_admin_sidebar) {
this.siteSettings.userInAnyGroups(
"admin_sidebar_enabled_groups",
this.currentUser
)
) {
if (!transition?.to.name.startsWith("admin")) { if (!transition?.to.name.startsWith("admin")) {
this.sidebarState.setPanel(MAIN_PANEL); this.sidebarState.setPanel(MAIN_PANEL);
} }

View File

@ -1,4 +1,3 @@
import { isEmpty } from "@ember/utils";
import PreloadStore from "discourse/lib/preload-store"; import PreloadStore from "discourse/lib/preload-store";
import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map"; import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map";
import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel"; import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel";
@ -214,8 +213,9 @@ export default class AdminSidebarPanel extends BaseCustomSidebarPanel {
hidden = true; hidden = true;
get sections() { get sections() {
const currentUser = getOwnerWithFallback().lookup("service:current-user");
const siteSettings = getOwnerWithFallback().lookup("service:site-settings"); const siteSettings = getOwnerWithFallback().lookup("service:site-settings");
if (isEmpty(siteSettings.admin_sidebar_enabled_groups)) { if (!currentUser.use_admin_sidebar) {
return []; return [];
} }

View File

@ -29,13 +29,7 @@ export default class AdminRevampSectionLink extends BaseSectionLink {
return false; return false;
} }
return ( return this.currentUser.use_admin_sidebar;
this.currentUser.staff &&
this.siteSettings.userInAnyGroups(
"admin_sidebar_enabled_groups",
this.currentUser
)
);
} }
get defaultPrefixValue() { get defaultPrefixValue() {

View File

@ -9,10 +9,7 @@ acceptance("Admin Sidebar - Sections", function (needs) {
needs.user({ needs.user({
admin: true, admin: true,
groups: [AUTO_GROUPS.admins], groups: [AUTO_GROUPS.admins],
}); use_admin_sidebar: true,
needs.settings({
admin_sidebar_enabled_groups: "1",
}); });
needs.hooks.beforeEach(() => { needs.hooks.beforeEach(() => {
@ -75,10 +72,7 @@ acceptance("Admin Sidebar - Sections - Plugin API", function (needs) {
needs.user({ needs.user({
admin: true, admin: true,
groups: [AUTO_GROUPS.admins], groups: [AUTO_GROUPS.admins],
}); use_admin_sidebar: true,
needs.settings({
admin_sidebar_enabled_groups: "1",
}); });
needs.hooks.beforeEach(() => { needs.hooks.beforeEach(() => {

View File

@ -7,13 +7,4 @@ class Admin::AdminController < ApplicationController
def index def index
render body: nil render body: nil
end end
private
def preload_additional_json
store_preloaded(
"enabledPluginAdminRoutes",
MultiJson.dump(Discourse.plugins_sorted_by_name.filter_map(&:admin_route)),
)
end
end end

View File

@ -8,13 +8,4 @@ class Admin::PluginsController < Admin::StaffController
root: "plugins", root: "plugins",
) )
end end
private
def preload_additional_json
store_preloaded(
"enabledPluginAdminRoutes",
MultiJson.dump(Discourse.plugins_sorted_by_name.filter_map(&:admin_route)),
)
end
end end

View File

@ -446,8 +446,6 @@ class ApplicationController < ActionController::Base
current_user.sync_notification_channel_position current_user.sync_notification_channel_position
preload_current_user_data preload_current_user_data
end end
preload_additional_json
end end
def set_mobile_view def set_mobile_view
@ -669,12 +667,16 @@ class ApplicationController < ActionController::Base
store_preloaded("topicTrackingStates", MultiJson.dump(hash[:data])) store_preloaded("topicTrackingStates", MultiJson.dump(hash[:data]))
store_preloaded("topicTrackingStateMeta", MultiJson.dump(hash[:meta])) store_preloaded("topicTrackingStateMeta", MultiJson.dump(hash[:meta]))
# This is used in the wizard so we can preload fonts using the FontMap JS API. if current_user.admin?
store_preloaded("fontMap", MultiJson.dump(load_font_map)) if current_user.admin? # This is used in the wizard so we can preload fonts using the FontMap JS API.
end store_preloaded("fontMap", MultiJson.dump(load_font_map))
def preload_additional_json # Used to show plugin-specific admin routes in the sidebar.
# noop, should be defined by subcontrollers store_preloaded(
"enabledPluginAdminRoutes",
MultiJson.dump(Discourse.plugins_sorted_by_name.filter_map(&:admin_route)),
)
end
end end
def custom_html_json def custom_html_json

View File

@ -69,7 +69,9 @@ class CurrentUserSerializer < BasicUserSerializer
:sidebar_category_ids, :sidebar_category_ids,
:sidebar_sections, :sidebar_sections,
:new_new_view_enabled?, :new_new_view_enabled?,
:use_experimental_topic_bulk_actions? :use_experimental_topic_bulk_actions?,
:use_experimental_topic_bulk_actions?,
:use_admin_sidebar
delegate :user_stat, to: :object, private: true delegate :user_stat, to: :object, private: true
delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat
@ -122,6 +124,14 @@ class CurrentUserSerializer < BasicUserSerializer
scope.can_send_private_messages? scope.can_send_private_messages?
end end
def use_admin_sidebar
object.admin? && object.in_any_groups?(SiteSetting.admin_sidebar_enabled_groups_map)
end
def include_user_admin_sidebar?
object.admin?
end
def can_post_anonymously def can_post_anonymously
SiteSetting.allow_anonymous_posting && SiteSetting.allow_anonymous_posting &&
(is_anonymous || object.in_any_groups?(SiteSetting.anonymous_posting_allowed_groups_map)) (is_anonymous || object.in_any_groups?(SiteSetting.anonymous_posting_allowed_groups_map))

View File

@ -2332,7 +2332,6 @@ developer:
hidden: true hidden: true
client: true client: true
admin_sidebar_enabled_groups: admin_sidebar_enabled_groups:
client: true
type: group_list type: group_list
list_type: compact list_type: compact
default: "" default: ""

View File

@ -0,0 +1,37 @@
# frozen_string_literal: true
require "rails_helper"
RSpec.describe ApplicationController do
fab!(:user)
fab!(:admin)
def preloaded_json
JSON.parse(
Nokogiri::HTML5.fragment(response.body).css("div#data-preloaded").first["data-preloaded"],
)
end
before do
SiteSetting.chat_enabled = true
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
context "when user is admin" do
it "has correctly loaded preloaded data for enabledPluginAdminRoutes" do
sign_in(admin)
get "/latest"
expect(JSON.parse(preloaded_json["enabledPluginAdminRoutes"])).to include(
{ "label" => "chat.admin.title", "location" => "chat" },
)
end
end
context "when user is not admin" do
it "does not include preloaded data for enabledPluginAdminRoutes" do
sign_in(user)
get "/latest"
expect(preloaded_json["enabledPluginAdminRoutes"]).to eq(nil)
end
end
end

View File

@ -1308,6 +1308,7 @@ RSpec.describe ApplicationController do
"topicTrackingStates", "topicTrackingStates",
"topicTrackingStateMeta", "topicTrackingStateMeta",
"fontMap", "fontMap",
"enabledPluginAdminRoutes",
], ],
) )
end end
@ -1320,6 +1321,11 @@ RSpec.describe ApplicationController do
DiscourseFonts.fonts.filter { |f| f[:variants].present? }.map { |f| f[:key] }, DiscourseFonts.fonts.filter { |f| f[:variants].present? }.map { |f| f[:key] },
) )
end end
it "has correctly loaded enabledPluginAdminRoutes" do
get "/latest"
expect(JSON.parse(preloaded_json["enabledPluginAdminRoutes"])).to eq([])
end
end end
end end
end end