Skip to content

Commit d59b616

Browse files
authored
fix(global-header): updated menu link display logic (#1425)
* fix(global-header): updated menu link display logic Signed-off-by: Yi Cai <yicai@redhat.com> * chore(global-header): update API report Signed-off-by: Yi Cai <yicai@redhat.com> * chore(global-header): correct API report (and test report if present) Signed-off-by: Yi Cai <yicai@redhat.com> * updated report.api.md Signed-off-by: Yi Cai <yicai@redhat.com> * updated report.api.md Signed-off-by: Yi Cai <yicai@redhat.com> * revert back as no real changes in this file Signed-off-by: Yi Cai <yicai@redhat.com> --------- Signed-off-by: Yi Cai <yicai@redhat.com>
1 parent 47fd25f commit d59b616

6 files changed

Lines changed: 114 additions & 14 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-global-header': patch
3+
---
4+
5+
Add `type` prop for MenuItemLink identification and hide "My profile" menuItemLink for guest users
6+
7+
- Add `type` prop to MenuItemLink component config for better identification when i18n applied
8+
- Hide "My Profile" menu item when user entity reference contains '/guest'
9+
- Hide "My Profile" menu item when catalog API fails to fetch user entity
10+
- Export `globalHeaderTranslations` from plugin for i18n integration

workspaces/global-header/plugins/global-header/src/components/HeaderDropdownComponent/ProfileDropdown.test.tsx

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ jest.mock('../../hooks/useProfileDropdownMountPoints', () => {
5959
props: {
6060
icon: 'account',
6161
title: 'profile.myProfile',
62+
type: 'myProfile',
6263
},
6364
priority: 150,
6465
},
@@ -187,4 +188,73 @@ describe('ProfileDropdown', () => {
187188
);
188189
});
189190
});
191+
192+
it('should hide My Profile menu item for guest users', async () => {
193+
setUserProfileMock({
194+
displayName: 'Guest',
195+
userEntityRef: 'user:development/guest',
196+
});
197+
198+
const catalogApi = createMockCatalogApi({
199+
metadata: {
200+
name: 'guest',
201+
title: 'Guest',
202+
},
203+
spec: {
204+
profile: { displayName: 'Guest' },
205+
},
206+
});
207+
208+
await renderComponent(catalogApi);
209+
const profileButton = screen.getByRole('button', {
210+
name: /Profile picture Guest/i,
211+
});
212+
fireEvent.click(profileButton);
213+
214+
await waitFor(() => {
215+
// Should find Settings menu item
216+
const settingsLink = screen.getByRole('menuitem', {
217+
name: /Settings/i,
218+
});
219+
expect(settingsLink).toBeInTheDocument();
220+
221+
// Should NOT find My Profile menu item for guest users
222+
const myProfileLink = screen.queryByRole('menuitem', {
223+
name: /My profile/i,
224+
});
225+
expect(myProfileLink).not.toBeInTheDocument();
226+
});
227+
});
228+
229+
it('should hide My Profile menu item when catalog API fails', async () => {
230+
setUserProfileMock({
231+
displayName: 'Test User',
232+
userEntityRef: 'user:default/test-user',
233+
});
234+
235+
// Mock catalog API to throw an error (user not found in catalog)
236+
const catalogApi = {
237+
getEntityByRef: jest.fn().mockRejectedValue(new Error('User not found')),
238+
} as unknown as CatalogApi;
239+
240+
await renderComponent(catalogApi);
241+
const profileButton = screen.getByRole('button', {
242+
name: /Profile picture Test User/i,
243+
});
244+
fireEvent.click(profileButton);
245+
246+
await waitFor(() => {
247+
// Should find Settings menu item
248+
const settingsLink = screen.getByRole('menuitem', {
249+
name: /Settings/i,
250+
});
251+
expect(settingsLink).toBeInTheDocument();
252+
253+
// Should NOT find My Profile menu item when catalog API fails
254+
const myProfileLink = screen.queryByRole('menuitem', {
255+
name: /My profile/i,
256+
});
257+
expect(myProfileLink).not.toBeInTheDocument();
258+
});
259+
});
190260
});

