Skip to content

Commit

Permalink
[WOR-1399] Follow-up to reduce the size of payload for list API. (#4556)
Browse files Browse the repository at this point in the history
  • Loading branch information
cahrens authored Dec 8, 2023
1 parent d5e8339 commit 295405d
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 18 deletions.
19 changes: 16 additions & 3 deletions src/pages/workspaces/WorkspacesList/RenderedWorkspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import * as Utils from 'src/libs/utils';
import {
canRead,
getCloudProviderFromWorkspace,
isGoogleWorkspace,
workspaceAccessLevels,
WorkspaceInfo,
WorkspacePolicy,
WorkspaceWrapper as Workspace,
} from 'src/libs/workspace-utils';
import { WorkspaceMenu } from 'src/pages/workspaces/workspace/WorkspaceMenu';
Expand Down Expand Up @@ -380,11 +382,22 @@ const ActionsCell = (props: ActionsCellProps): ReactNode => {
return null;
}
const getWorkspace = (id: string): Workspace => _.find({ workspace: { workspaceId: id } }, props.workspaces)!;

const onClone = () => setUserActions({ cloningWorkspaceId: workspaceId });
const extendWorkspace = (workspaceId: string, policies?: WorkspacePolicy[], bucketName?: string): Workspace => {
// The workspaces from the list API have fewer properties to keep the payload as small as possible.
const listWorkspace = getWorkspace(workspaceId);
const extendedWorkspace = policies === undefined ? listWorkspace : { ...listWorkspace, policies };
if (bucketName !== undefined && isGoogleWorkspace(extendedWorkspace)) {
extendedWorkspace.workspace.bucketName = bucketName;
}
return extendedWorkspace;
};

const onClone = (policies, bucketName) =>
setUserActions({ cloningWorkspace: extendWorkspace(workspaceId, policies, bucketName) });
const onDelete = () => setUserActions({ deletingWorkspaceId: workspaceId });
const onLock = () => setUserActions({ lockingWorkspaceId: workspaceId });
const onShare = () => setUserActions({ sharingWorkspace: getWorkspace(workspaceId) });
const onShare = (policies, bucketName) =>
setUserActions({ sharingWorkspace: extendWorkspace(workspaceId, policies, bucketName) });
const onLeave = () => setUserActions({ leavingWorkspaceId: workspaceId });

return div({ style: { ...styles.tableCellContainer, paddingRight: 0 } }, [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ export interface WorkspaceUserActionsState {
// TODO: these should be removed in favor of the modal manager once available
export interface WorkspaceUserActions {
creatingNewWorkspace: boolean;
cloningWorkspaceId?: string;
deletingWorkspaceId?: string;
lockingWorkspaceId?: string;
leavingWorkspaceId?: string;
requestingAccessWorkspaceId?: string;
sharingWorkspace?: WorkspaceWrapper;
cloningWorkspace?: WorkspaceWrapper;
}
6 changes: 3 additions & 3 deletions src/pages/workspaces/WorkspacesList/WorkspacesListModals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ export const WorkspacesListModals = (props: WorkspacesListModalsProps): ReactNod
onDismiss: () => setUserActions({ creatingNewWorkspace: false }),
onSuccess: ({ namespace, name }) => goToPath('workspace-dashboard', { namespace, name }),
}),
!!userActions.cloningWorkspaceId &&
!!userActions.cloningWorkspace &&
h(NewWorkspaceModal, {
cloneWorkspace: getWorkspace(userActions.cloningWorkspaceId),
onDismiss: () => setUserActions({ cloningWorkspaceId: undefined }),
cloneWorkspace: userActions.cloningWorkspace,
onDismiss: () => setUserActions({ cloningWorkspace: undefined }),
onSuccess: ({ namespace, name }) => goToPath('workspace-dashboard', { namespace, name }),
}),
!!userActions.deletingWorkspaceId &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@ export const useWorkspacesWithSubmissionStats = (): WorkspacesWithSubmissionStat
} = useWorkspaces(
[
'accessLevel',
'policies', // Needed for ShareWorkspace modal on list view
'public',
'workspace.attributes.description',
'workspace.attributes.tag:tags',
'workspace.authorizationDomain',
'workspace.bucketName', // Needed for ShareWorkspace modal on list view
'workspace.cloudPlatform',
'workspace.createdBy',
'workspace.lastModified',
Expand Down
147 changes: 145 additions & 2 deletions src/pages/workspaces/workspace/WorkspaceMenu.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { asMockedFn } from '@terra-ui-packages/test-utils';
import { fireEvent, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { axe } from 'jest-axe';
import { div, h } from 'react-hyperscript-helpers';
import { MenuTrigger } from 'src/components/PopupTrigger';
import { useWorkspaceDetails } from 'src/components/workspace-utils';
import * as WorkspaceUtils from 'src/libs/workspace-utils';
import { WorkspaceAccessLevel } from 'src/libs/workspace-utils';
import { AzureWorkspace, GoogleWorkspace, WorkspaceAccessLevel } from 'src/libs/workspace-utils';
import { tooltipText, WorkspaceMenu } from 'src/pages/workspaces/workspace/WorkspaceMenu';
import { renderWithAppContexts as render } from 'src/testing/test-utils';
import { defaultGoogleWorkspace } from 'src/testing/workspace-fixtures';
import { defaultGoogleWorkspace, protectedDataPolicy } from 'src/testing/workspace-fixtures';

jest.mock('src/components/workspace-utils', () => {
const originalModule = jest.requireActual('src/components/workspace-utils');
Expand Down Expand Up @@ -358,3 +359,145 @@ describe('WorkspaceMenu - defined workspace (GCP or Azure)', () => {
}
});
});

describe('DynamicWorkspaceMenuContent fetches specific workspace details', () => {
const googleWorkspace: GoogleWorkspace = {
// @ts-expect-error - Limit return values based on what is requested
workspace: {
cloudPlatform: 'Gcp',
bucketName: 'fc-bucketname',
isLocked: false,
state: 'Ready',
},
accessLevel: 'OWNER',
canShare: true,
policies: [],
};

const azureWorkspace: AzureWorkspace = {
// @ts-expect-error - Limit return values based on what is requested
workspace: {
cloudPlatform: 'Azure',
isLocked: false,
state: 'Ready',
},
accessLevel: 'OWNER',
canShare: true,
policies: [protectedDataPolicy],
};

const onClone = jest.fn((_policies, _bucketName) => {});
const onShare = jest.fn((_policies, _bucketName) => {});
const namespace = 'test-namespace';
const name = 'test-name';

const workspaceMenuProps = {
iconSize: 20,
popupLocation: 'left',
callbacks: { onClone, onShare, onLock: jest.fn(), onDelete: jest.fn(), onLeave: jest.fn() },
workspaceInfo: { namespace, name },
};

it('requests expected fields', async () => {
// Arrange
const workspaceDetails = asMockedFn(useWorkspaceDetails).mockReturnValue({
// @ts-expect-error - the type checker thinks workspace is only of type undefined
workspace: googleWorkspace,
refresh: jest.fn(),
loading: false,
});

// cloudPlatform is necessary to determine if a workspace is a Google Workspace.
const expectedRequestedFields = [
'accessLevel',
'canShare',
'policies',
'workspace.bucketName',
'workspace.cloudPlatform',
'workspace.isLocked',
'workspace.state',
];

// Act
render(h(WorkspaceMenu, workspaceMenuProps));

// Assert
expect(workspaceDetails).toHaveBeenCalledWith({ namespace, name }, expectedRequestedFields);
});

it('passes onClone the bucketName for a Google workspace', async () => {
// Arrange
const user = userEvent.setup();
asMockedFn(useWorkspaceDetails).mockReturnValue({
// @ts-expect-error - the type checker thinks workspace is only of type undefined
workspace: googleWorkspace,
refresh: jest.fn(),
loading: false,
});

// Act
render(h(WorkspaceMenu, workspaceMenuProps));

// Assert
const menuItem = screen.getByText('Clone');
await user.click(menuItem);
expect(onClone).toBeCalledWith([], 'fc-bucketname');
});

it('passes onClone the policies for an Azure workspace', async () => {
// Arrange
const user = userEvent.setup();
asMockedFn(useWorkspaceDetails).mockReturnValue({
// @ts-expect-error - the type checker thinks workspace is only of type undefined
workspace: azureWorkspace,
refresh: jest.fn(),
loading: false,
});

// Act
render(h(WorkspaceMenu, workspaceMenuProps));

// Assert
const menuItem = screen.getByText('Clone');
await user.click(menuItem);
expect(onClone).toBeCalledWith([protectedDataPolicy], undefined);
});

it('passes onShare the bucketName for a Google workspace', async () => {
// Arrange
const user = userEvent.setup();
asMockedFn(useWorkspaceDetails).mockReturnValue({
// @ts-expect-error - the type checker thinks workspace is only of type undefined
workspace: googleWorkspace,
refresh: jest.fn(),
loading: false,
});

// Act
render(h(WorkspaceMenu, workspaceMenuProps));

// Assert
const menuItem = screen.getByText('Share');
await user.click(menuItem);
expect(onShare).toBeCalledWith([], 'fc-bucketname');
});

it('passes onShare the policies for an Azure workspace', async () => {
// Arrange
const user = userEvent.setup();
asMockedFn(useWorkspaceDetails).mockReturnValue({
// @ts-expect-error - the type checker thinks workspace is only of type undefined
workspace: azureWorkspace,
refresh: jest.fn(),
loading: false,
});

// Act
render(h(WorkspaceMenu, workspaceMenuProps));

// Assert
const menuItem = screen.getByText('Share');
await user.click(menuItem);
expect(onShare).toBeCalledWith([protectedDataPolicy], undefined);
});
});
31 changes: 24 additions & 7 deletions src/pages/workspaces/workspace/WorkspaceMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { Clickable } from 'src/components/common';
import { MenuButton } from 'src/components/MenuButton';
import { makeMenuIcon, MenuTrigger } from 'src/components/PopupTrigger';
import { useWorkspaceDetails } from 'src/components/workspace-utils';
import { isOwner, WorkspaceState, WorkspaceWrapper as Workspace } from 'src/libs/workspace-utils';
import {
isGoogleWorkspace,
isOwner,
WorkspacePolicy,
WorkspaceState,
WorkspaceWrapper as Workspace,
} from 'src/libs/workspace-utils';

const isNameType = (o: WorkspaceInfo): o is DynamicWorkspaceInfo =>
'name' in o && typeof o.name === 'string' && 'namespace' in o && typeof o.namespace === 'string';
Expand All @@ -22,8 +28,8 @@ type DynamicWorkspaceInfo = { name: string; namespace: string };
type WorkspaceInfo = DynamicWorkspaceInfo | LoadedWorkspaceInfo;

interface WorkspaceMenuCallbacks {
onClone: () => void;
onShare: () => void;
onClone: (policies?: WorkspacePolicy[], bucketName?: string) => void;
onShare: (policies?: WorkspacePolicy[], bucketName?: string) => void;
onLock: () => void;
onDelete: () => void;
onLeave: () => void;
Expand Down Expand Up @@ -77,9 +83,9 @@ interface DynamicWorkspaceMenuContentProps {
}

/**
* DynamicWorkspaceInfo is invoked when the name/namespace is passed instead of the dirived states.
* This happens from the list component, which also needs the workspace policies for sharing the workspace.
* So the onShare callback is wrapped here, and passed to LoadedWorkspaceMenuContent.
* DynamicWorkspaceInfo is invoked when the name/namespace is passed instead of the derived states.
* This happens from the list component, which also needs the workspace policies and bucketName for
* sharing and cloning the workspace.
*/
const DynamicWorkspaceMenuContent = (props: DynamicWorkspaceMenuContentProps) => {
const {
Expand All @@ -89,9 +95,13 @@ const DynamicWorkspaceMenuContent = (props: DynamicWorkspaceMenuContentProps) =>
const { workspace } = useWorkspaceDetails({ namespace, name }, [
'accessLevel',
'canShare',
'policies',
'workspace.bucketName',
'workspace.cloudPlatform',
'workspace.isLocked',
'workspace.state',
]) as { workspace?: Workspace };
const bucketName = !!workspace && isGoogleWorkspace(workspace) ? workspace.workspace.bucketName : undefined;

return h(LoadedWorkspaceMenuContent, {
workspaceInfo: {
Expand All @@ -101,7 +111,14 @@ const DynamicWorkspaceMenuContent = (props: DynamicWorkspaceMenuContentProps) =>
isOwner: !!workspace && isOwner(workspace.accessLevel),
workspaceLoaded: !!workspace,
},
callbacks,
// The list component doesn't fetch all the workspace details in order to keep the size of returned payload
// as small as possible, so we need to pass policies and bucketName for use by the ShareWorkspaceModal
// and NewWorkspaceModal (cloning). The dashboard component already has the fields, so it will ignore them.
callbacks: {
...callbacks,
onShare: () => callbacks.onShare(workspace?.policies, bucketName),
onClone: () => callbacks.onClone(workspace?.policies, bucketName),
},
});
};

Expand Down

0 comments on commit 295405d

Please sign in to comment.