From 315c146fe4184f239452635ad3acff1594d071d6 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Mon, 6 Jan 2025 17:50:52 -0500 Subject: [PATCH] [AN-149] Add ability to edit namespace permissions (#5203) --- src/libs/ajax/methods/Methods.ts | 13 ++ .../providers/PermissionsProvider.test.ts | 145 ++++++++++++ .../methods/providers/PermissionsProvider.ts | 51 ++++ src/pages/workflows/WorkflowList.test.tsx | 78 ++++++- src/pages/workflows/WorkflowList.tsx | 186 +++++++++------ .../workflow-details/WorkflowWrapper.test.tsx | 2 +- .../workflow-details/WorkflowWrapper.ts | 4 +- .../modals/PermissionsModal.test.tsx | 220 ++++++++++-------- src/workflows/modals/PermissionsModal.tsx | 148 ++++++------ 9 files changed, 606 insertions(+), 241 deletions(-) create mode 100644 src/libs/ajax/methods/providers/PermissionsProvider.test.ts create mode 100644 src/libs/ajax/methods/providers/PermissionsProvider.ts diff --git a/src/libs/ajax/methods/Methods.ts b/src/libs/ajax/methods/Methods.ts index 9ef1f4e902..7609f443b8 100644 --- a/src/libs/ajax/methods/Methods.ts +++ b/src/libs/ajax/methods/Methods.ts @@ -47,6 +47,19 @@ export const Methods = (signal?: AbortSignal) => ({ return res.json(); }, + getNamespacePermissions: async (namespace: string): Promise => { + const res = await fetchOrchestration(`api/methods/${namespace}/permissions`, _.merge(authOpts(), { signal })); + return res.json(); + }, + + setNamespacePermissions: async (namespace: string, payload: MethodConfigACL): Promise => { + const res = await fetchOrchestration( + `api/methods/${namespace}/permissions`, + _.mergeAll([authOpts(), jsonBody(payload), { signal, method: 'POST' }]) + ); + return res.json(); + }, + method: (namespace, name, snapshotId) => { const root = `methods/${namespace}/${name}/${snapshotId}`; diff --git a/src/libs/ajax/methods/providers/PermissionsProvider.test.ts b/src/libs/ajax/methods/providers/PermissionsProvider.test.ts new file mode 100644 index 0000000000..f6ee2d67b4 --- /dev/null +++ b/src/libs/ajax/methods/providers/PermissionsProvider.test.ts @@ -0,0 +1,145 @@ +import { asMockedFn, partial } from '@terra-ui-packages/test-utils'; +import { Methods, MethodsAjaxContract } from 'src/libs/ajax/methods/Methods'; +import { + namespacePermissionsProvider, + snapshotPermissionsProvider, +} from 'src/libs/ajax/methods/providers/PermissionsProvider'; +import { WorkflowsPermissions } from 'src/workflows/workflows-acl-utils'; + +jest.mock('src/libs/ajax/methods/Methods'); + +const mockPermissions: WorkflowsPermissions = [ + { + role: 'OWNER', + user: 'groot@gmail.com', + }, + { + role: 'READER', + user: 'rocket.racoon@gmail.com', + }, +]; + +const updatedMockPermissions: WorkflowsPermissions = [ + { + role: 'OWNER', + user: 'groot@gmail.com', + }, + { + role: 'OWNER', + user: 'rocket.racoon@gmail.com', + }, +]; + +type MethodsNamespaceAjaxNeeds = Pick; +type MethodObjContract = MethodsAjaxContract['method']; + +const mockSnapshotAjax = () => { + const mockGetSnapshotPermissions = jest.fn().mockReturnValue(mockPermissions); + const mockSetSnapshotPermissions = jest.fn().mockReturnValue(updatedMockPermissions); + asMockedFn(Methods).mockReturnValue({ + method: jest.fn(() => { + return partial>({ + permissions: mockGetSnapshotPermissions, + setPermissions: mockSetSnapshotPermissions, + }); + }) as MethodObjContract, + } as MethodsAjaxContract); +}; + +const mockNamespaceAjax = (): MethodsNamespaceAjaxNeeds => { + const partialMethods: MethodsNamespaceAjaxNeeds = { + getNamespacePermissions: jest.fn().mockReturnValue(mockPermissions), + setNamespacePermissions: jest.fn().mockReturnValue(updatedMockPermissions), + }; + asMockedFn(Methods).mockReturnValue(partial(partialMethods)); + + return partialMethods; +}; + +describe('PermissionsProvider', () => { + it('handles get snapshot permissions call', async () => { + // Arrange + mockSnapshotAjax(); + const signal = new window.AbortController().signal; + + // Act + const result = await snapshotPermissionsProvider('groot-method', 3).getPermissions('groot-namespace', { signal }); + + // Assert + expect(Methods).toBeCalledTimes(1); + expect(Methods().method).toHaveBeenCalledWith('groot-namespace', 'groot-method', 3); + expect(Methods().method('groot-namespace', 'groot-method', 3).permissions).toHaveBeenCalled(); + expect(result).toEqual(mockPermissions); + }); + + it('handles set snapshot permissions call', async () => { + // Arrange + mockSnapshotAjax(); + const signal = new window.AbortController().signal; + + // Act + const result = await snapshotPermissionsProvider('groot-method', 3).updatePermissions( + 'groot-namespace', + [ + { + role: 'OWNER', + user: 'rocket.racoon@gmail.com', + }, + ], + { signal } + ); + + // Assert + expect(Methods).toBeCalledTimes(1); + expect(Methods().method).toHaveBeenCalledWith('groot-namespace', 'groot-method', 3); + expect(Methods().method('groot-namespace', 'groot-method', 3).setPermissions).toHaveBeenCalledWith([ + { + role: 'OWNER', + user: 'rocket.racoon@gmail.com', + }, + ]); + expect(result).toEqual(updatedMockPermissions); + }); + + it('handles get namespace permissions call', async () => { + // Arrange + mockNamespaceAjax(); + const signal = new window.AbortController().signal; + + // Act + const result = await namespacePermissionsProvider.getPermissions('groot-namespace', { signal }); + + // Assert + expect(Methods).toBeCalledTimes(1); + expect(Methods().getNamespacePermissions).toHaveBeenCalledWith('groot-namespace'); + expect(result).toEqual(mockPermissions); + }); + + it('handles set namespace permissions call', async () => { + // Arrange + mockNamespaceAjax(); + const signal = new window.AbortController().signal; + + // Act + const result = await namespacePermissionsProvider.updatePermissions( + 'groot-namespace', + [ + { + role: 'OWNER', + user: 'rocket.racoon@gmail.com', + }, + ], + { signal } + ); + + // Assert + expect(Methods).toBeCalledTimes(1); + expect(Methods().setNamespacePermissions).toHaveBeenCalledWith('groot-namespace', [ + { + role: 'OWNER', + user: 'rocket.racoon@gmail.com', + }, + ]); + expect(result).toEqual(updatedMockPermissions); + }); +}); diff --git a/src/libs/ajax/methods/providers/PermissionsProvider.ts b/src/libs/ajax/methods/providers/PermissionsProvider.ts new file mode 100644 index 0000000000..278d0d4798 --- /dev/null +++ b/src/libs/ajax/methods/providers/PermissionsProvider.ts @@ -0,0 +1,51 @@ +import { AbortOption } from '@terra-ui-packages/data-client-core'; +import { Methods } from 'src/libs/ajax/methods/Methods'; +import { MethodConfigACL } from 'src/libs/ajax/methods/methods-models'; + +export interface PermissionsProvider { + getPermissions: (methodNamespace: string, options?: AbortOption) => Promise; + updatePermissions: ( + methodNamespace: string, + updatedPermissions: MethodConfigACL, + options?: AbortOption + ) => Promise; +} + +/** + * Permissions provider to fetch and update the namespace permissions in Broad Methods Repository + */ +export const namespacePermissionsProvider: PermissionsProvider = { + getPermissions: async (methodNamespace: string, options: AbortOption = {}): Promise => { + const { signal } = options; + return await Methods(signal).getNamespacePermissions(methodNamespace); + }, + + updatePermissions: async ( + methodNamespace: string, + updatedPermissions: MethodConfigACL, + options: AbortOption = {} + ): Promise => { + const { signal } = options; + return await Methods(signal).setNamespacePermissions(methodNamespace, updatedPermissions); + }, +}; + +/** + * Permissions provider to fetch and update the method snapshot permissions in Broad Methods Repository + */ +export const snapshotPermissionsProvider = (methodName: string, snapshotId: number): PermissionsProvider => { + return { + getPermissions: async (methodNamespace: string, options: AbortOption = {}): Promise => { + const { signal } = options; + return await Methods(signal).method(methodNamespace, methodName, snapshotId).permissions(); + }, + updatePermissions: async ( + methodNamespace, + updatedPermissions: MethodConfigACL, + options: AbortOption = {} + ): Promise => { + const { signal } = options; + return await Methods(signal).method(methodNamespace, methodName, snapshotId).setPermissions(updatedPermissions); + }, + }; +}; diff --git a/src/pages/workflows/WorkflowList.test.tsx b/src/pages/workflows/WorkflowList.test.tsx index b196e3ce29..e3a518c050 100644 --- a/src/pages/workflows/WorkflowList.test.tsx +++ b/src/pages/workflows/WorkflowList.test.tsx @@ -13,6 +13,7 @@ import { notify } from 'src/libs/notifications'; import { TerraUser, TerraUserState, userStore } from 'src/libs/state'; import { WorkflowList } from 'src/pages/workflows/WorkflowList'; import { asMockedFn, partial, renderWithAppContexts as render } from 'src/testing/test-utils'; +import { WorkflowsPermissions } from 'src/workflows/workflows-acl-utils'; jest.mock('src/libs/ajax/methods/Methods'); @@ -52,9 +53,21 @@ jest.mock('react-virtualized', () => { }; }); +const mockPublicPermissions: WorkflowsPermissions = [ + { + role: 'READER', + user: 'public', + }, + { + role: 'OWNER', + user: 'revali@gale.com', + }, +]; + const mockMethods = (methods: MethodDefinition[]): MethodsAjaxContract => { return partial({ definitions: jest.fn(async () => methods), + getNamespacePermissions: jest.fn(async () => mockPublicPermissions), }); }; @@ -226,26 +239,67 @@ describe('workflows table', () => { const table: HTMLElement = await screen.findByRole('table'); // Assert - expect(table).toHaveAttribute('aria-colcount', '4'); + expect(table).toHaveAttribute('aria-colcount', '5'); expect(table).toHaveAttribute('aria-rowcount', '2'); const headers: HTMLElement[] = within(table).getAllByRole('columnheader'); - expect(headers).toHaveLength(4); - expect(headers[0]).toHaveTextContent('Workflow'); - expect(headers[1]).toHaveTextContent('Synopsis'); - expect(headers[2]).toHaveTextContent('Owners'); - expect(headers[3]).toHaveTextContent('Versions'); + expect(headers).toHaveLength(5); + expect(headers[0]).toHaveTextContent('Actions'); + expect(headers[1]).toHaveTextContent('Workflow'); + expect(headers[2]).toHaveTextContent('Synopsis'); + expect(headers[3]).toHaveTextContent('Owners'); + expect(headers[4]).toHaveTextContent('Versions'); const rows: HTMLElement[] = within(table).getAllByRole('row'); expect(rows).toHaveLength(2); const methodCells: HTMLElement[] = within(rows[1]).getAllByRole('cell'); - expect(methodCells).toHaveLength(4); - within(methodCells[0]).getByText('revali bird namespace'); - within(methodCells[0]).getByText('revali method 2'); - expect(methodCells[1]).toHaveTextContent('another revali description'); - expect(methodCells[2]).toHaveTextContent('revali@gale.com, revali@champions.com'); - expect(methodCells[3]).toHaveTextContent('1'); + expect(methodCells).toHaveLength(5); + within(methodCells[1]).getByText('revali bird namespace'); + within(methodCells[1]).getByText('revali method 2'); + expect(methodCells[2]).toHaveTextContent('another revali description'); + expect(methodCells[3]).toHaveTextContent('revali@gale.com, revali@champions.com'); + expect(methodCells[4]).toHaveTextContent('1'); + }); + + it('allows editing permissions for namespace owned by user', async () => { + // Arrange + asMockedFn(Methods).mockReturnValue(mockMethods([revaliMethod])); + const user: UserEvent = userEvent.setup(); + + // set the user's email + jest.spyOn(userStore, 'get').mockImplementation(jest.fn().mockReturnValue(mockUserState('revali@gale.com'))); + + // Act + await act(async () => { + render(); + }); + + const table: HTMLElement = await screen.findByRole('table'); + + const rows: HTMLElement[] = within(table).getAllByRole('row'); + expect(rows).toHaveLength(2); + + const methodCells: HTMLElement[] = within(rows[1]).getAllByRole('cell'); + expect(methodCells).toHaveLength(5); + within(methodCells[1]).getByText('revali bird namespace'); + within(methodCells[1]).getByText('revali method'); + + const actionsMenu = within(methodCells[0]).getByRole('button'); + + // Act + await user.click(actionsMenu); + + // Assert + expect(screen.getByText('Edit namespace permissions')); + + // Act + await user.click(screen.getByRole('button', { name: 'Edit namespace permissions' })); + + // Assert + expect( + screen.queryByRole('dialog', { name: /Edit permissions for namespace revali bird namespace/i }) + ).toBeInTheDocument(); }); it('displays a message with no my workflows', async () => { diff --git a/src/pages/workflows/WorkflowList.tsx b/src/pages/workflows/WorkflowList.tsx index 867161e037..861a635ec1 100644 --- a/src/pages/workflows/WorkflowList.tsx +++ b/src/pages/workflows/WorkflowList.tsx @@ -1,4 +1,4 @@ -import { SpinnerOverlay } from '@terra-ui-packages/components'; +import { Clickable, Icon, SpinnerOverlay } from '@terra-ui-packages/components'; import _ from 'lodash/fp'; import * as qs from 'qs'; import React, { useState } from 'react'; @@ -6,11 +6,14 @@ import { AutoSizer } from 'react-virtualized'; import { ButtonPrimary, Link } from 'src/components/common'; import FooterWrapper from 'src/components/FooterWrapper'; import { DelayedSearchInput } from 'src/components/input'; +import { MenuButton } from 'src/components/MenuButton'; +import { makeMenuIcon, MenuTrigger } from 'src/components/PopupTrigger'; import { TabBar } from 'src/components/tabBars'; import { FlexTable, HeaderCell, Paginator, Sortable, TooltipCell } from 'src/components/table'; import { TopBar } from 'src/components/TopBar'; import { Methods } from 'src/libs/ajax/methods/Methods'; import { MethodDefinition } from 'src/libs/ajax/methods/methods-models'; +import { namespacePermissionsProvider } from 'src/libs/ajax/methods/providers/PermissionsProvider'; import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import * as Nav from 'src/libs/nav'; import { notify } from 'src/libs/notifications'; @@ -19,6 +22,7 @@ import { getTerraUser } from 'src/libs/state'; import * as Utils from 'src/libs/utils'; import { withBusyState } from 'src/libs/utils'; import { CreateWorkflowModal } from 'src/workflows/modals/CreateWorkflowModal'; +import { PermissionsModal } from 'src/workflows/modals/PermissionsModal'; // Note: The first tab key in this array will determine the default tab selected // if the tab query parameter is not present or has an invalid value (and when @@ -105,6 +109,8 @@ export const WorkflowList = (props: WorkflowListProps) => { const [sort, setSort] = useState({ field: 'name', direction: 'asc' }); const [createWorkflowModalOpen, setCreateWorkflowModalOpen] = useState(false); + const [editNamespacePermissionsModalOpen, setEditNamespacePermissionsModalOpen] = useState(false); + const [namespacePermissionsToEdit, setNamespacePermissionsToEdit] = useState(''); const [pageNumber, setPageNumber] = useState(1); const [itemsPerPage, setItemsPerPage] = useState(25); @@ -261,7 +267,15 @@ export const WorkflowList = (props: WorkflowListProps) => { height={height} sort={sort as any /* necessary until FlexTable is converted to TS */} rowCount={paginatedWorkflows.length} - columns={getColumns(sort, onSort, paginatedWorkflows)} + columns={getColumns( + sort, + onSort, + (namespace) => { + setEditNamespacePermissionsModalOpen(true); + setNamespacePermissionsToEdit(namespace); + }, + paginatedWorkflows + )} variant={null} noContentMessage={workflows === undefined ? ' ' : 'Nothing to display'} tabIndex={-1} @@ -269,6 +283,15 @@ export const WorkflowList = (props: WorkflowListProps) => { )} + {editNamespacePermissionsModalOpen && ( + {}} // there is no need to refresh the page as permissions are always fetched when opening PermissionsModal + permissionsProvider={namespacePermissionsProvider} + /> + )} {!_.isEmpty(sortedWorkflows) && (
{ @@ -305,74 +328,107 @@ export const WorkflowList = (props: WorkflowListProps) => { const getColumns = ( sort: SortProperties, onSort: (newSort: SortProperties) => void, + onEditNamespacePermissions: (namespace: string) => void, paginatedWorkflows: MethodDefinition[] -) => [ - // Note: 'field' values should be MethodDefinition property names for sorting - // to work properly - { - field: 'name', - headerRenderer: () => ( - - Workflow - - ), - cellRenderer: ({ rowIndex }) => { - const { namespace, name } = paginatedWorkflows[rowIndex]; - - return ( - -
{namespace}
- - {name} - -
- ); +) => { + return [ + // Note: for sortable columns 'field' values should be MethodDefinition property names for sorting + // to work properly. This doesn't apply to 'Actions' column as it is not sortable. + { + field: 'actions', + headerRenderer: () => Actions, + cellRenderer: ({ rowIndex }) => { + const { namespace } = paginatedWorkflows[rowIndex]; + + return ( + onEditNamespacePermissions(namespace)}> + {makeMenuIcon('edit')} + Edit namespace permissions + + } + > + + + + + ); + }, + size: { basis: 100, grow: 0 }, }, - size: { basis: 300 }, - }, - { - field: 'synopsis', - headerRenderer: () => ( - - Synopsis - - ), - cellRenderer: ({ rowIndex }) => { - const { synopsis } = paginatedWorkflows[rowIndex]; - - return {synopsis}; + { + field: 'name', + headerRenderer: () => ( + + Workflow + + ), + cellRenderer: ({ rowIndex }) => { + const { namespace, name } = paginatedWorkflows[rowIndex]; + + return ( + +
{namespace}
+ + {name} + +
+ ); + }, + size: { basis: 300 }, }, - size: { basis: 475 }, - }, - { - field: 'managers', - headerRenderer: () => ( - - Owners - - ), - cellRenderer: ({ rowIndex }) => { - const { managers } = paginatedWorkflows[rowIndex]; - - return {managers?.join(', ')}; + { + field: 'synopsis', + headerRenderer: () => ( + + Synopsis + + ), + cellRenderer: ({ rowIndex }) => { + const { synopsis } = paginatedWorkflows[rowIndex]; + + return {synopsis}; + }, + size: { basis: 475 }, }, - size: { basis: 225 }, - }, - { - field: 'numSnapshots', - headerRenderer: () => ( - - Versions - - ), - cellRenderer: ({ rowIndex }) => { - const { numSnapshots } = paginatedWorkflows[rowIndex]; - - return
{numSnapshots}
; + { + field: 'managers', + headerRenderer: () => ( + + Owners + + ), + cellRenderer: ({ rowIndex }) => { + const { managers } = paginatedWorkflows[rowIndex]; + + return {managers?.join(', ')}; + }, + size: { basis: 225 }, }, - size: { basis: 115, grow: 0, shrink: 0 }, - }, -]; + { + field: 'numSnapshots', + headerRenderer: () => ( + + Versions + + ), + cellRenderer: ({ rowIndex }) => { + const { numSnapshots } = paginatedWorkflows[rowIndex]; + + return
{numSnapshots}
; + }, + size: { basis: 115, grow: 0, shrink: 0 }, + }, + ]; +}; export const navPaths = [ { diff --git a/src/pages/workflows/workflow-details/WorkflowWrapper.test.tsx b/src/pages/workflows/workflow-details/WorkflowWrapper.test.tsx index b6249d2a80..ba562ff779 100644 --- a/src/pages/workflows/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/workflows/workflow-details/WorkflowWrapper.test.tsx @@ -1191,7 +1191,7 @@ describe('workflows container', () => { await user.click(screen.getByRole('button', { name: 'Edit version permissions' })); // Assert - expect(screen.getByText('Edit Version Permissions')); + expect(screen.getByText('Edit version permissions')); }); it('hides edit permissions modal when it is dismissed', async () => { diff --git a/src/pages/workflows/workflow-details/WorkflowWrapper.ts b/src/pages/workflows/workflow-details/WorkflowWrapper.ts index c96aaa52f1..27abd62905 100644 --- a/src/pages/workflows/workflow-details/WorkflowWrapper.ts +++ b/src/pages/workflows/workflow-details/WorkflowWrapper.ts @@ -10,6 +10,7 @@ import { TopBar } from 'src/components/TopBar'; import { Methods } from 'src/libs/ajax/methods/Methods'; import { Snapshot } from 'src/libs/ajax/methods/methods-models'; import { editMethodProvider } from 'src/libs/ajax/methods/providers/EditMethodProvider'; +import { snapshotPermissionsProvider } from 'src/libs/ajax/methods/providers/PermissionsProvider'; import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import { makeExportWorkflowFromMethodsRepoProvider } from 'src/libs/ajax/workspaces/providers/ExportWorkflowToWorkspaceProvider'; import { ErrorCallback, withErrorReporting } from 'src/libs/error'; @@ -292,10 +293,9 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { h(PermissionsModal, { versionOrNamespace: 'Version', namespace, - name, - selectedSnapshot, setPermissionsModalOpen, refresh: loadSnapshot, + permissionsProvider: snapshotPermissionsProvider(name, selectedSnapshot), }), showCloneModal && h(CreateWorkflowModal, { diff --git a/src/workflows/modals/PermissionsModal.test.tsx b/src/workflows/modals/PermissionsModal.test.tsx index 5a23df6aa8..5893460320 100644 --- a/src/workflows/modals/PermissionsModal.test.tsx +++ b/src/workflows/modals/PermissionsModal.test.tsx @@ -1,10 +1,9 @@ -import { DeepPartial } from '@terra-ui-packages/core-utils'; import { act, fireEvent, screen } from '@testing-library/react'; import userEvent, { UserEvent } from '@testing-library/user-event'; import React from 'react'; -import { Ajax, AjaxContract } from 'src/libs/ajax'; +import { PermissionsProvider } from 'src/libs/ajax/methods/providers/PermissionsProvider'; import { reportError } from 'src/libs/error'; -import { asMockedFn, renderWithAppContexts, SelectHelper } from 'src/testing/test-utils'; +import { renderWithAppContexts, SelectHelper } from 'src/testing/test-utils'; import { PermissionsModal } from 'src/workflows/modals/PermissionsModal'; import { WorkflowsPermissions } from 'src/workflows/workflows-acl-utils'; @@ -51,47 +50,75 @@ const mockPermissions: WorkflowsPermissions = [ user: 'user2@bar.com', }, ]; -const mockWorkflowPermissions = jest.fn().mockReturnValue(Promise.resolve(mockPermissions)); +const mockGetPermissions = jest.fn().mockReturnValue(Promise.resolve(mockPermissions)); const mockSetPermissions = jest.fn(); -interface AjaxMocks { - permissionsImpl?: jest.Mock; - setPermissionsImpl?: jest.Mock; -} - -const mockAjax = (mocks: AjaxMocks = {}) => { - const { permissionsImpl, setPermissionsImpl } = mocks; - const mockAjax: DeepPartial = { - Methods: { - method: jest.fn().mockReturnValue({ - permissions: permissionsImpl || mockWorkflowPermissions, - setPermissions: setPermissionsImpl || mockSetPermissions, - }), - }, - }; - asMockedFn(Ajax).mockImplementation(() => mockAjax as AjaxContract); +const permissionsProviderSuccess: PermissionsProvider = { + getPermissions: mockGetPermissions, + updatePermissions: mockSetPermissions, +}; + +const permissionsProviderError: PermissionsProvider = { + getPermissions: jest.fn().mockImplementation(() => { + throw new Error('BOOM'); + }), + updatePermissions: jest.fn().mockImplementation(() => { + throw new Error('BOOM'); + }), +}; + +const permissionsProviderForbidden: PermissionsProvider = { + getPermissions: jest.fn().mockImplementation(() => { + throw new Response(JSON.stringify('No access'), { status: 403 }); + }), + updatePermissions: mockSetPermissions, }; describe('PermissionsModal', () => { - it('loads the correct title and basic elements', async () => { + it('loads the correct title and basic elements for version permissions', async () => { // ARRANGE - mockAjax(); await act(async () => { renderWithAppContexts( + ); + }); + + // ASSERT + expect(screen.getByText('Edit version permissions')); + expect(screen.getByText('Note: Sharing with user groups is not supported.')); + expect(screen.getByText('User')); + expect(screen.getByRole('textbox', { name: 'User' })); + expect(screen.getByRole('button', { name: 'Add' })); + expect(screen.getByText('Current Users')); + expect(screen.getByRole('checkbox', { name: 'Make Publicly Readable?' })); + expect(screen.getByRole('button', { name: 'Cancel' })); + expect(screen.getByRole('button', { name: 'Save' })); + }); + + it('loads the correct title and basic elements for namespace permissions', async () => { + // ARRANGE + + await act(async () => { + renderWithAppContexts( + ); }); // ASSERT - expect(screen.getByText('Edit Version Permissions')); + expect(screen.getByText('Edit permissions for namespace my-namespace')); expect(screen.getByText('Note: Sharing with user groups is not supported.')); expect(screen.getByText('User')); expect(screen.getByRole('textbox', { name: 'User' })); @@ -104,25 +131,23 @@ describe('PermissionsModal', () => { it('loads users with proper permissions', async () => { // ARRANGE - mockAjax(); const user = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); // ASSERT - expect(Ajax().Methods.method).toHaveBeenCalledWith('namespace', 'test', 3); - expect(mockWorkflowPermissions).toHaveBeenCalled(); + expect(mockGetPermissions).toHaveBeenCalled(); + expect(mockGetPermissions).toHaveBeenCalledWith('namespace', expect.any(Object)); expect(screen.getByText('user1@foo.com')); expect(screen.getByText('user2@bar.com')); @@ -141,18 +166,16 @@ describe('PermissionsModal', () => { it('adds a new user and permissions', async () => { // ARRANGE - mockAjax(); const user = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); @@ -177,27 +200,29 @@ describe('PermissionsModal', () => { await user.click(saveButton); expect(mockSetPermissions).toHaveBeenCalledTimes(1); - expect(mockSetPermissions).toHaveBeenCalledWith([ - { role: 'OWNER', user: 'user1@foo.com' }, - { role: 'READER', user: 'user2@bar.com' }, - { role: 'OWNER', user: 'newuser@boo.com' }, - ]); + expect(mockSetPermissions).toHaveBeenCalledWith( + 'namespace', + [ + { role: 'OWNER', user: 'user1@foo.com' }, + { role: 'READER', user: 'user2@bar.com' }, + { role: 'OWNER', user: 'newuser@boo.com' }, + ], + expect.any(Object) + ); }); it('removes a user and their permissions', async () => { // ARRANGE - mockAjax(); const user = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); @@ -214,26 +239,28 @@ describe('PermissionsModal', () => { await user.click(saveButton); expect(mockSetPermissions).toHaveBeenCalledTimes(1); - expect(mockSetPermissions).toHaveBeenCalledWith([ - { role: 'OWNER', user: 'user1@foo.com' }, - { role: 'NO ACCESS', user: 'user2@bar.com' }, - ]); + expect(mockSetPermissions).toHaveBeenCalledWith( + 'namespace', + [ + { role: 'OWNER', user: 'user1@foo.com' }, + { role: 'NO ACCESS', user: 'user2@bar.com' }, + ], + expect.any(Object) + ); }); it('does not allow the current user to edit their own permissions', async () => { // ARRANGE - mockAjax(); const user: UserEvent = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); @@ -248,17 +275,15 @@ describe('PermissionsModal', () => { it('gives an error with invalid user email', async () => { // ARRANGE - mockAjax(); await act(async () => { renderWithAppContexts( ); }); @@ -278,18 +303,16 @@ describe('PermissionsModal', () => { it('gives an error with a user email that already exists', async () => { // ARRANGE - mockAjax(); const user: UserEvent = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); @@ -318,20 +341,20 @@ describe('PermissionsModal', () => { it('lets you make a public workflow private', async () => { // ARRANGE - mockAjax({ - permissionsImpl: jest.fn().mockResolvedValue(mockPublicSnapshotPermissions), - }); const user: UserEvent = userEvent.setup(); + const mockProvider = { + getPermissions: jest.fn().mockReturnValue(Promise.resolve(mockPublicSnapshotPermissions)), + updatePermissions: mockSetPermissions, + }; await act(async () => { renderWithAppContexts( ); }); @@ -345,27 +368,29 @@ describe('PermissionsModal', () => { await user.click(screen.getByRole('button', { name: 'Save' })); expect(mockSetPermissions).toHaveBeenCalledTimes(1); - expect(mockSetPermissions).toHaveBeenCalledWith([ - { role: 'NO ACCESS', user: 'public' }, - { role: 'OWNER', user: 'user1@foo.com' }, - { role: 'READER', user: 'user2@bar.com' }, - ]); + expect(mockSetPermissions).toHaveBeenCalledWith( + 'namespace', + [ + { role: 'NO ACCESS', user: 'public' }, + { role: 'OWNER', user: 'user1@foo.com' }, + { role: 'READER', user: 'user2@bar.com' }, + ], + expect.any(Object) + ); }); it('lets you make a private workflow public', async () => { // ARRANGE - mockAjax(); const user: UserEvent = userEvent.setup(); await act(async () => { renderWithAppContexts( ); }); @@ -379,16 +404,19 @@ describe('PermissionsModal', () => { await user.click(screen.getByRole('button', { name: 'Save' })); expect(mockSetPermissions).toHaveBeenCalledTimes(1); - expect(mockSetPermissions).toHaveBeenCalledWith([ - { role: 'READER', user: 'public' }, - { role: 'OWNER', user: 'user1@foo.com' }, - { role: 'READER', user: 'user2@bar.com' }, - ]); + expect(mockSetPermissions).toHaveBeenCalledWith( + 'namespace', + [ + { role: 'READER', user: 'public' }, + { role: 'OWNER', user: 'user1@foo.com' }, + { role: 'READER', user: 'user2@bar.com' }, + ], + expect.any(Object) + ); }); it('closes the modal when you press the cancel button', async () => { // ARRANGE - mockAjax(); const user: UserEvent = userEvent.setup(); const mockSetPermissionsModalOpen = jest.fn(); @@ -397,12 +425,11 @@ describe('PermissionsModal', () => { await act(async () => { renderWithAppContexts( ); }); @@ -418,21 +445,18 @@ describe('PermissionsModal', () => { it('closes the modal and calls the refresh callback when you save permissions', async () => { // ARRANGE - mockAjax(); const user: UserEvent = userEvent.setup(); - const mockSetPermissionsModalOpen = jest.fn(); const mockRefresh = jest.fn(); await act(async () => { renderWithAppContexts( ); }); @@ -447,24 +471,17 @@ describe('PermissionsModal', () => { it('displays an error popup and closes the modal if there is an error loading permissions', async () => { // ARRANGE - mockAjax({ - permissionsImpl: jest.fn().mockImplementation(() => { - throw new Error('BOOM'); - }), - }); - const mockSetPermissionsModalOpen = jest.fn(); const mockRefresh = jest.fn(); await act(async () => { renderWithAppContexts( ); }); @@ -475,13 +492,29 @@ describe('PermissionsModal', () => { expect(mockRefresh).not.toHaveBeenCalled(); }); - it('displays an error popup and closes the modal if there is an error saving permissions', async () => { + it('displays an error message is user does not have edit permissions', async () => { // ARRANGE - mockAjax({ - setPermissionsImpl: jest.fn().mockImplementation(() => { - throw new Error('BOOM'); - }), + const mockSetPermissionsModalOpen = jest.fn(); + const mockRefresh = jest.fn(); + + await act(async () => { + renderWithAppContexts( + + ); }); + + // ASSERT + expect(screen.getByText('You do not have permissions to edit namespace settings.')).toBeInTheDocument(); + }); + + it('displays an error popup and closes the modal if there is an error saving permissions', async () => { + // ARRANGE const user: UserEvent = userEvent.setup(); const mockSetPermissionsModalOpen = jest.fn(); @@ -490,12 +523,11 @@ describe('PermissionsModal', () => { await act(async () => { renderWithAppContexts( ); }); diff --git a/src/workflows/modals/PermissionsModal.tsx b/src/workflows/modals/PermissionsModal.tsx index 77f39dcece..3d48cbc57b 100644 --- a/src/workflows/modals/PermissionsModal.tsx +++ b/src/workflows/modals/PermissionsModal.tsx @@ -13,7 +13,7 @@ import React, { CSSProperties, Dispatch, SetStateAction, useRef, useState } from import { IdContainer, LabeledCheckbox } from 'src/components/common'; import { ValidatedInput } from 'src/components/input'; import { getPopupRoot } from 'src/components/popup-utils'; -import { Ajax } from 'src/libs/ajax'; +import { PermissionsProvider } from 'src/libs/ajax/methods/providers/PermissionsProvider'; import colors from 'src/libs/colors'; import { reportError } from 'src/libs/error'; import { FormLabel } from 'src/libs/forms'; @@ -33,10 +33,9 @@ import validate from 'validate.js'; type WorkflowPermissionsModalProps = { versionOrNamespace: 'Version' | 'Namespace'; namespace: string; - name: string; - selectedSnapshot: number; setPermissionsModalOpen: (b: boolean) => void; refresh: () => void; + permissionsProvider: PermissionsProvider; }; type UserProps = { @@ -83,7 +82,7 @@ const UserSelectInput = (props: UserSelectProps) => { return (
-
+
aria-label={`selected role ${role}`} value={role} @@ -167,7 +166,7 @@ const CurrentUsers = (props: CurrentUsersProps) => { }; export const PermissionsModal = (props: WorkflowPermissionsModalProps) => { - const { versionOrNamespace, namespace, name, selectedSnapshot, setPermissionsModalOpen, refresh } = props; + const { versionOrNamespace, namespace, setPermissionsModalOpen, refresh, permissionsProvider } = props; const signal: AbortSignal = useCancellation(); const [searchValue, setSearchValue] = useState(''); const [permissions, setPermissions] = useState([]); @@ -179,18 +178,24 @@ export const PermissionsModal = (props: WorkflowPermissionsModalProps) => { const errors = validate({ searchValue }, constraints(userEmails), { prettify: (v) => ({ searchValue: 'User' }[v] || validate.prettify(v)), }); + const [noEditPermissions, setNoEditPermissions] = useState(false); useOnMount(() => { const loadWorkflowPermissions = withBusyState(setWorking, async () => { try { - const workflowPermissions: WorkflowsPermissions = await Ajax(signal) - .Methods.method(namespace, name, selectedSnapshot) - .permissions(); + const workflowPermissions: WorkflowsPermissions = await permissionsProvider.getPermissions(namespace, { + signal, + }); setPermissions(workflowPermissions); setOriginalPermissions(workflowPermissions); } catch (error) { - await reportError('Error loading permissions.', error); - setPermissionsModalOpen(false); + // user doesn't have permissions to edit the namespace/version permissions + if (error instanceof Response && error.status === 403) { + setNoEditPermissions(true); + } else { + await reportError('Error loading permissions.', error); + setPermissionsModalOpen(false); + } } }); @@ -223,7 +228,7 @@ export const PermissionsModal = (props: WorkflowPermissionsModalProps) => { const permissionUpdates: WorkflowsPermissions = [...permissions, ...toBeDeletedPermissionUpdates]; try { - await Ajax(signal).Methods.method(namespace, name, selectedSnapshot).setPermissions(permissionUpdates); + await permissionsProvider.updatePermissions(namespace, permissionUpdates, { signal }); refresh(); setPermissionsModalOpen(false); } catch (error) { @@ -232,65 +237,74 @@ export const PermissionsModal = (props: WorkflowPermissionsModalProps) => { } }); + const modalTitle = + versionOrNamespace === 'Version' ? 'Edit version permissions' : `Edit permissions for namespace ${namespace}`; + const noEditPermissionsMsg = + versionOrNamespace === 'Version' + ? 'You do not have permissions to edit version settings.' + : 'You do not have permissions to edit namespace settings.'; + return ( - setPermissionsModalOpen(false)} - width='30rem' - showButtons={false} - > -
- - - Note: Sharing with user groups is not supported. - -
- - {(id) => ( -
- - User - - { - setSearchValue(v); - setUserValueModified(true); - }, - }} - error={Utils.summarizeErrors(userValueModified && errors?.searchValue)} - /> -
- )} -
- addUser(searchValue)}> - Add - -
-
- -
+ setPermissionsModalOpen(false)} width='600px' showButtons={false} showX> + {noEditPermissions && ( +
{noEditPermissionsMsg}
+ )} + {!working && !noEditPermissions && (
- { - updatePublicUser(v); - }} - > - Make Publicly Readable? - +
+ + + Note: Sharing with user groups is not supported. + +
+ + {(id) => ( +
+ + User + + { + setSearchValue(v); + setUserValueModified(true); + }, + }} + error={Utils.summarizeErrors(userValueModified && errors?.searchValue)} + /> +
+ )} +
+ addUser(searchValue)}> + Add + +
+
+ +
+
+ { + updatePublicUser(v); + }} + > + Make Publicly Readable? + +
+ + setPermissionsModalOpen(false)}> + Cancel + + Save + +
- - setPermissionsModalOpen(false)}> - Cancel - - Save - -
+ )} {working && }
);