workspaces/global-header/plugins/global-header/src/components/HeaderDropdownComponent/ProfileDropdown.tsx

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ export const ProfileDropdown = ({ layout }: ProfileDropdownProps) => {
100100
setUser(null);
101101
setProfileLink(null);
102102
}
103-
} catch (_err) {
103+
} catch (err) {
104+
// User entity doesn't exist in catalog (e.g., guest user)
104105
setUser(null);
105106
setProfileLink(null);
106107
}
@@ -110,25 +111,36 @@ export const ProfileDropdown = ({ layout }: ProfileDropdownProps) => {
110111
}, [backstageIdentity, catalogApi]);
111112

112113
const menuItems = useMemo(() => {
114+
// Check if user is a guest (guest user has userEntityRef like "user:development/guest" or "user:default/guest")
115+
const isGuestUser =
116+
backstageIdentity?.userEntityRef?.includes('/guest') ||
117+
profileLink === null;
118+
113119
return (profileDropdownMountPoints ?? [])
114120
.map(mp => {
115121
const {
116122
title = '',
117123
icon = '',
118124
link: staticLink = '',
125+
type = '',
119126
} = mp.config?.props ?? {};
120-
const isMyProfile = title === 'profile.myProfile';
127+
const isMyProfile = type === 'myProfile';
121128
const link = isMyProfile ? profileLink ?? '' : staticLink;
122129

123-
if (!link && title) {
130+
// Hide "My Profile" for guest users or when user doesn't exist in catalog
131+
if (isMyProfile && isGuestUser) {
132+
return null;
133+
}
134+
135+
// Hide items without links (but allow "My Profile" to pass through for authenticated users)
136+
if (!link && title && !isMyProfile) {
124137
return null;
125138
}
126139

127140
// Check if title looks like a translation key (contains dots)
128-
const translatedTitle =
129-
title && title.includes('.')
130-
? t(title as any, {}) || title // Fallback to original title if translation fails
131-
: title;
141+
const translatedTitle = title?.includes('.')
142+
? t(title as any, {}) || title // Fallback to original title if translation fails
143+
: title;
132144

133145
return {
134146
Component: mp.Component,
@@ -140,7 +152,7 @@ export const ProfileDropdown = ({ layout }: ProfileDropdownProps) => {
140152
})
141153
.filter((item: MenuItemConfig) => item !== null)
142154
.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));
143-
}, [profileDropdownMountPoints, profileLink, t]);
155+
}, [profileDropdownMountPoints, profileLink, backstageIdentity, t]);
144156

145157
if (menuItems.length === 0) {
146158
return null;

workspaces/global-header/plugins/global-header/src/components/MenuItemLink/MenuItemLink.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ export const MenuItemLink = ({
3838
tooltip,
3939
}: MenuItemLinkProps) => {
4040
const { t } = useTranslation();
41-
const isExternalLink =
42-
to?.startsWith('http://') || to?.startsWith('https://');
41+
const isExternalLink = Boolean(
42+
to && (to.startsWith('http://') || to.startsWith('https://')),
43+
);
4344

4445
// Check if title looks like a translation key (contains dots)
45-
const translatedTitle =
46-
title && title.includes('.')
47-
? t(title as any, {}) || title // Fallback to original title if translation fails
48-
: title;
46+
const translatedTitle = title?.includes('.')
47+
? t(title as any, {}) || title // Fallback to original title if translation fails
48+
: title;
4949

5050
const headerLinkContent = () => (
5151
<MenuItemLinkContent

workspaces/global-header/plugins/global-header/src/defaultMountPoints/defaultMountPoints.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ export const defaultProfileDropdownMountPoints: ProfileDropdownMountPoint[] = [
159159
props: {
160160
title: 'profile.myProfile',
161161
icon: 'account',
162+
type: 'myProfile', // Semantic identifier
162163
},
163164
},
164165
},

workspaces/global-header/plugins/global-header/src/plugin.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,3 +414,10 @@ export const CompanyLogo = globalHeaderPlugin.provide(
414414
},
415415
}),
416416
);
417+
418+
/**
419+
* Translation resource for the global header plugin
420+
*
421+
* @public
422+
*/
423+
export { globalHeaderTranslations } from './translations';

0 commit comments

Comments
 (0)