From 95014e9ab85d64d1a6526752aceeab5a36752f18 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 21 Feb 2024 12:58:31 +1000 Subject: [PATCH] FIX: Do not duplicate admin sidebar plugin links (#25780) When hiding/showing the sidebar, as is the case on mobile and using the toggle in the top left on desktop, we delete and recreate the ember component on the page. This causes the `sections` for each sidebar panel to get re-evaluated every time. For the admin sidebar, this means that we were constantly re-adding the plugin links to the sidebar, causing duplication. This can be fixed by just adding @cached to the getter for sections. --- .../discourse/app/lib/sidebar/admin-sidebar.js | 2 ++ .../spec/system/admin_sidebar_navigation_spec.rb | 8 ++++++++ .../components/navigation_menu/base.rb | 14 +++++++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js b/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js index 5d7a221d047..c575aea55ac 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js @@ -1,3 +1,4 @@ +import { cached } from "@glimmer/tracking"; import PreloadStore from "discourse/lib/preload-store"; import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map"; import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel"; @@ -213,6 +214,7 @@ export default class AdminSidebarPanel extends BaseCustomSidebarPanel { key = ADMIN_PANEL; hidden = true; + @cached get sections() { const currentUser = getOwnerWithFallback().lookup("service:current-user"); const siteSettings = getOwnerWithFallback().lookup("service:site-settings"); diff --git a/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb index e79f6e955e9..4cbe806a2f7 100644 --- a/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb +++ b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb @@ -14,4 +14,12 @@ describe "Admin Revamp | Sidebar Navigation | Plugin Links", type: :system do visit("/admin") expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat") end + + it "does not duplicate links to enabled plugin admin routes when showing and hiding sidebar" do + visit("/admin") + expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat", count: 1) + find(".header-sidebar-toggle").click + find(".header-sidebar-toggle").click + expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat", count: 1) + end end diff --git a/spec/system/page_objects/components/navigation_menu/base.rb b/spec/system/page_objects/components/navigation_menu/base.rb index 890af45926b..4e335595e61 100644 --- a/spec/system/page_objects/components/navigation_menu/base.rb +++ b/spec/system/page_objects/components/navigation_menu/base.rb @@ -36,8 +36,15 @@ module PageObjects has_css?(".#{SIDEBAR_SECTION_LINK_SELECTOR}--active", count: 1) end - def has_section_link?(name, href: nil, active: false, target: nil) - section_link_present?(name, href: href, active: active, target: target, present: true) + def has_section_link?(name, href: nil, active: false, target: nil, count: 1) + section_link_present?( + name, + href: href, + active: active, + target: target, + present: true, + count: count, + ) end def has_no_section_link?(name, href: nil, active: false) @@ -163,12 +170,13 @@ module PageObjects private - def section_link_present?(name, href: nil, active: false, target: nil, present:) + def section_link_present?(name, href: nil, active: false, target: nil, count: 1, present:) attributes = { exact_text: name } attributes[:href] = href if href attributes[:class] = SIDEBAR_SECTION_LINK_SELECTOR attributes[:class] += "--active" if active attributes[:target] = target if target + attributes[:count] = count page.public_send(present ? :has_link? : :has_no_link?, **attributes) end