From c066254e67c1ba14af199b4aaed037819df346e5 Mon Sep 17 00:00:00 2001 From: Nathan Marrs Date: Wed, 6 Aug 2025 14:15:55 -0700 Subject: [PATCH] Invite User: Clean up previous experiment (#109214) * remove invite user button from quick add menu as redundant with top nav invite user button * clean up final invite user experiment * remove invite user button from megamenu, clean up and consolidate usage of util functions across top nav and command palette * add some basic tests to util functions * fix tests / jest mock conflicts * address PR feedback --- .github/CODEOWNERS | 1 - .../AppChrome/MegaMenu/MegaMenu.tsx | 13 -- .../AppChrome/QuickAdd/QuickAdd.tsx | 38 +----- .../TopBar/InviteUserButton.test.tsx | 3 + .../AppChrome/TopBar/InviteUserButton.tsx | 45 +++---- .../TopBar/InviteUserButtonUtils.test.tsx | 122 ++++++++++++++++++ .../TopBar/InviteUserButtonUtils.tsx} | 7 +- .../AppChrome/TopBar/SingleTopBar.tsx | 2 +- .../InviteUserButton/InviteUserButton.tsx | 21 --- .../commandPalette/actions/staticActions.ts | 9 +- public/locales/en-US/grafana.json | 2 +- 11 files changed, 162 insertions(+), 101 deletions(-) create mode 100644 public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.test.tsx rename public/app/core/components/{InviteUserButton/utils.ts => AppChrome/TopBar/InviteUserButtonUtils.tsx} (72%) delete mode 100644 public/app/core/components/InviteUserButton/InviteUserButton.tsx diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 350790df440..d9429f2f1d6 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -737,7 +737,6 @@ playwright.storybook.config.ts @grafana/grafana-frontend-platform /public/app/core/components/GraphNG/ @grafana/dataviz-squad /public/app/core/components/help/ @grafana/grafana-search-navigate-organise /public/app/core/components/Indent/ @grafana/grafana-search-navigate-organise -/public/app/core/components/InviteUserButton/ @grafana/sharing-squad /public/app/core/components/Layers/ @grafana/dataviz-squad /public/app/core/components/Login/ @grafana/grafana-search-navigate-organise /public/app/core/components/NativeScrollbar.tsx @grafana/grafana-search-navigate-organise diff --git a/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx b/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx index 1c8b3959230..36955e8d405 100644 --- a/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx +++ b/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx @@ -13,9 +13,6 @@ import { setBookmark } from 'app/core/reducers/navBarTree'; import { usePatchUserPreferencesMutation } from 'app/features/preferences/api/index'; import { useDispatch, useSelector } from 'app/types/store'; -import { InviteUserButton } from '../../InviteUserButton/InviteUserButton'; -import { shouldRenderInviteUserButton } from '../../InviteUserButton/utils'; - import { MegaMenuHeader } from './MegaMenuHeader'; import { MegaMenuItem } from './MegaMenuItem'; import { usePinnedItems } from './hooks'; @@ -128,11 +125,6 @@ export const MegaMenu = memo( ))} - {shouldRenderInviteUserButton && ( -
- -
- )} ); @@ -170,11 +162,6 @@ const getStyles = (theme: GrafanaTheme2) => { width: MENU_WIDTH, }, }), - inviteNewMemberButton: css({ - display: 'flex', - padding: theme.spacing(1.5, 1, 1.5, 1), - borderTop: `1px solid ${theme.colors.border.weak}`, - }), dockMenuButton: css({ display: 'none', position: 'relative', diff --git a/public/app/core/components/AppChrome/QuickAdd/QuickAdd.tsx b/public/app/core/components/AppChrome/QuickAdd/QuickAdd.tsx index 0a9a48ac6ed..3d38c543105 100644 --- a/public/app/core/components/AppChrome/QuickAdd/QuickAdd.tsx +++ b/public/app/core/components/AppChrome/QuickAdd/QuickAdd.tsx @@ -3,10 +3,8 @@ import { useMemo, useState } from 'react'; import { t } from '@grafana/i18n'; import { reportInteraction } from '@grafana/runtime'; import { Menu, Dropdown, ToolbarButton } from '@grafana/ui'; -import { getExternalUserMngLinkUrl } from 'app/features/users/utils'; import { useSelector } from 'app/types/store'; -import { performInviteUserClick, shouldRenderInviteUserButton } from '../../InviteUserButton/utils'; import { NavToolbarSeparator } from '../NavToolbar/NavToolbarSeparator'; import { findCreateActions } from './utils'; @@ -17,23 +15,7 @@ export const QuickAdd = ({}: Props) => { const navBarTree = useSelector((state) => state.navBarTree); const [isOpen, setIsOpen] = useState(false); - const createActions = useMemo(() => { - const actions = findCreateActions(navBarTree); - - if (shouldRenderInviteUserButton) { - actions.push({ - text: t('navigation.invite-user.invite-new-member-button', 'Invite new member'), - url: getExternalUserMngLinkUrl('invite-user-top-bar'), - target: '_blank', - isCreateAction: true, - onClick: () => { - performInviteUserClick('quick_add_button', 'invite-user-quick-add-button'); - }, - }); - } - - return actions; - }, [navBarTree]); + const createActions = useMemo(() => findCreateActions(navBarTree), [navBarTree]); const showQuickAdd = createActions.length > 0; if (!showQuickAdd) { @@ -44,18 +26,12 @@ export const QuickAdd = ({}: Props) => { return ( {createActions.map((createAction, index) => ( -
- {shouldRenderInviteUserButton && index === createActions.length - 1 && } - { - reportInteraction('grafana_menu_item_clicked', { url: createAction.url, from: 'quickadd' }); - createAction.onClick?.(); - }} - /> -
+ reportInteraction('grafana_menu_item_clicked', { url: createAction.url, from: 'quickadd' })} + /> ))} ); diff --git a/public/app/core/components/AppChrome/TopBar/InviteUserButton.test.tsx b/public/app/core/components/AppChrome/TopBar/InviteUserButton.test.tsx index d867c17488b..de2921b3915 100644 --- a/public/app/core/components/AppChrome/TopBar/InviteUserButton.test.tsx +++ b/public/app/core/components/AppChrome/TopBar/InviteUserButton.test.tsx @@ -11,6 +11,9 @@ import { InviteUserButton } from './InviteUserButton'; jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), config: { + featureToggles: { + inviteUserExperimental: true, + }, externalUserMngLinkUrl: 'https://example.com/invite', }, reportInteraction: jest.fn(), diff --git a/public/app/core/components/AppChrome/TopBar/InviteUserButton.tsx b/public/app/core/components/AppChrome/TopBar/InviteUserButton.tsx index deec044c42e..9c86223b026 100644 --- a/public/app/core/components/AppChrome/TopBar/InviteUserButton.tsx +++ b/public/app/core/components/AppChrome/TopBar/InviteUserButton.tsx @@ -1,43 +1,36 @@ import { t } from '@grafana/i18n'; -import { config, reportInteraction } from '@grafana/runtime'; import { ToolbarButton } from '@grafana/ui'; -import { contextSrv } from 'app/core/core'; import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth'; -import { getExternalUserMngLinkUrl } from 'app/features/users/utils'; -import { AccessControlAction } from 'app/types/accessControl'; import { NavToolbarSeparator } from '../NavToolbar/NavToolbarSeparator'; +import { performInviteUserClick, shouldRenderInviteUserButton } from './InviteUserButtonUtils'; + export function InviteUserButton() { const isLargeScreen = useMediaQueryMinWidth('lg'); const handleClick = () => { try { - reportInteraction('invite_user_button_clicked', { - placement: 'top_bar_right', - }); - - const url = getExternalUserMngLinkUrl('invite-user-top-bar'); - window.open(url.toString(), '_blank'); + performInviteUserClick('top_bar_right', 'invite-user-top-bar'); } catch (error) { console.error('Failed to handle invite user click:', error); } }; - const shouldRender = config.externalUserMngLinkUrl && contextSrv.hasPermission(AccessControlAction.OrgUsersAdd); - - return shouldRender ? ( - <> - - {isLargeScreen ? t('navigation.invite-user.invite-button', 'Invite') : undefined} - - - - ) : null; + return ( + shouldRenderInviteUserButton() && ( + <> + + {isLargeScreen ? t('navigation.invite-user.invite-button', 'Invite') : undefined} + + + + ) + ); } diff --git a/public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.test.tsx b/public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.test.tsx new file mode 100644 index 00000000000..b5ac54be8c9 --- /dev/null +++ b/public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.test.tsx @@ -0,0 +1,122 @@ +import type { FeatureToggles } from '@grafana/data'; +import { reportInteraction, config } from '@grafana/runtime'; +import { contextSrv } from 'app/core/core'; +import { getExternalUserMngLinkUrl } from 'app/features/users/utils'; +import { AccessControlAction } from 'app/types/accessControl'; + +import { performInviteUserClick, shouldRenderInviteUserButton } from './InviteUserButtonUtils'; + +// Mock dependencies +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + reportInteraction: jest.fn(), + config: { + featureToggles: {} as Partial, + externalUserMngLinkUrl: '', + }, +})); + +jest.mock('app/core/core', () => ({ + contextSrv: { + hasPermission: jest.fn(), + }, +})); + +jest.mock('app/features/users/utils', () => ({ + getExternalUserMngLinkUrl: jest.fn(), +})); + +const mockReportInteraction = jest.mocked(reportInteraction); +const mockConfig = jest.mocked(config); +const mockContextSrv = jest.mocked(contextSrv); +const mockGetExternalUserMngLinkUrl = jest.mocked(getExternalUserMngLinkUrl); + +// Type assertion to make mockConfig.featureToggles assignable +const mockFeatureToggles = mockConfig.featureToggles as Partial; + +// Mock window.open +const mockWindowOpen = jest.fn(); +Object.defineProperty(window, 'open', { + value: mockWindowOpen, + writable: true, +}); + +describe('InviteUserButtonUtils', () => { + beforeEach(() => { + jest.clearAllMocks(); + + // Set up default mocks + mockFeatureToggles.inviteUserExperimental = true; + mockConfig.externalUserMngLinkUrl = 'https://example.com/invite'; + mockContextSrv.hasPermission.mockReturnValue(true); + + // Dynamic mock that returns URL with the actual cnt parameter + mockGetExternalUserMngLinkUrl.mockImplementation((cnt: string) => `https://example.com/invite?cnt=${cnt}`); + }); + + describe('shouldRenderInviteUserButton', () => { + it('should return true when all conditions are met', () => { + expect(shouldRenderInviteUserButton()).toBe(true); + expect(mockContextSrv.hasPermission).toHaveBeenCalledWith(AccessControlAction.OrgUsersAdd); + }); + + it('should return false when feature toggle is disabled', () => { + mockFeatureToggles.inviteUserExperimental = false; + + expect(shouldRenderInviteUserButton()).toBe(false); + }); + + it('should return false when URL is not configured', () => { + mockConfig.externalUserMngLinkUrl = ''; + + expect(shouldRenderInviteUserButton()).toBeFalsy(); + }); + + it('should return false when user lacks permission', () => { + mockContextSrv.hasPermission.mockReturnValue(false); + + expect(shouldRenderInviteUserButton()).toBe(false); + }); + }); + + describe('performInviteUserClick', () => { + it('should report interaction, get URL, and open window with correct cnt parameter', () => { + const placement = 'top_bar_right'; + const cnt = 'invite-user-test'; + + performInviteUserClick(placement, cnt); + + expect(mockReportInteraction).toHaveBeenCalledWith('invite_user_button_clicked', { + placement, + }); + expect(mockGetExternalUserMngLinkUrl).toHaveBeenCalledWith(cnt); + expect(mockWindowOpen).toHaveBeenCalledWith(`https://example.com/invite?cnt=${cnt}`, '_blank'); + }); + + it('should work with different cnt values', () => { + const placement = 'top_bar_right'; + const cnt = 'dashboard-header'; + + performInviteUserClick(placement, cnt); + + expect(mockReportInteraction).toHaveBeenCalledWith('invite_user_button_clicked', { + placement, + }); + expect(mockGetExternalUserMngLinkUrl).toHaveBeenCalledWith(cnt); + expect(mockWindowOpen).toHaveBeenCalledWith(`https://example.com/invite?cnt=${cnt}`, '_blank'); + }); + + it('should handle special characters in cnt parameter', () => { + const placement = 'menu-item'; + const cnt = 'admin-panel_users-section'; + + performInviteUserClick(placement, cnt); + + expect(mockReportInteraction).toHaveBeenCalledWith('invite_user_button_clicked', { + placement, + }); + expect(mockGetExternalUserMngLinkUrl).toHaveBeenCalledWith(cnt); + expect(mockWindowOpen).toHaveBeenCalledWith(`https://example.com/invite?cnt=${cnt}`, '_blank'); + }); + }); +}); diff --git a/public/app/core/components/InviteUserButton/utils.ts b/public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.tsx similarity index 72% rename from public/app/core/components/InviteUserButton/utils.ts rename to public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.tsx index 6fb774a059b..1e931c90e54 100644 --- a/public/app/core/components/InviteUserButton/utils.ts +++ b/public/app/core/components/AppChrome/TopBar/InviteUserButtonUtils.tsx @@ -1,10 +1,9 @@ -import { reportInteraction } from '@grafana/runtime'; -import { config } from 'app/core/config'; -import { contextSrv } from 'app/core/services/context_srv'; +import { reportInteraction, config } from '@grafana/runtime'; +import { contextSrv } from 'app/core/core'; import { getExternalUserMngLinkUrl } from 'app/features/users/utils'; import { AccessControlAction } from 'app/types/accessControl'; -export const shouldRenderInviteUserButton = +export const shouldRenderInviteUserButton = () => config.featureToggles.inviteUserExperimental && config.externalUserMngLinkUrl && contextSrv.hasPermission(AccessControlAction.OrgUsersAdd); diff --git a/public/app/core/components/AppChrome/TopBar/SingleTopBar.tsx b/public/app/core/components/AppChrome/TopBar/SingleTopBar.tsx index e5033c75c1c..f2b02f35bd9 100644 --- a/public/app/core/components/AppChrome/TopBar/SingleTopBar.tsx +++ b/public/app/core/components/AppChrome/TopBar/SingleTopBar.tsx @@ -107,7 +107,7 @@ export const SingleTopBar = memo(function SingleTopBar({ {config.featureToggles.extensionSidebar && !isSmallScreen && } {!showToolbarLevel && actions} {!contextSrv.user.isSignedIn && } - {config.featureToggles.inviteUserExperimental && } + {profileNode && } diff --git a/public/app/core/components/InviteUserButton/InviteUserButton.tsx b/public/app/core/components/InviteUserButton/InviteUserButton.tsx deleted file mode 100644 index 6d6b5b620fd..00000000000 --- a/public/app/core/components/InviteUserButton/InviteUserButton.tsx +++ /dev/null @@ -1,21 +0,0 @@ -import { t } from '@grafana/i18n'; -import { Button } from '@grafana/ui'; - -import { performInviteUserClick } from './utils'; - -export function InviteUserButton() { - return ( - - ); -} diff --git a/public/app/features/commandPalette/actions/staticActions.ts b/public/app/features/commandPalette/actions/staticActions.ts index e7e8c8d6734..b70c5ee4f1c 100644 --- a/public/app/features/commandPalette/actions/staticActions.ts +++ b/public/app/features/commandPalette/actions/staticActions.ts @@ -3,7 +3,10 @@ import { useMemo } from 'react'; import { NavModelItem } from '@grafana/data'; import { t } from '@grafana/i18n'; import { enrichHelpItem } from 'app/core/components/AppChrome/MegaMenu/utils'; -import { performInviteUserClick, shouldRenderInviteUserButton } from 'app/core/components/InviteUserButton/utils'; +import { + shouldRenderInviteUserButton, + performInviteUserClick, +} from 'app/core/components/AppChrome/TopBar/InviteUserButtonUtils'; import { changeTheme } from 'app/core/services/theme'; import { currentMockApiState, toggleMockApiAndReload, togglePseudoLocale } from 'app/dev-utils'; import { useSelector } from 'app/types/store'; @@ -139,10 +142,10 @@ export function useStaticActions(): CommandPaletteAction[] { return useMemo(() => { const navBarActions = navTreeToActions(navBarTree); - if (shouldRenderInviteUserButton) { + if (shouldRenderInviteUserButton()) { navBarActions.push({ id: 'invite-user', - name: t('navigation.invite-user.invite-new-member-button', 'Invite new member'), + name: t('navigation.invite-user.invite-new-user-button', 'Invite new user'), section: t('command-palette.section.actions', 'Actions'), priority: ACTIONS_PRIORITY, perform: () => { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index fb7dfd15732..2c040613239 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -10315,7 +10315,7 @@ }, "invite-user": { "invite-button": "Invite", - "invite-new-member-button": "Invite new member", + "invite-new-user-button": "Invite new user", "invite-tooltip": "Invite user" }, "item": {