From afd0aa6f3bf306a397fa8bd9eade95d6eb8d6b35 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Mon, 4 May 2026 16:43:34 +1000 Subject: [PATCH] PM-4988: Hide project header edit for copilots What was broken The earlier PM-4988 fix hid project edit affordances in the projects list and blocked the edit route, but copilot users could still see the pencil edit link in project-scoped page headers such as Challenges, Users, and Assets. Root cause Those header title actions still used the broader project management permission check. Copilot project membership is allowed by that helper for other write flows, but project detail editing requires the narrower Full Access edit check. What was changed Updated the project header edit links on the Challenges, Users, and Assets tabs to render only when the loaded project passes checkCanEditProjectDetails. Kept the existing broader manage permission for non-detail workflows such as member, asset, challenge, and billing notice behavior. Any added/updated tests Added page coverage for hiding the project header edit link when a copilot can manage the project but cannot edit project details, and for keeping the link visible when detail editing is allowed. --- .../ProjectAssetsPage.spec.tsx | 68 +++++++++++++++++++ .../ProjectAssetsPage/ProjectAssetsPage.tsx | 13 +++- .../ChallengesListPage.spec.tsx | 58 ++++++++++++++++ .../ChallengesListPage/ChallengesListPage.tsx | 36 +++++++++- .../UsersManagementPage.spec.tsx | 52 +++++++++++++- .../UsersManagementPage.tsx | 13 +++- 6 files changed, 231 insertions(+), 9 deletions(-) diff --git a/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.spec.tsx b/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.spec.tsx index 3cd56360b..4a53baa5c 100644 --- a/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.spec.tsx +++ b/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.spec.tsx @@ -4,6 +4,7 @@ import type { Context, PropsWithChildren, ReactNode } from 'react' import { render, screen, + within, } from '@testing-library/react' import { MemoryRouter, Route, Routes } from 'react-router-dom' @@ -13,6 +14,10 @@ import { useFetchProjectAttachments, useFetchProjectMembers, } from '../../../lib/hooks' +import { + checkCanEditProjectDetails, + checkCanManageProject, +} from '../../../lib/utils' import { ProjectAssetsPage } from './ProjectAssetsPage' @@ -116,12 +121,15 @@ jest.mock('../../../lib/services', () => ({ updateProjectAttachment: jest.fn(), })) jest.mock('../../../lib/utils', () => ({ + checkCanEditProjectDetails: jest.fn(() => false), checkCanManageProject: jest.fn(() => false), })) const mockedUseFetchProject = useFetchProject as jest.Mock const mockedUseFetchProjectAttachments = useFetchProjectAttachments as jest.Mock const mockedUseFetchProjectMembers = useFetchProjectMembers as jest.Mock +const mockedCheckCanEditProjectDetails = checkCanEditProjectDetails as jest.Mock +const mockedCheckCanManageProject = checkCanManageProject as jest.Mock const defaultContextValue: WorkAppContextModel = { isAdmin: true, @@ -160,6 +168,8 @@ function renderPage( describe('ProjectAssetsPage', () => { beforeEach(() => { jest.clearAllMocks() + mockedCheckCanEditProjectDetails.mockReturnValue(false) + mockedCheckCanManageProject.mockReturnValue(false) mockedUseFetchProject.mockReturnValue({ error: undefined, @@ -201,4 +211,62 @@ describe('ProjectAssetsPage', () => { expect(screen.getByText('Assets Library')) .toBeTruthy() }) + + it('hides project edit action when a copilot can manage but cannot edit project details', () => { + mockedCheckCanManageProject.mockReturnValue(true) + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + mutate: jest.fn(), + project: { + id: 200, + members: [ + { + role: 'copilot', + userId: 12345, + }, + ], + name: 'Copilot Project', + }, + }) + + renderPage('/projects/200/assets', { + ...defaultContextValue, + isAdmin: false, + isCopilot: true, + loginUserInfo: { + email: 'copilot@example.com', + exp: 0, + handle: 'copilot-user', + iat: 0, + roles: ['copilot'], + userId: 12345, + } as WorkAppContextModel['loginUserInfo'], + userRoles: ['copilot'], + }) + + expect(within(screen.getByTestId('page-title-action')) + .queryByRole('link', { name: 'Edit project' })) + .toBeNull() + }) + + it('shows project edit action when project detail editing is allowed', () => { + mockedCheckCanEditProjectDetails.mockReturnValue(true) + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + mutate: jest.fn(), + project: { + id: 200, + name: 'Payment Testing', + }, + }) + + renderPage('/projects/200/assets') + + expect(within(screen.getByTestId('page-title-action')) + .getByRole('link', { name: 'Edit project' }) + .getAttribute('href')) + .toBe('/projects/200/edit') + }) }) diff --git a/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.tsx b/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.tsx index b5c63b441..4f486ac03 100644 --- a/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.tsx +++ b/src/apps/work/src/pages/assets/ProjectAssetsPage/ProjectAssetsPage.tsx @@ -58,7 +58,10 @@ import { removeProjectAttachment, updateProjectAttachment, } from '../../../lib/services' -import { checkCanManageProject } from '../../../lib/utils' +import { + checkCanEditProjectDetails, + checkCanManageProject, +} from '../../../lib/utils' import styles from './ProjectAssetsPage.module.scss' @@ -324,6 +327,12 @@ export const ProjectAssetsPage: FC = () => { workAppContext.loginUserInfo?.userId, projectResult.project, ) + const canEditProjectDetails = !!projectResult.project + && checkCanEditProjectDetails( + workAppContext.userRoles, + workAppContext.loginUserInfo?.userId, + projectResult.project, + ) const [activeTab, setActiveTab] = useState('files') const [isOpeningPicker, setIsOpeningPicker] = useState(false) @@ -749,7 +758,7 @@ export const ProjectAssetsPage: FC = () => { const titleAction = projectId ? (
- {canManageProject + {canEditProjectDetails ? ( ({ jest.mock('../../../lib/utils', () => ({ buildProjectLandingPath: jest.fn((project: { id?: string | number }) => `/projects/${project.id}/challenges`), canCreateEngagement: jest.fn(() => false), + checkCanEditProjectDetails: jest.fn(() => false), checkCanManageProject: jest.fn(() => false), checkProjectAccess: jest.fn(( _userRoles: string[], @@ -148,6 +150,7 @@ const mockedUseFetchChallengeTypes = useFetchChallengeTypes as jest.Mock const mockedUseFetchProject = useFetchProject as jest.Mock const mockedUseFetchProjects = useFetchProjects as jest.Mock const mockedCanCreateEngagement = canCreateEngagement as jest.Mock +const mockedCheckCanEditProjectDetails = checkCanEditProjectDetails as jest.Mock const mockedGetAuthAccessToken = getAuthAccessToken as jest.Mock const defaultContextValue: WorkAppContextModel = { @@ -194,6 +197,7 @@ describe('ChallengesListPage', () => { jest.clearAllMocks() mockedCanCreateEngagement.mockReturnValue(false) + mockedCheckCanEditProjectDetails.mockReturnValue(false) mockedCheckProjectAccess.mockImplementation(( _userRoles: string[], _userId: number | string | undefined, @@ -292,6 +296,60 @@ describe('ChallengesListPage', () => { .toBe(true) }) + it('hides project edit action when a copilot can manage but cannot edit project details', () => { + const project = { + id: 200, + members: [ + { + role: 'copilot', + userId: 12345, + }, + ], + name: 'Authorized Project', + status: 'active', + } + + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + project, + }) + + renderPage('/projects/200/challenges', '/projects/:projectId/challenges') + + expect(mockedCheckCanEditProjectDetails) + .toHaveBeenCalledWith(['copilot'], 12345, project) + expect(within(screen.getByTestId('page-title-action')) + .queryByRole('link', { name: 'Edit project' })) + .toBeNull() + }) + + it('shows project edit action when project detail editing is allowed', () => { + mockedCheckCanEditProjectDetails.mockReturnValue(true) + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + project: { + id: 200, + members: [ + { + role: 'manager', + userId: 12345, + }, + ], + name: 'Authorized Project', + status: 'active', + }, + }) + + renderPage('/projects/200/challenges', '/projects/:projectId/challenges') + + expect(within(screen.getByTestId('page-title-action')) + .getByRole('link', { name: 'Edit project' }) + .getAttribute('href')) + .toBe('/projects/200/edit') + }) + it('waits for project access before fetching project challenges', () => { mockedUseFetchProject.mockReturnValue({ error: undefined, diff --git a/src/apps/work/src/pages/challenges/ChallengesListPage/ChallengesListPage.tsx b/src/apps/work/src/pages/challenges/ChallengesListPage/ChallengesListPage.tsx index b80b4367a..c480172ff 100644 --- a/src/apps/work/src/pages/challenges/ChallengesListPage/ChallengesListPage.tsx +++ b/src/apps/work/src/pages/challenges/ChallengesListPage/ChallengesListPage.tsx @@ -60,6 +60,7 @@ import { import { buildProjectLandingPath, canCreateEngagement, + checkCanEditProjectDetails, checkCanManageProject, checkProjectAccess, getAuthAccessToken, @@ -187,7 +188,7 @@ function renderContextualActions(params: RenderContextualActionsParams): JSX.Ele } interface RenderProjectTitleActionParams { - canManageProject: boolean + canEditProjectDetails: boolean projectId: string | undefined projectStatus: ProjectStatusValue | undefined } @@ -202,7 +203,7 @@ function renderProjectTitleAction(params: RenderProjectTitleActionParams): JSX.E {params.projectStatus ? : undefined} - {params.canManageProject + {params.canEditProjectDetails ? ( { : 'Challenges' const canManageProject = !!projectResult.project && checkCanManageProject(userRoles, loginUserInfo?.userId, projectResult.project) + const canEditProjectDetails = canRenderProjectDetailsEditAction( + userRoles, + loginUserInfo?.userId, + projectResult.project, + ) const isProjectActive = String(projectResult.project?.status || '') .trim() .toLowerCase() === PROJECT_STATUS.ACTIVE @@ -710,7 +740,7 @@ export const ChallengesListPage: FC = () => { }) const titleAction = renderProjectTitleAction({ - canManageProject, + canEditProjectDetails, projectId: projectIdFromRoute, projectStatus: projectResult.project?.status, }) diff --git a/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.spec.tsx b/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.spec.tsx index 39d056ca2..2030bd089 100644 --- a/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.spec.tsx +++ b/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.spec.tsx @@ -13,7 +13,10 @@ import { useFetchProject, useFetchProjectMembers, } from '../../../lib/hooks' -import { checkCanManageProject } from '../../../lib/utils' +import { + checkCanEditProjectDetails, + checkCanManageProject, +} from '../../../lib/utils' import { UsersManagementPage } from './UsersManagementPage' @@ -95,6 +98,7 @@ jest.mock('../../../lib/services', () => ({ removeMemberFromProject: jest.fn(), })) jest.mock('../../../lib/utils', () => ({ + checkCanEditProjectDetails: jest.fn(() => true), checkCanManageProject: jest.fn(() => true), showErrorToast: jest.fn(), showSuccessToast: jest.fn(), @@ -102,6 +106,7 @@ jest.mock('../../../lib/utils', () => ({ const mockedUseFetchProject = useFetchProject as jest.Mock const mockedUseFetchProjectMembers = useFetchProjectMembers as jest.Mock +const mockedCheckCanEditProjectDetails = checkCanEditProjectDetails as jest.Mock const mockedCheckCanManageProject = checkCanManageProject as jest.Mock const defaultContextValue: WorkAppContextModel = { @@ -141,6 +146,7 @@ function renderPage( describe('UsersManagementPage', () => { beforeEach(() => { jest.clearAllMocks() + mockedCheckCanEditProjectDetails.mockReturnValue(true) mockedCheckCanManageProject.mockReturnValue(true) mockedUseFetchProject.mockReturnValue({ @@ -212,6 +218,7 @@ describe('UsersManagementPage', () => { }) it('hides member management actions when a global manager role cannot manage the project', () => { + mockedCheckCanEditProjectDetails.mockReturnValue(false) mockedCheckCanManageProject.mockReturnValue(false) mockedUseFetchProject.mockReturnValue({ error: undefined, @@ -252,4 +259,47 @@ describe('UsersManagementPage', () => { .queryByRole('link', { name: 'Edit project' })) .toBeNull() }) + + it('hides project edit action when a copilot can manage but cannot edit project details', () => { + mockedCheckCanEditProjectDetails.mockReturnValue(false) + mockedCheckCanManageProject.mockReturnValue(true) + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + mutate: jest.fn(), + project: { + id: 200, + members: [ + { + role: 'copilot', + userId: 12345, + }, + ], + name: 'Copilot Project', + status: 'active', + }, + }) + + renderPage('/projects/200/users', { + ...defaultContextValue, + isAdmin: false, + isCopilot: true, + loginUserInfo: { + email: 'copilot@example.com', + exp: 0, + handle: 'copilot-user', + iat: 0, + roles: ['copilot'], + userId: 12345, + } as WorkAppContextModel['loginUserInfo'], + userRoles: ['copilot'], + }) + + expect(within(screen.getByTestId('page-right-header')) + .getByRole('button', { name: 'Add User' })) + .toBeTruthy() + expect(within(screen.getByTestId('page-title-action')) + .queryByRole('link', { name: 'Edit project' })) + .toBeNull() + }) }) diff --git a/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.tsx b/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.tsx index 8b029e7be..8fe7e5b16 100644 --- a/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.tsx +++ b/src/apps/work/src/pages/users/UsersManagementPage/UsersManagementPage.tsx @@ -38,6 +38,7 @@ import { removeMemberFromProject, } from '../../../lib/services' import { + checkCanEditProjectDetails, checkCanManageProject, showErrorToast, showSuccessToast, @@ -55,7 +56,7 @@ function toOptionalString(value: unknown): string | undefined { } interface RenderProjectTitleActionParams { - canManageProject: boolean + canEditProjectDetails: boolean projectId: string | undefined projectStatus: ProjectStatusValue | undefined } @@ -70,7 +71,7 @@ function renderProjectTitleAction(params: RenderProjectTitleActionParams): JSX.E {params.projectStatus ? : undefined} - {params.canManageProject + {params.canEditProjectDetails ? ( { loginUserInfo?.userId, projectResult.project, ) + const canEditProjectDetails = !!projectResult.project + && checkCanEditProjectDetails( + userRoles, + loginUserInfo?.userId, + projectResult.project, + ) const canManageMembers = canManageProject const hasMembers = members.length > 0 @@ -219,7 +226,7 @@ export const UsersManagementPage: FC = () => { setShowInviteUserModal(false) }, []) const titleAction = renderProjectTitleAction({ - canManageProject, + canEditProjectDetails, projectId, projectStatus: projectResult.project?.status, })