From 01837c78066d864439be3dcddecbc95ced72c936 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 16 Jan 2024 16:12:04 -0500 Subject: [PATCH 01/24] filter out cromwell app types; update test --- .../Environments/Environments.test.ts | 39 ++++++++++--------- src/analysis/Environments/Environments.ts | 5 ++- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 67092052af..bf993d1cb4 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -508,25 +508,26 @@ describe('Environments', () => { expect(getTextContentForColumn(secondAppRow, 7)).toBe(Utils.makeCompleteDate(googleApp2.auditInfo.createdDate)); expect(getTextContentForColumn(secondAppRow, 8)).toBe(Utils.makeCompleteDate(googleApp1.auditInfo.dateAccessed)); - const thirdAppRow: HTMLElement = tableRows[2]; - expect(getTextContentForColumn(thirdAppRow, 0)).toBe(azureApp1.labels.saturnWorkspaceNamespace); - expect(getTextContentForColumn(thirdAppRow, 1)).toBe(azureApp1.labels.saturnWorkspaceName); - expect(getTextContentForColumn(thirdAppRow, 2)).toBe('Kubernetes'); - expect(getTextContentForColumn(thirdAppRow, 3)).toBe(_.capitalize(azureApp1.appType)); - expect(getTextContentForColumn(thirdAppRow, 5)).toBe(_.capitalize(azureApp1.status)); - expect(getTextContentForColumn(thirdAppRow, 6)).toBe(azureApp1.region); - expect(getTextContentForColumn(thirdAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.createdDate)); - expect(getTextContentForColumn(thirdAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.dateAccessed)); - - const fourthAppRow: HTMLElement = tableRows[3]; - expect(getTextContentForColumn(fourthAppRow, 0)).toBe(azureApp2.labels.saturnWorkspaceNamespace); - expect(getTextContentForColumn(fourthAppRow, 1)).toBe(azureApp2.labels.saturnWorkspaceName); - expect(getTextContentForColumn(fourthAppRow, 2)).toBe('Kubernetes'); - expect(getTextContentForColumn(fourthAppRow, 3)).toBe(_.capitalize(azureApp2.appType)); - expect(getTextContentForColumn(fourthAppRow, 5)).toBe(_.capitalize(azureApp2.status)); - expect(getTextContentForColumn(fourthAppRow, 6)).toBe(azureApp2.region); - expect(getTextContentForColumn(fourthAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.createdDate)); - expect(getTextContentForColumn(fourthAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.dateAccessed)); + // Verify that Cromwell apps do not appear on the page + expect(screen.queryByText(azureApp1.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); + expect(screen.queryByText(azureApp2.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); + // expect(getTextContentForColumn(thirdAppRow, 1)).toBe(azureApp1.labels.saturnWorkspaceName); + // expect(getTextContentForColumn(thirdAppRow, 2)).toBe('Kubernetes'); + // expect(getTextContentForColumn(thirdAppRow, 3)).toBe(_.capitalize(azureApp1.appType)); + // expect(getTextContentForColumn(thirdAppRow, 5)).toBe(_.capitalize(azureApp1.status)); + // expect(getTextContentForColumn(thirdAppRow, 6)).toBe(azureApp1.region); + // expect(getTextContentForColumn(thirdAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.createdDate)); + // expect(getTextContentForColumn(thirdAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.dateAccessed)); + + // const fourthAppRow: HTMLElement = tableRows[2]; + // expect(getTextContentForColumn(fourthAppRow, 0)).toBe(azureApp2.labels.saturnWorkspaceNamespace); + // expect(getTextContentForColumn(fourthAppRow, 1)).toBe(azureApp2.labels.saturnWorkspaceName); + // expect(getTextContentForColumn(fourthAppRow, 2)).toBe('Kubernetes'); + // expect(getTextContentForColumn(fourthAppRow, 3)).toBe(_.capitalize(azureApp2.appType)); + // expect(getTextContentForColumn(fourthAppRow, 5)).toBe(_.capitalize(azureApp2.status)); + // expect(getTextContentForColumn(fourthAppRow, 6)).toBe(azureApp2.region); + // expect(getTextContentForColumn(fourthAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.createdDate)); + // expect(getTextContentForColumn(fourthAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.dateAccessed)); }); it.each([ diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index f24fbc91e5..220d9cf79c 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -283,7 +283,10 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { const [newRuntimes, newDisks, newApps] = await Promise.all([ leoRuntimeData.list(listArgs, { signal }), leoDiskData.list(diskArgs, { signal }), - leoAppData.listWithoutProject(listArgs, { signal }), + // Excluding cromwell/workflows app types for now, as trying to delete these app types puts associated workspace in an error state + ( + await leoAppData.listWithoutProject(listArgs, { signal }) + ).filter((app) => app.appType.includes('GALAXY' || 'HAIL_BATCH' || 'WDS')), ]); const endTimeForLeoCallsEpochMs = Date.now(); From f21115f8e5fccd513698520308a813f1223e2de2 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 16 Jan 2024 16:17:40 -0500 Subject: [PATCH 02/24] cleanup --- src/analysis/Environments/Environments.test.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index bf993d1cb4..15042c9a1c 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -511,23 +511,6 @@ describe('Environments', () => { // Verify that Cromwell apps do not appear on the page expect(screen.queryByText(azureApp1.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); expect(screen.queryByText(azureApp2.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); - // expect(getTextContentForColumn(thirdAppRow, 1)).toBe(azureApp1.labels.saturnWorkspaceName); - // expect(getTextContentForColumn(thirdAppRow, 2)).toBe('Kubernetes'); - // expect(getTextContentForColumn(thirdAppRow, 3)).toBe(_.capitalize(azureApp1.appType)); - // expect(getTextContentForColumn(thirdAppRow, 5)).toBe(_.capitalize(azureApp1.status)); - // expect(getTextContentForColumn(thirdAppRow, 6)).toBe(azureApp1.region); - // expect(getTextContentForColumn(thirdAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.createdDate)); - // expect(getTextContentForColumn(thirdAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp1.auditInfo.dateAccessed)); - - // const fourthAppRow: HTMLElement = tableRows[2]; - // expect(getTextContentForColumn(fourthAppRow, 0)).toBe(azureApp2.labels.saturnWorkspaceNamespace); - // expect(getTextContentForColumn(fourthAppRow, 1)).toBe(azureApp2.labels.saturnWorkspaceName); - // expect(getTextContentForColumn(fourthAppRow, 2)).toBe('Kubernetes'); - // expect(getTextContentForColumn(fourthAppRow, 3)).toBe(_.capitalize(azureApp2.appType)); - // expect(getTextContentForColumn(fourthAppRow, 5)).toBe(_.capitalize(azureApp2.status)); - // expect(getTextContentForColumn(fourthAppRow, 6)).toBe(azureApp2.region); - // expect(getTextContentForColumn(fourthAppRow, 7)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.createdDate)); - // expect(getTextContentForColumn(fourthAppRow, 8)).toBe(Utils.makeCompleteDate(azureApp2.auditInfo.dateAccessed)); }); it.each([ From f94f52ec17efc00fce20fc623177021ccb28f2ea Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 16 Jan 2024 16:49:53 -0500 Subject: [PATCH 03/24] Commenting out azure tests to be used again in the future --- src/analysis/Environments/Environments.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 15042c9a1c..0077a114f9 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -515,7 +515,7 @@ describe('Environments', () => { it.each([ { app: generateTestAppWithGoogleWorkspace({}, defaultGoogleWorkspace), workspace: defaultGoogleWorkspace }, - { app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), workspace: defaultAzureWorkspace }, + // Reimplement when Azure apps can be deleted safely { app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), workspace: defaultAzureWorkspace }, ])('Renders app details view correctly', async ({ app, workspace }) => { // Arrange const props = getEnvironmentsProps(); @@ -553,11 +553,11 @@ describe('Environments', () => { workspace: defaultGoogleWorkspace, isAzure: false, }, - { - app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), - workspace: defaultAzureWorkspace, - isAzure: true, - }, + // Reimplement when Azure apps can be deleted safely { + // app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), + // workspace: defaultAzureWorkspace, + // isAzure: true, + // }, ])('Behaves properly when we click pause/delete for azure/gce app', async ({ app, workspace }) => { // Arrange const user = userEvent.setup(); From cc9aa483faf7e1149eec32ce0f597c8f212d4a51 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 16 Jan 2024 17:10:48 -0500 Subject: [PATCH 04/24] refactor --- src/analysis/Environments/Environments.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index 220d9cf79c..2df9b4dd62 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -280,13 +280,14 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { includeLabels: 'saturnApplication,saturnWorkspaceNamespace,saturnWorkspaceName', }; + const noCromwellAppTypes = ['GALAXY', 'HAIL_BATCH', 'WDS']; const [newRuntimes, newDisks, newApps] = await Promise.all([ leoRuntimeData.list(listArgs, { signal }), leoDiskData.list(diskArgs, { signal }), // Excluding cromwell/workflows app types for now, as trying to delete these app types puts associated workspace in an error state ( await leoAppData.listWithoutProject(listArgs, { signal }) - ).filter((app) => app.appType.includes('GALAXY' || 'HAIL_BATCH' || 'WDS')), + ).filter((app) => _.includes(app.appType, noCromwellAppTypes)), ]); const endTimeForLeoCallsEpochMs = Date.now(); From 4268574d35453ad157679471bf16b7c0610b9297 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Fri, 19 Jan 2024 13:19:15 -0500 Subject: [PATCH 05/24] excluding cromwell app types --- src/analysis/Environments/Environments.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index 2df9b4dd62..89af8b86bd 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -280,14 +280,14 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { includeLabels: 'saturnApplication,saturnWorkspaceNamespace,saturnWorkspaceName', }; - const noCromwellAppTypes = ['GALAXY', 'HAIL_BATCH', 'WDS']; + const cromwellAppTypes = ['CROMWELL', 'WORKFLOWS_APP', 'CROMWELL_RUNNER_APP']; const [newRuntimes, newDisks, newApps] = await Promise.all([ leoRuntimeData.list(listArgs, { signal }), leoDiskData.list(diskArgs, { signal }), // Excluding cromwell/workflows app types for now, as trying to delete these app types puts associated workspace in an error state ( await leoAppData.listWithoutProject(listArgs, { signal }) - ).filter((app) => _.includes(app.appType, noCromwellAppTypes)), + ).filter((app) => !_.includes(app.appType, cromwellAppTypes)), ]); const endTimeForLeoCallsEpochMs = Date.now(); From b50e1288b693c7cada4a58eee9870e9ecc37fb14 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 23 Jan 2024 13:16:46 -0500 Subject: [PATCH 06/24] using environmentPermissions logic --- src/analysis/Environments/DeleteButton.ts | 24 +++++++++++++------ .../Environments/Environments.models.ts | 4 ++++ src/analysis/Environments/Environments.ts | 6 +---- src/analysis/utils/tool-utils.ts | 7 ++++++ .../environmentsPermissions.ts | 11 ++++++++- 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index a7172179ed..e52e3584b3 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -3,10 +3,13 @@ import { ReactNode } from 'react'; import { h } from 'react-hyperscript-helpers'; import { isResourceDeletable } from 'src/analysis/utils/resource-utils'; import { getDisplayRuntimeStatus } from 'src/analysis/utils/runtime-utils'; +import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { Link } from 'src/components/common'; import { makeMenuIcon } from 'src/components/PopupTrigger'; import { isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { LeoRuntimeStatus } from 'src/libs/ajax/leonardo/models/runtime-models'; +import * as Utils from 'src/libs/utils'; +import { makeCromwellAppsNotDeletable } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { DecoratedComputeResource } from './Environments.models'; @@ -18,19 +21,26 @@ export interface DeleteButtonProps { export const DeleteButton = (props: DeleteButtonProps): ReactNode => { const { resource, onClick } = props; const resourceType = isApp(resource) ? 'app' : 'runtime'; - const isDeletable = isResourceDeletable(resourceType, resource); + const appsToDelete = makeCromwellAppsNotDeletable(); + const isDeletable = appsToDelete.canBeDeleted(resource) && isResourceDeletable(resourceType, resource); return h( Link, { disabled: !isDeletable, - tooltip: isDeletable - ? 'Delete cloud environment' - : `Cannot delete a cloud environment while in status ${_.upperCase( - getDisplayRuntimeStatus(resource.status as LeoRuntimeStatus) - )}.`, + tooltip: Utils.cond( + [isDeletable, () => 'Delete cloud environment'], + [Object.keys(cromwellAppToolLabels).includes(resource.appType), () => 'Deleting not yet supported'], + [ + Utils.DEFAULT, + () => + `Cannot delete a cloud environment while in status ${_.upperCase( + getDisplayRuntimeStatus(resource.status as LeoRuntimeStatus) + )}.`, + ] + ), onClick: () => onClick(resource), }, - [makeMenuIcon('trash'), 'Delete'] + [makeMenuIcon('trash', undefined), 'Delete'] ); }; diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index 8bfba33a67..4d9b90b945 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -22,5 +22,9 @@ export interface LeoResourcePermissionsProvider { canPauseResource: (resource: App | ListRuntimeItem) => boolean; } +export interface CanDeleteProvider { + canBeDeleted: (resource: App | ListRuntimeItem) => boolean; +} + export type DeleteRuntimeProvider = Pick; export type DeleteDiskProvider = Pick; diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index db357e5dd1..bbd160cb55 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -132,14 +132,10 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { includeLabels: 'saturnApplication,saturnWorkspaceNamespace,saturnWorkspaceName', }; - const cromwellAppTypes = ['CROMWELL', 'WORKFLOWS_APP', 'CROMWELL_RUNNER_APP']; const [newRuntimes, newDisks, newApps] = await Promise.all([ leoRuntimeData.list(listArgs, { signal }), leoDiskData.list(diskArgs, { signal }), - // Excluding cromwell/workflows app types for now, as trying to delete these app types puts associated workspace in an error state - ( - await leoAppData.listWithoutProject(listArgs, { signal }) - ).filter((app) => !_.includes(app.appType, cromwellAppTypes)), + await leoAppData.listWithoutProject(listArgs, { signal }), ]); const endTimeForLeoCallsEpochMs = Date.now(); diff --git a/src/analysis/utils/tool-utils.ts b/src/analysis/utils/tool-utils.ts index 4eb5d8e29f..fda2c0ca14 100644 --- a/src/analysis/utils/tool-utils.ts +++ b/src/analysis/utils/tool-utils.ts @@ -10,6 +10,7 @@ import { CloudProvider, cloudProviderTypes } from 'src/libs/workspace-utils'; export type RuntimeToolLabel = 'Jupyter' | 'RStudio' | 'JupyterLab'; export type AppToolLabel = 'GALAXY' | 'CROMWELL' | 'HAIL_BATCH' | 'WDS' | 'WORKFLOWS_APP' | 'CROMWELL_RUNNER_APP'; +export type CromwellAppToolLabel = 'CROMWELL' | 'WORKFLOWS_APP' | 'CROMWELL_RUNNER_APP'; export type AppAccessScope = 'USER_PRIVATE' | 'WORKSPACE_SHARED'; export type LaunchableToolLabel = 'spark' | 'terminal' | 'RStudio' | 'JupyterLab'; export type ToolLabel = RuntimeToolLabel | AppToolLabel; @@ -48,6 +49,12 @@ export const appToolLabels: Record = { WDS: 'WDS', }; +export const cromwellAppToolLabels: Record = { + CROMWELL: 'CROMWELL', + WORKFLOWS_APP: 'WORKFLOWS_APP', + CROMWELL_RUNNER_APP: 'CROMWELL_RUNNER_APP', +}; + export const appAccessScopes: Record = { USER_PRIVATE: 'USER_PRIVATE', WORKSPACE_SHARED: 'WORKSPACE_SHARED', diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index 7607f14b2e..458863f005 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -1,5 +1,6 @@ -import { LeoResourcePermissionsProvider } from 'src/analysis/Environments/Environments.models'; +import { CanDeleteProvider, LeoResourcePermissionsProvider } from 'src/analysis/Environments/Environments.models'; import { getCreatorForCompute } from 'src/analysis/utils/resource-utils'; +import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; @@ -21,6 +22,14 @@ const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePerm return permissionsProvider; }; +export const makeCromwellAppsNotDeletable = (): CanDeleteProvider => { + return { + canBeDeleted: (resource: App | ListRuntimeItem) => { + return !Object.keys(cromwellAppToolLabels).includes(resource.appType); + }, + }; +}; + const currentUserEmailGetter = (): string => { return getTerraUser().email!; }; From 8dabb33b0ccfc72e71713e2e2a1aa7dd9d06768b Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 23 Jan 2024 16:00:27 -0500 Subject: [PATCH 07/24] add a test; revert other tests --- .../Environments/Environments.test.ts | 81 ++++++++++++++++--- src/analysis/Environments/Environments.ts | 2 +- 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 4deb3cf19f..7f70d2b082 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -1,5 +1,5 @@ import { NavLinkProvider } from '@terra-ui-packages/core-utils'; -import { act, fireEvent, getAllByRole, screen } from '@testing-library/react'; +import { act, fireEvent, getAllByRole, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import _ from 'lodash/fp'; import { h } from 'react-hyperscript-helpers'; @@ -503,14 +503,75 @@ describe('Environments', () => { expect(getTextContentForColumn(secondAppRow, 7)).toBe(makeCompleteDate(googleApp2.auditInfo.createdDate)); expect(getTextContentForColumn(secondAppRow, 8)).toBe(makeCompleteDate(googleApp1.auditInfo.dateAccessed)); - // Verify that Cromwell apps do not appear on the page - expect(screen.queryByText(azureApp1.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); - expect(screen.queryByText(azureApp2.labels.saturnWorkspaceNamespace)).not.toBeInTheDocument(); + const thirdAppRow: HTMLElement = tableRows[2]; + expect(getTextContentForColumn(thirdAppRow, 0)).toBe(azureApp1.labels.saturnWorkspaceNamespace); + expect(getTextContentForColumn(thirdAppRow, 1)).toBe(azureApp1.labels.saturnWorkspaceName); + expect(getTextContentForColumn(thirdAppRow, 2)).toBe('Kubernetes'); + expect(getTextContentForColumn(thirdAppRow, 3)).toBe(_.capitalize(azureApp1.appType)); + expect(getTextContentForColumn(thirdAppRow, 5)).toBe(_.capitalize(azureApp1.status)); + expect(getTextContentForColumn(thirdAppRow, 6)).toBe(azureApp1.region); + expect(getTextContentForColumn(thirdAppRow, 7)).toBe(makeCompleteDate(azureApp1.auditInfo.createdDate)); + expect(getTextContentForColumn(thirdAppRow, 8)).toBe(makeCompleteDate(azureApp1.auditInfo.dateAccessed)); + + const fourthAppRow: HTMLElement = tableRows[3]; + expect(getTextContentForColumn(fourthAppRow, 0)).toBe(azureApp2.labels.saturnWorkspaceNamespace); + expect(getTextContentForColumn(fourthAppRow, 1)).toBe(azureApp2.labels.saturnWorkspaceName); + expect(getTextContentForColumn(fourthAppRow, 2)).toBe('Kubernetes'); + expect(getTextContentForColumn(fourthAppRow, 3)).toBe(_.capitalize(azureApp2.appType)); + expect(getTextContentForColumn(fourthAppRow, 5)).toBe(_.capitalize(azureApp2.status)); + expect(getTextContentForColumn(fourthAppRow, 6)).toBe(azureApp2.region); + expect(getTextContentForColumn(fourthAppRow, 7)).toBe(makeCompleteDate(azureApp2.auditInfo.createdDate)); + expect(getTextContentForColumn(fourthAppRow, 8)).toBe(makeCompleteDate(azureApp2.auditInfo.dateAccessed)); + }); + + it('Renders Cromwell apps with disabled delete', async () => { + // Arrange + const props = getEnvironmentsProps(); + + const googleApp1 = generateTestAppWithGoogleWorkspace({}, defaultGoogleWorkspace); + const azureApp1 = generateTestAppWithAzureWorkspace({ appType: appToolLabels.CROMWELL }, defaultAzureWorkspace); + const azureWorkspace2 = generateAzureWorkspace(); + const azureApp2 = generateTestAppWithAzureWorkspace({ appType: appToolLabels.WORKFLOWS_APP }, azureWorkspace2); + asMockedFn(props.leoAppData.listWithoutProject).mockResolvedValue([googleApp1, azureApp1, azureApp2]); + asMockedFn(props.useWorkspaces).mockReturnValue({ + ...defaultUseWorkspacesProps, + workspaces: [defaultGoogleWorkspace, defaultAzureWorkspace, azureWorkspace2], + }); + + // Act + await act(async () => { + render(h(Environments, props)); + }); + + // Assert + const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row + const firstAppRow: HTMLElement = tableRows[0]; + const actionColumnButton1 = within(firstAppRow) + .getAllByRole('button') + .filter((button) => button.textContent.includes('Delete'))[0]; + expect(actionColumnButton1).not.toHaveAttribute('disabled'); + + const secondAppRow: HTMLElement = tableRows[1]; + const actionColumnButton2 = within(secondAppRow) + .getAllByRole('button') + .filter((button) => button.textContent.includes('Delete'))[0]; + expect(actionColumnButton2).toHaveAttribute('disabled'); + + const thirdAppRow: HTMLElement = tableRows[2]; + const actionColumnButton3 = within(thirdAppRow) + .getAllByRole('button') + .filter((button) => button.textContent.includes('Delete'))[0]; + expect(actionColumnButton3).toHaveAttribute('disabled'); + + // Check for tooltip, and that it's only present for azure apps + const notSupportedTooltip = screen.getAllByText('Deleting not yet supported'); + expect(notSupportedTooltip).toBeInTheDocument; + expect(notSupportedTooltip.length).toBe(2); }); it.each([ { app: generateTestAppWithGoogleWorkspace({}, defaultGoogleWorkspace), workspace: defaultGoogleWorkspace }, - // Reimplement when Azure apps can be deleted safely { app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), workspace: defaultAzureWorkspace }, + { app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), workspace: defaultAzureWorkspace }, ])('Renders app details view correctly', async ({ app, workspace }) => { // Arrange const props = getEnvironmentsProps(); @@ -548,11 +609,11 @@ describe('Environments', () => { workspace: defaultGoogleWorkspace, isAzure: false, }, - // Reimplement when Azure apps can be deleted safely { - // app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), - // workspace: defaultAzureWorkspace, - // isAzure: true, - // }, + { + app: generateTestAppWithAzureWorkspace({}, defaultAzureWorkspace), + workspace: defaultAzureWorkspace, + isAzure: true, + }, ])('Behaves properly when we click pause/delete for azure/gce app', async ({ app, workspace }) => { // Arrange const user = userEvent.setup(); diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index bbd160cb55..8e1a38cec0 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -135,7 +135,7 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { const [newRuntimes, newDisks, newApps] = await Promise.all([ leoRuntimeData.list(listArgs, { signal }), leoDiskData.list(diskArgs, { signal }), - await leoAppData.listWithoutProject(listArgs, { signal }), + leoAppData.listWithoutProject(listArgs, { signal }), ]); const endTimeForLeoCallsEpochMs = Date.now(); From dcdc6a4d30c3f88b44dc29eafb58eef1b69d5ed0 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 23 Jan 2024 16:29:19 -0500 Subject: [PATCH 08/24] change type of resource --- src/pages/EnvironmentsPage/environmentsPermissions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index 458863f005..59cab11d3e 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -24,7 +24,7 @@ const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePerm export const makeCromwellAppsNotDeletable = (): CanDeleteProvider => { return { - canBeDeleted: (resource: App | ListRuntimeItem) => { + canBeDeleted: (resource: App) => { return !Object.keys(cromwellAppToolLabels).includes(resource.appType); }, }; From fcb1fbc3dfd48b6c568834d87542d45ba43924fb Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 23 Jan 2024 16:48:32 -0500 Subject: [PATCH 09/24] cast to access appType --- src/analysis/Environments/DeleteButton.ts | 8 +++++--- src/analysis/Environments/Environments.models.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index e52e3584b3..34ea4d895c 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -6,7 +6,7 @@ import { getDisplayRuntimeStatus } from 'src/analysis/utils/runtime-utils'; import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { Link } from 'src/components/common'; import { makeMenuIcon } from 'src/components/PopupTrigger'; -import { isApp } from 'src/libs/ajax/leonardo/models/app-models'; +import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { LeoRuntimeStatus } from 'src/libs/ajax/leonardo/models/runtime-models'; import * as Utils from 'src/libs/utils'; import { makeCromwellAppsNotDeletable } from 'src/pages/EnvironmentsPage/environmentsPermissions'; @@ -21,8 +21,10 @@ export interface DeleteButtonProps { export const DeleteButton = (props: DeleteButtonProps): ReactNode => { const { resource, onClick } = props; const resourceType = isApp(resource) ? 'app' : 'runtime'; + // Casting resource to type App to access the appType property + const castToApp = resource as App; const appsToDelete = makeCromwellAppsNotDeletable(); - const isDeletable = appsToDelete.canBeDeleted(resource) && isResourceDeletable(resourceType, resource); + const isDeletable = appsToDelete.canBeDeleted(castToApp) && isResourceDeletable(resourceType, resource); return h( Link, @@ -30,7 +32,7 @@ export const DeleteButton = (props: DeleteButtonProps): ReactNode => { disabled: !isDeletable, tooltip: Utils.cond( [isDeletable, () => 'Delete cloud environment'], - [Object.keys(cromwellAppToolLabels).includes(resource.appType), () => 'Deleting not yet supported'], + [Object.keys(cromwellAppToolLabels).includes(castToApp.appType), () => 'Deleting not yet supported'], [ Utils.DEFAULT, () => diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index 4d9b90b945..df02fcd6f7 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -23,7 +23,7 @@ export interface LeoResourcePermissionsProvider { } export interface CanDeleteProvider { - canBeDeleted: (resource: App | ListRuntimeItem) => boolean; + canBeDeleted: (resource: App) => boolean; } export type DeleteRuntimeProvider = Pick; From 5e09b16da04d1170321cc41daf10e9a69eb4f425 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 23 Jan 2024 17:00:57 -0500 Subject: [PATCH 10/24] update tests --- src/analysis/Environments/Environments.test.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 7f70d2b082..091cbe45f4 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -546,21 +546,16 @@ describe('Environments', () => { // Assert const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row const firstAppRow: HTMLElement = tableRows[0]; - const actionColumnButton1 = within(firstAppRow) - .getAllByRole('button') - .filter((button) => button.textContent.includes('Delete'))[0]; + const actionColumnButton1 = within(firstAppRow).getByRole('button', { name: 'Delete' }); + // .filter((button) => button.textContent.includes('Delete'))[0]; expect(actionColumnButton1).not.toHaveAttribute('disabled'); const secondAppRow: HTMLElement = tableRows[1]; - const actionColumnButton2 = within(secondAppRow) - .getAllByRole('button') - .filter((button) => button.textContent.includes('Delete'))[0]; + const actionColumnButton2 = within(secondAppRow).getByRole('button', { name: 'Delete' }); expect(actionColumnButton2).toHaveAttribute('disabled'); const thirdAppRow: HTMLElement = tableRows[2]; - const actionColumnButton3 = within(thirdAppRow) - .getAllByRole('button') - .filter((button) => button.textContent.includes('Delete'))[0]; + const actionColumnButton3 = within(thirdAppRow).getByRole('button', { name: 'Delete' }); expect(actionColumnButton3).toHaveAttribute('disabled'); // Check for tooltip, and that it's only present for azure apps From f870837e927087528ce1bd0530065b9369df312e Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Wed, 24 Jan 2024 09:42:26 -0500 Subject: [PATCH 11/24] wip --- src/analysis/Environments/DeleteButton.ts | 11 +++++----- .../Environments/Environments.models.ts | 5 +---- .../Environments/Environments.test.ts | 4 +++- src/analysis/Environments/Environments.ts | 1 + .../environmentsPermissions.ts | 20 +++++++++---------- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index 34ea4d895c..9fafdc90e4 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -9,22 +9,23 @@ import { makeMenuIcon } from 'src/components/PopupTrigger'; import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { LeoRuntimeStatus } from 'src/libs/ajax/leonardo/models/runtime-models'; import * as Utils from 'src/libs/utils'; -import { makeCromwellAppsNotDeletable } from 'src/pages/EnvironmentsPage/environmentsPermissions'; -import { DecoratedComputeResource } from './Environments.models'; +import { DecoratedComputeResource, LeoResourcePermissionsProvider } from './Environments.models'; + +type DeletePermissionsProvider = Pick; export interface DeleteButtonProps { resource: DecoratedComputeResource; + permissions: DeletePermissionsProvider; onClick: (resource: DecoratedComputeResource) => void; } export const DeleteButton = (props: DeleteButtonProps): ReactNode => { - const { resource, onClick } = props; + const { resource, permissions, onClick } = props; const resourceType = isApp(resource) ? 'app' : 'runtime'; // Casting resource to type App to access the appType property const castToApp = resource as App; - const appsToDelete = makeCromwellAppsNotDeletable(); - const isDeletable = appsToDelete.canBeDeleted(castToApp) && isResourceDeletable(resourceType, resource); + const isDeletable = isResourceDeletable(resourceType, resource) && permissions.canDeleteApp(resource); return h( Link, diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index df02fcd6f7..4b7c772993 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -20,10 +20,7 @@ export type DecoratedResource = DecoratedComputeResource | DiskWithWorkspace; export interface LeoResourcePermissionsProvider { canDeleteDisk: (disk: PersistentDisk) => boolean; canPauseResource: (resource: App | ListRuntimeItem) => boolean; -} - -export interface CanDeleteProvider { - canBeDeleted: (resource: App) => boolean; + canDeleteApp: (resource: DecoratedComputeResource) => boolean; } export type DeleteRuntimeProvider = Pick; diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 091cbe45f4..a085294205 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -39,6 +39,7 @@ const mockNav: NavLinkProvider = { const mockPermissions: LeoResourcePermissionsProvider = { canDeleteDisk: jest.fn(), canPauseResource: jest.fn(), + canDeleteApp: jest.fn(), }; const defaultUseWorkspacesProps = { @@ -83,6 +84,7 @@ const getMockLeoDiskProvider = (overrides?: Partial): LeoDiskPr const getEnvironmentsProps = (propsOverrides?: Partial): EnvironmentsProps => { asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); + asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); const defaultProps: EnvironmentsProps = { nav: mockNav, @@ -547,11 +549,11 @@ describe('Environments', () => { const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row const firstAppRow: HTMLElement = tableRows[0]; const actionColumnButton1 = within(firstAppRow).getByRole('button', { name: 'Delete' }); - // .filter((button) => button.textContent.includes('Delete'))[0]; expect(actionColumnButton1).not.toHaveAttribute('disabled'); const secondAppRow: HTMLElement = tableRows[1]; const actionColumnButton2 = within(secondAppRow).getByRole('button', { name: 'Delete' }); + screen.debug(undefined, 300000); expect(actionColumnButton2).toHaveAttribute('disabled'); const thirdAppRow: HTMLElement = tableRows[2]; diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index 8e1a38cec0..3ab681f691 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -600,6 +600,7 @@ export const Environments = (props: EnvironmentsProps): ReactNode => { h(PauseButton, { cloudEnvironment, permissions, pauseComputeAndRefresh }), h(DeleteButton, { resource: cloudEnvironment, + permissions, onClick: (resource) => { isApp(resource) ? deleteAppModal.open(resource) : setDeleteRuntimeId(resource.id); }, diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index 59cab11d3e..90f71773f6 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -1,7 +1,10 @@ -import { CanDeleteProvider, LeoResourcePermissionsProvider } from 'src/analysis/Environments/Environments.models'; +import { + DecoratedComputeResource, + LeoResourcePermissionsProvider, +} from 'src/analysis/Environments/Environments.models'; import { getCreatorForCompute } from 'src/analysis/utils/resource-utils'; import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; -import { App } from 'src/libs/ajax/leonardo/models/app-models'; +import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser } from 'src/libs/state'; @@ -18,16 +21,13 @@ const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePerm const creator = getCreatorForCompute(resource); return currentUserEmail === creator; }, - }; - return permissionsProvider; -}; - -export const makeCromwellAppsNotDeletable = (): CanDeleteProvider => { - return { - canBeDeleted: (resource: App) => { - return !Object.keys(cromwellAppToolLabels).includes(resource.appType); + canDeleteApp: (resource: DecoratedComputeResource) => { + if (isApp(resource)) { + return !Object.keys(cromwellAppToolLabels).includes(resource.appType); + } }, }; + return permissionsProvider; }; const currentUserEmailGetter = (): string => { From 6d16f2365e58276fceae753c589c5411bb73c766 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Mon, 29 Jan 2024 18:34:00 -0500 Subject: [PATCH 12/24] new permission and state provider --- src/analysis/Environments/DeleteButton.ts | 15 +- .../Environments/Environments.models.ts | 10 +- .../Environments/Environments.test.ts | 10 +- src/analysis/Environments/Environments.ts | 4 +- src/analysis/Environments/PauseButton.ts | 4 +- src/pages/EnvironmentsPage.ts | 4 +- .../environmentsPermissions.test.ts | 156 +++++++++++++++++- .../environmentsPermissions.ts | 22 ++- 8 files changed, 187 insertions(+), 38 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index 9fafdc90e4..1339557d64 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -1,18 +1,16 @@ import _ from 'lodash/fp'; import { ReactNode } from 'react'; import { h } from 'react-hyperscript-helpers'; -import { isResourceDeletable } from 'src/analysis/utils/resource-utils'; import { getDisplayRuntimeStatus } from 'src/analysis/utils/runtime-utils'; -import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { Link } from 'src/components/common'; import { makeMenuIcon } from 'src/components/PopupTrigger'; -import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; +import { isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { LeoRuntimeStatus } from 'src/libs/ajax/leonardo/models/runtime-models'; import * as Utils from 'src/libs/utils'; -import { DecoratedComputeResource, LeoResourcePermissionsProvider } from './Environments.models'; +import { DecoratedComputeResource, PermissionsAndStateProvider } from './Environments.models'; -type DeletePermissionsProvider = Pick; +type DeletePermissionsProvider = Pick; export interface DeleteButtonProps { resource: DecoratedComputeResource; @@ -23,9 +21,8 @@ export interface DeleteButtonProps { export const DeleteButton = (props: DeleteButtonProps): ReactNode => { const { resource, permissions, onClick } = props; const resourceType = isApp(resource) ? 'app' : 'runtime'; - // Casting resource to type App to access the appType property - const castToApp = resource as App; - const isDeletable = isResourceDeletable(resourceType, resource) && permissions.canDeleteApp(resource); + const isDeletable = + resourceType === 'app' ? permissions.canDeleteApp(resource) : permissions.canDeleteResource(resourceType, resource); return h( Link, @@ -33,7 +30,7 @@ export const DeleteButton = (props: DeleteButtonProps): ReactNode => { disabled: !isDeletable, tooltip: Utils.cond( [isDeletable, () => 'Delete cloud environment'], - [Object.keys(cromwellAppToolLabels).includes(castToApp.appType), () => 'Deleting not yet supported'], + [!permissions.canDeleteApp(resource), () => 'Deleting not yet supported'], [ Utils.DEFAULT, () => diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index 4b7c772993..a9b3c3038f 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -1,6 +1,6 @@ import { App, ListAppItem } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; -import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; +import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { LeoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvider'; import { LeoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { WorkspaceInfo } from 'src/libs/workspace-utils'; @@ -17,11 +17,17 @@ export type AppWithWorkspace = DecoratedResourceAttributes & ListAppItem; export type DecoratedComputeResource = RuntimeWithWorkspace | AppWithWorkspace; export type DecoratedResource = DecoratedComputeResource | DiskWithWorkspace; -export interface LeoResourcePermissionsProvider { +export interface PermissionsAndStateProvider { canDeleteDisk: (disk: PersistentDisk) => boolean; canPauseResource: (resource: App | ListRuntimeItem) => boolean; canDeleteApp: (resource: DecoratedComputeResource) => boolean; + canDeleteResource: (resourceType, resource: App | PersistentDisk | Runtime) => boolean; } +// export interface LeoResourcePermissionsProvider { +// canDeleteDisk: (disk: PersistentDisk) => boolean; +// canPauseResource: (resource: App | ListRuntimeItem) => boolean; +// } + export type DeleteRuntimeProvider = Pick; export type DeleteDiskProvider = Pick; diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index a085294205..73ee21ba22 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -22,11 +22,12 @@ import { LeoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvide import { LeoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { makeCompleteDate } from 'src/libs/utils'; import { WorkspaceWrapper } from 'src/libs/workspace-utils'; +import { stateAndPermissionsProvider } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; import { defaultAzureWorkspace, defaultGoogleWorkspace } from 'src/testing/workspace-fixtures'; import { EnvironmentNavActions, Environments, EnvironmentsProps } from './Environments'; -import { LeoResourcePermissionsProvider } from './Environments.models'; +import { PermissionsAndStateProvider } from './Environments.models'; jest.mock('src/libs/notifications', () => ({ notify: jest.fn(), @@ -36,10 +37,11 @@ const mockNav: NavLinkProvider = { getUrl: jest.fn().mockReturnValue('/'), navTo: jest.fn(), }; -const mockPermissions: LeoResourcePermissionsProvider = { +const mockPermissions: PermissionsAndStateProvider = { canDeleteDisk: jest.fn(), canPauseResource: jest.fn(), canDeleteApp: jest.fn(), + canDeleteResource: jest.fn(), }; const defaultUseWorkspacesProps = { @@ -85,6 +87,7 @@ const getEnvironmentsProps = (propsOverrides?: Partial): Envi asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); + asMockedFn(mockPermissions.canDeleteResource).mockReturnValue(true); const defaultProps: EnvironmentsProps = { nav: mockNav, @@ -539,6 +542,7 @@ describe('Environments', () => { ...defaultUseWorkspacesProps, workspaces: [defaultGoogleWorkspace, defaultAzureWorkspace, azureWorkspace2], }); + props.permissions = stateAndPermissionsProvider; // Act await act(async () => { @@ -549,11 +553,11 @@ describe('Environments', () => { const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row const firstAppRow: HTMLElement = tableRows[0]; const actionColumnButton1 = within(firstAppRow).getByRole('button', { name: 'Delete' }); + screen.logTestingPlaygroundURL(); expect(actionColumnButton1).not.toHaveAttribute('disabled'); const secondAppRow: HTMLElement = tableRows[1]; const actionColumnButton2 = within(secondAppRow).getByRole('button', { name: 'Delete' }); - screen.debug(undefined, 300000); expect(actionColumnButton2).toHaveAttribute('disabled'); const thirdAppRow: HTMLElement = tableRows[2]; diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index 3ab681f691..a06049df9a 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -49,7 +49,7 @@ import { DecoratedComputeResource, DecoratedResourceAttributes, DiskWithWorkspace, - LeoResourcePermissionsProvider, + PermissionsAndStateProvider, RuntimeWithWorkspace, } from './Environments.models'; import { PauseButton } from './PauseButton'; @@ -74,7 +74,7 @@ export interface EnvironmentsProps { leoRuntimeData: LeoRuntimeProviderNeeds; leoDiskData: LeoDiskProviderNeeds; metrics: MetricsProvider; - permissions: LeoResourcePermissionsProvider; + permissions: PermissionsAndStateProvider; } export const Environments = (props: EnvironmentsProps): ReactNode => { diff --git a/src/analysis/Environments/PauseButton.ts b/src/analysis/Environments/PauseButton.ts index 8ad0eaa814..cd3264afc0 100644 --- a/src/analysis/Environments/PauseButton.ts +++ b/src/analysis/Environments/PauseButton.ts @@ -7,9 +7,9 @@ import { makeMenuIcon } from 'src/components/PopupTrigger'; import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; -import { LeoResourcePermissionsProvider } from './Environments.models'; +import { PermissionsAndStateProvider } from './Environments.models'; -type PausePermissionsProvider = Pick; +type PausePermissionsProvider = Pick; interface PauseButtonProps { cloudEnvironment: App | ListRuntimeItem; diff --git a/src/pages/EnvironmentsPage.ts b/src/pages/EnvironmentsPage.ts index 46c1b07537..812751437c 100644 --- a/src/pages/EnvironmentsPage.ts +++ b/src/pages/EnvironmentsPage.ts @@ -9,7 +9,7 @@ import { leoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvide import { leoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { useMetricsEvent } from 'src/libs/ajax/metrics/useMetrics'; import { terraNavKey, TerraNavLinkProvider, terraNavLinkProvider } from 'src/libs/nav'; -import { leoResourcePermissions } from 'src/pages/EnvironmentsPage/environmentsPermissions'; +import { stateAndPermissionsProvider } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { useWorkspaces } from 'src/workspaces/useWorkspaces'; type NavMap = { @@ -48,7 +48,7 @@ export const EnvironmentsPage = (): ReactNode => { leoRuntimeData: leoRuntimeProvider, leoDiskData: leoDiskProvider, metrics: metricsProvider, - permissions: leoResourcePermissions, + permissions: stateAndPermissionsProvider, }), ]); }; diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts index edfc8d335a..190f8ff800 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts @@ -1,10 +1,12 @@ import { DeepPartial } from '@terra-ui-packages/core-utils'; import { asMockedFn } from '@terra-ui-packages/test-utils'; +import { DecoratedComputeResource } from 'src/analysis/Environments/Environments.models'; +import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; -import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; +import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser, TerraUser } from 'src/libs/state'; -import { leoResourcePermissions } from './environmentsPermissions'; +import { stateAndPermissionsProvider } from './environmentsPermissions'; jest.mock('src/libs/state', () => ({ ...jest.requireActual('src/libs/state'), @@ -22,7 +24,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as PersistentDisk; // Act - const canIDeleteDisk = leoResourcePermissions.canDeleteDisk(myDisk); + const canIDeleteDisk = stateAndPermissionsProvider.canDeleteDisk(myDisk); // Assert expect(canIDeleteDisk).toBe(true); @@ -38,7 +40,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as PersistentDisk; // Act - const canIDeleteDisk = leoResourcePermissions.canDeleteDisk(otherDisk); + const canIDeleteDisk = stateAndPermissionsProvider.canDeleteDisk(otherDisk); // Assert expect(canIDeleteDisk).toBe(false); @@ -54,7 +56,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as ListRuntimeItem; // Act - const canIDeleteDisk = leoResourcePermissions.canPauseResource(myRuntime); + const canIDeleteDisk = stateAndPermissionsProvider.canPauseResource(myRuntime); // Assert expect(canIDeleteDisk).toBe(true); @@ -70,9 +72,151 @@ describe('environmentsPermissions', () => { } as DeepPartial as ListRuntimeItem; // Act - const canIDeleteDisk = leoResourcePermissions.canPauseResource(otherRuntime); + const canIDeleteDisk = stateAndPermissionsProvider.canPauseResource(otherRuntime); // Assert expect(canIDeleteDisk).toBe(false); }); + + it.each([ + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'GALAXY', + }, + canDeleteApp: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'CROMWELL', + }, + canDeleteApp: false, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'WORKFLOWS_APP', + }, + canDeleteApp: false, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'CROMWELL_RUNNER_APP', + }, + canDeleteApp: false, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'WDS', + }, + canDeleteApp: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'HAIL_BATCH', + }, + canDeleteApp: true, + resourceType: 'app', + }, + ] as { resource: DecoratedComputeResource; canDeleteApp: boolean }[])( + 'returns proper boolean for app deletion', + ({ resource, canDeleteApp }) => { + expect(stateAndPermissionsProvider.canDeleteApp(resource)).toBe(canDeleteApp); + } + ); + + it.each([ + { + resource: { + workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', + runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', + status: 'Stopped', + }, + canDeleteResource: true, + resourceType: 'runtime', + }, + { + resource: { + workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', + runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', + status: 'Creating', + }, + canDeleteResource: false, + resourceType: 'runtime', + }, + { + resource: { + workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', + runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', + status: 'Creating', + }, + canDeleteResource: false, + resourceType: 'disk', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'GALAXY', + }, + canDeleteResource: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'CROMWELL', + }, + canDeleteResource: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'WORKFLOWS_APP', + }, + canDeleteResource: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'CROMWELL_RUNNER_APP', + }, + canDeleteResource: true, + resourceType: 'app', + }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'RUNNING', + appType: 'WDS', + }, + canDeleteResource: true, + resourceType: 'app', + }, + ] as { resource: App | PersistentDisk | Runtime; canDeleteResource: boolean; resourceType: string }[])( + 'returns correct boolean for resource deletion', + ({ resource, canDeleteResource, resourceType }) => { + expect(stateAndPermissionsProvider.canDeleteResource(resourceType, resource)).toBe(canDeleteResource); + } + ); }); diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index 90f71773f6..ff61c117df 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -1,20 +1,16 @@ -import { - DecoratedComputeResource, - LeoResourcePermissionsProvider, -} from 'src/analysis/Environments/Environments.models'; -import { getCreatorForCompute } from 'src/analysis/utils/resource-utils'; +import { DecoratedComputeResource, PermissionsAndStateProvider } from 'src/analysis/Environments/Environments.models'; +import { getCreatorForCompute, isResourceDeletable } from 'src/analysis/utils/resource-utils'; import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; -import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; +import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser } from 'src/libs/state'; -const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePermissionsProvider => { - const permissionsProvider: LeoResourcePermissionsProvider = { +const makeStateAndPermissionsProvider = (userEmailGetter: () => string): PermissionsAndStateProvider => { + return { canDeleteDisk: (disk: PersistentDisk) => { const currentUserEmail = userEmailGetter(); - const canDelete = disk.auditInfo.creator === currentUserEmail; - return canDelete; + return disk.auditInfo.creator === currentUserEmail; }, canPauseResource: (resource: App | ListRuntimeItem) => { const currentUserEmail = userEmailGetter(); @@ -26,12 +22,14 @@ const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePerm return !Object.keys(cromwellAppToolLabels).includes(resource.appType); } }, + canDeleteResource: (resourceType, resource: App | PersistentDisk | Runtime) => { + return isResourceDeletable(resourceType, resource); + }, }; - return permissionsProvider; }; const currentUserEmailGetter = (): string => { return getTerraUser().email!; }; -export const leoResourcePermissions = makePermissionsProvider(currentUserEmailGetter); +export const stateAndPermissionsProvider = makeStateAndPermissionsProvider(currentUserEmailGetter); From 5d35d621e03ab534f498205ba2e3b7137051a4d7 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Mon, 29 Jan 2024 18:35:17 -0500 Subject: [PATCH 13/24] cleanup --- src/analysis/Environments/Environments.models.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index a9b3c3038f..1900f4b922 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -24,10 +24,5 @@ export interface PermissionsAndStateProvider { canDeleteResource: (resourceType, resource: App | PersistentDisk | Runtime) => boolean; } -// export interface LeoResourcePermissionsProvider { -// canDeleteDisk: (disk: PersistentDisk) => boolean; -// canPauseResource: (resource: App | ListRuntimeItem) => boolean; -// } - export type DeleteRuntimeProvider = Pick; export type DeleteDiskProvider = Pick; From 268eae775f7ff3d8b204bd89e907a6b860f263c7 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Mon, 29 Jan 2024 18:47:01 -0500 Subject: [PATCH 14/24] test cleanup --- src/pages/EnvironmentsPage/EnvironmentsPage.test.ts | 4 ++-- src/pages/EnvironmentsPage/environmentsPermissions.test.ts | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts b/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts index f9b9282eac..270ceb52e4 100644 --- a/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts +++ b/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts @@ -11,7 +11,7 @@ import { useWorkspaces } from 'src/workspaces/useWorkspaces'; import { UseWorkspaces } from 'src/workspaces/useWorkspaces.models'; import { EnvironmentsPage, makeNavProvider, navProvider } from './EnvironmentsPage'; -import { leoResourcePermissions } from './environmentsPermissions'; +import { stateAndPermissionsProvider } from './environmentsPermissions'; jest.mock('src/analysis/Environments/Environments'); @@ -65,7 +65,7 @@ describe('Environments Page', () => { leoAppData: leoAppProvider, leoRuntimeData: leoRuntimeProvider, leoDiskData: leoDiskProvider, - permissions: leoResourcePermissions, + permissions: stateAndPermissionsProvider, metrics: mockMetricsProvider, } satisfies EnvironmentsProps), expect.anything() diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts index 190f8ff800..23d5df7ef2 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts @@ -86,7 +86,6 @@ describe('environmentsPermissions', () => { appType: 'GALAXY', }, canDeleteApp: true, - resourceType: 'app', }, { resource: { @@ -95,7 +94,6 @@ describe('environmentsPermissions', () => { appType: 'CROMWELL', }, canDeleteApp: false, - resourceType: 'app', }, { resource: { @@ -104,7 +102,6 @@ describe('environmentsPermissions', () => { appType: 'WORKFLOWS_APP', }, canDeleteApp: false, - resourceType: 'app', }, { resource: { @@ -113,7 +110,6 @@ describe('environmentsPermissions', () => { appType: 'CROMWELL_RUNNER_APP', }, canDeleteApp: false, - resourceType: 'app', }, { resource: { @@ -122,7 +118,6 @@ describe('environmentsPermissions', () => { appType: 'WDS', }, canDeleteApp: true, - resourceType: 'app', }, { resource: { @@ -131,7 +126,6 @@ describe('environmentsPermissions', () => { appType: 'HAIL_BATCH', }, canDeleteApp: true, - resourceType: 'app', }, ] as { resource: DecoratedComputeResource; canDeleteApp: boolean }[])( 'returns proper boolean for app deletion', From fcf557a4b2b07fdea1af083191a55e5690fa6a7e Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Mon, 29 Jan 2024 19:00:22 -0500 Subject: [PATCH 15/24] test cleanup --- src/analysis/Environments/Environments.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 73ee21ba22..3e78a2fd7a 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -553,7 +553,6 @@ describe('Environments', () => { const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row const firstAppRow: HTMLElement = tableRows[0]; const actionColumnButton1 = within(firstAppRow).getByRole('button', { name: 'Delete' }); - screen.logTestingPlaygroundURL(); expect(actionColumnButton1).not.toHaveAttribute('disabled'); const secondAppRow: HTMLElement = tableRows[1]; From 9814dee51f65541f6c5e9e283b3fed7cf347a9a9 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 30 Jan 2024 10:49:16 -0500 Subject: [PATCH 16/24] test cleanup --- src/analysis/Environments/Environments.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 3e78a2fd7a..a800f3217c 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -242,6 +242,7 @@ describe('Environments', () => { ...defaultUseWorkspacesProps, workspaces: [defaultGoogleWorkspace, defaultAzureWorkspace], }); + props.permissions.canDeleteResource = stateAndPermissionsProvider.canDeleteResource; // Act await act(async () => { @@ -291,6 +292,7 @@ describe('Environments', () => { // TODO: Back to true once https://broadworkbench.atlassian.net/browse/PROD-905 is resolved expect(buttons4[0].getAttribute('aria-disabled')).toBe('false'); expect(buttons4[1].textContent).toBe('Delete'); + screen.logTestingPlaygroundURL(); expect(buttons4[1].getAttribute('aria-disabled')).toBe('true'); }); From 27fead36a21ce2e524d38d6554d0adcca26e8a3a Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 30 Jan 2024 10:51:13 -0500 Subject: [PATCH 17/24] cleanup --- src/analysis/Environments/Environments.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index a800f3217c..2dd660936c 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -292,7 +292,6 @@ describe('Environments', () => { // TODO: Back to true once https://broadworkbench.atlassian.net/browse/PROD-905 is resolved expect(buttons4[0].getAttribute('aria-disabled')).toBe('false'); expect(buttons4[1].textContent).toBe('Delete'); - screen.logTestingPlaygroundURL(); expect(buttons4[1].getAttribute('aria-disabled')).toBe('true'); }); From 8d14b12938df78dce2041117c1ca03925fb62aae Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 30 Jan 2024 11:29:51 -0500 Subject: [PATCH 18/24] tests --- .../Environments/Environments.test.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index 2dd660936c..efdb5d8137 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -37,11 +37,17 @@ const mockNav: NavLinkProvider = { getUrl: jest.fn().mockReturnValue('/'), navTo: jest.fn(), }; +// const mockPermissions = jest.mock('src/pages/EnvironmentsPage/environmentsPermissions', () => ({ +// canDeleteDisk: jest.fn().mockReturnValue(true), +// canPauseResource: jest.fn().mockReturnValue(true), +// canDeleteApp: jest.fn().mockReturnValue(true), +// canDeleteResource: jest.fn().mockReturnValue(true), +// })); const mockPermissions: PermissionsAndStateProvider = { - canDeleteDisk: jest.fn(), - canPauseResource: jest.fn(), - canDeleteApp: jest.fn(), - canDeleteResource: jest.fn(), + canDeleteDisk: jest.fn().mockReturnValue(true), + canPauseResource: jest.fn().mockReturnValue(true), + canDeleteApp: jest.fn().mockReturnValue(true), + canDeleteResource: jest.fn().mockReturnValue(true), }; const defaultUseWorkspacesProps = { @@ -84,10 +90,10 @@ const getMockLeoDiskProvider = (overrides?: Partial): LeoDiskPr }; const getEnvironmentsProps = (propsOverrides?: Partial): EnvironmentsProps => { - asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); - asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); - asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); - asMockedFn(mockPermissions.canDeleteResource).mockReturnValue(true); + // asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); + // asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); + // asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); + // asMockedFn(mockPermissions.canDeleteResource).mockReturnValue(true); const defaultProps: EnvironmentsProps = { nav: mockNav, From 0b708250b66e0080045783f10fb60e01c05d696a Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 30 Jan 2024 13:49:12 -0500 Subject: [PATCH 19/24] tests --- .../Environments/Environments.test.ts | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index efdb5d8137..bb8490d618 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -37,18 +37,6 @@ const mockNav: NavLinkProvider = { getUrl: jest.fn().mockReturnValue('/'), navTo: jest.fn(), }; -// const mockPermissions = jest.mock('src/pages/EnvironmentsPage/environmentsPermissions', () => ({ -// canDeleteDisk: jest.fn().mockReturnValue(true), -// canPauseResource: jest.fn().mockReturnValue(true), -// canDeleteApp: jest.fn().mockReturnValue(true), -// canDeleteResource: jest.fn().mockReturnValue(true), -// })); -const mockPermissions: PermissionsAndStateProvider = { - canDeleteDisk: jest.fn().mockReturnValue(true), - canPauseResource: jest.fn().mockReturnValue(true), - canDeleteApp: jest.fn().mockReturnValue(true), - canDeleteResource: jest.fn().mockReturnValue(true), -}; const defaultUseWorkspacesProps = { workspaces: [defaultGoogleWorkspace] as WorkspaceWrapper[], @@ -90,10 +78,12 @@ const getMockLeoDiskProvider = (overrides?: Partial): LeoDiskPr }; const getEnvironmentsProps = (propsOverrides?: Partial): EnvironmentsProps => { - // asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); - // asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); - // asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); - // asMockedFn(mockPermissions.canDeleteResource).mockReturnValue(true); + const mockPermissions: PermissionsAndStateProvider = { + canDeleteDisk: jest.fn().mockReturnValue(true), + canPauseResource: jest.fn().mockReturnValue(true), + canDeleteApp: jest.fn().mockReturnValue(true), + canDeleteResource: jest.fn().mockReturnValue(true), + }; const defaultProps: EnvironmentsProps = { nav: mockNav, From 967a63205db06fab284ddb23f9892fe89ec7c8ca Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Thu, 1 Feb 2024 14:31:55 -0500 Subject: [PATCH 20/24] renaming PR comment --- src/analysis/Environments/DeleteButton.ts | 4 ++-- src/analysis/Environments/Environments.models.ts | 2 +- src/analysis/Environments/Environments.test.ts | 10 +++++----- src/analysis/Environments/Environments.ts | 4 ++-- src/analysis/Environments/PauseButton.ts | 4 ++-- .../EnvironmentsPage/EnvironmentsPage.test.ts | 4 ++-- src/pages/EnvironmentsPage/EnvironmentsPage.ts | 4 ++-- .../environmentsPermissions.test.ts | 14 +++++++------- .../EnvironmentsPage/environmentsPermissions.ts | 11 +++++++---- 9 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index 1339557d64..d41f41678f 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -8,9 +8,9 @@ import { isApp } from 'src/libs/ajax/leonardo/models/app-models'; import { LeoRuntimeStatus } from 'src/libs/ajax/leonardo/models/runtime-models'; import * as Utils from 'src/libs/utils'; -import { DecoratedComputeResource, PermissionsAndStateProvider } from './Environments.models'; +import { DecoratedComputeResource, LeoResourcePermissionsProvider } from './Environments.models'; -type DeletePermissionsProvider = Pick; +type DeletePermissionsProvider = Pick; export interface DeleteButtonProps { resource: DecoratedComputeResource; diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index 1900f4b922..d6d3db55ef 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -17,7 +17,7 @@ export type AppWithWorkspace = DecoratedResourceAttributes & ListAppItem; export type DecoratedComputeResource = RuntimeWithWorkspace | AppWithWorkspace; export type DecoratedResource = DecoratedComputeResource | DiskWithWorkspace; -export interface PermissionsAndStateProvider { +export interface LeoResourcePermissionsProvider { canDeleteDisk: (disk: PersistentDisk) => boolean; canPauseResource: (resource: App | ListRuntimeItem) => boolean; canDeleteApp: (resource: DecoratedComputeResource) => boolean; diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index bb8490d618..cdd2a2eedb 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -22,12 +22,12 @@ import { LeoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvide import { LeoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { makeCompleteDate } from 'src/libs/utils'; import { WorkspaceWrapper } from 'src/libs/workspace-utils'; -import { stateAndPermissionsProvider } from 'src/pages/EnvironmentsPage/environmentsPermissions'; +import { leoResourcePermissions } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; import { defaultAzureWorkspace, defaultGoogleWorkspace } from 'src/testing/workspace-fixtures'; import { EnvironmentNavActions, Environments, EnvironmentsProps } from './Environments'; -import { PermissionsAndStateProvider } from './Environments.models'; +import { LeoResourcePermissionsProvider } from './Environments.models'; jest.mock('src/libs/notifications', () => ({ notify: jest.fn(), @@ -78,7 +78,7 @@ const getMockLeoDiskProvider = (overrides?: Partial): LeoDiskPr }; const getEnvironmentsProps = (propsOverrides?: Partial): EnvironmentsProps => { - const mockPermissions: PermissionsAndStateProvider = { + const mockPermissions: LeoResourcePermissionsProvider = { canDeleteDisk: jest.fn().mockReturnValue(true), canPauseResource: jest.fn().mockReturnValue(true), canDeleteApp: jest.fn().mockReturnValue(true), @@ -238,7 +238,7 @@ describe('Environments', () => { ...defaultUseWorkspacesProps, workspaces: [defaultGoogleWorkspace, defaultAzureWorkspace], }); - props.permissions.canDeleteResource = stateAndPermissionsProvider.canDeleteResource; + props.permissions.canDeleteResource = leoResourcePermissions.canDeleteResource; // Act await act(async () => { @@ -539,7 +539,7 @@ describe('Environments', () => { ...defaultUseWorkspacesProps, workspaces: [defaultGoogleWorkspace, defaultAzureWorkspace, azureWorkspace2], }); - props.permissions = stateAndPermissionsProvider; + props.permissions = leoResourcePermissions; // Act await act(async () => { diff --git a/src/analysis/Environments/Environments.ts b/src/analysis/Environments/Environments.ts index a06049df9a..3ab681f691 100644 --- a/src/analysis/Environments/Environments.ts +++ b/src/analysis/Environments/Environments.ts @@ -49,7 +49,7 @@ import { DecoratedComputeResource, DecoratedResourceAttributes, DiskWithWorkspace, - PermissionsAndStateProvider, + LeoResourcePermissionsProvider, RuntimeWithWorkspace, } from './Environments.models'; import { PauseButton } from './PauseButton'; @@ -74,7 +74,7 @@ export interface EnvironmentsProps { leoRuntimeData: LeoRuntimeProviderNeeds; leoDiskData: LeoDiskProviderNeeds; metrics: MetricsProvider; - permissions: PermissionsAndStateProvider; + permissions: LeoResourcePermissionsProvider; } export const Environments = (props: EnvironmentsProps): ReactNode => { diff --git a/src/analysis/Environments/PauseButton.ts b/src/analysis/Environments/PauseButton.ts index cd3264afc0..8ad0eaa814 100644 --- a/src/analysis/Environments/PauseButton.ts +++ b/src/analysis/Environments/PauseButton.ts @@ -7,9 +7,9 @@ import { makeMenuIcon } from 'src/components/PopupTrigger'; import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { ListRuntimeItem } from 'src/libs/ajax/leonardo/models/runtime-models'; -import { PermissionsAndStateProvider } from './Environments.models'; +import { LeoResourcePermissionsProvider } from './Environments.models'; -type PausePermissionsProvider = Pick; +type PausePermissionsProvider = Pick; interface PauseButtonProps { cloudEnvironment: App | ListRuntimeItem; diff --git a/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts b/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts index 270ceb52e4..f9b9282eac 100644 --- a/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts +++ b/src/pages/EnvironmentsPage/EnvironmentsPage.test.ts @@ -11,7 +11,7 @@ import { useWorkspaces } from 'src/workspaces/useWorkspaces'; import { UseWorkspaces } from 'src/workspaces/useWorkspaces.models'; import { EnvironmentsPage, makeNavProvider, navProvider } from './EnvironmentsPage'; -import { stateAndPermissionsProvider } from './environmentsPermissions'; +import { leoResourcePermissions } from './environmentsPermissions'; jest.mock('src/analysis/Environments/Environments'); @@ -65,7 +65,7 @@ describe('Environments Page', () => { leoAppData: leoAppProvider, leoRuntimeData: leoRuntimeProvider, leoDiskData: leoDiskProvider, - permissions: stateAndPermissionsProvider, + permissions: leoResourcePermissions, metrics: mockMetricsProvider, } satisfies EnvironmentsProps), expect.anything() diff --git a/src/pages/EnvironmentsPage/EnvironmentsPage.ts b/src/pages/EnvironmentsPage/EnvironmentsPage.ts index 812751437c..46c1b07537 100644 --- a/src/pages/EnvironmentsPage/EnvironmentsPage.ts +++ b/src/pages/EnvironmentsPage/EnvironmentsPage.ts @@ -9,7 +9,7 @@ import { leoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvide import { leoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { useMetricsEvent } from 'src/libs/ajax/metrics/useMetrics'; import { terraNavKey, TerraNavLinkProvider, terraNavLinkProvider } from 'src/libs/nav'; -import { stateAndPermissionsProvider } from 'src/pages/EnvironmentsPage/environmentsPermissions'; +import { leoResourcePermissions } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { useWorkspaces } from 'src/workspaces/useWorkspaces'; type NavMap = { @@ -48,7 +48,7 @@ export const EnvironmentsPage = (): ReactNode => { leoRuntimeData: leoRuntimeProvider, leoDiskData: leoDiskProvider, metrics: metricsProvider, - permissions: stateAndPermissionsProvider, + permissions: leoResourcePermissions, }), ]); }; diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts index 23d5df7ef2..6053764971 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts @@ -6,7 +6,7 @@ import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser, TerraUser } from 'src/libs/state'; -import { stateAndPermissionsProvider } from './environmentsPermissions'; +import { leoResourcePermissions } from './environmentsPermissions'; jest.mock('src/libs/state', () => ({ ...jest.requireActual('src/libs/state'), @@ -24,7 +24,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as PersistentDisk; // Act - const canIDeleteDisk = stateAndPermissionsProvider.canDeleteDisk(myDisk); + const canIDeleteDisk = leoResourcePermissions.canDeleteDisk(myDisk); // Assert expect(canIDeleteDisk).toBe(true); @@ -40,7 +40,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as PersistentDisk; // Act - const canIDeleteDisk = stateAndPermissionsProvider.canDeleteDisk(otherDisk); + const canIDeleteDisk = leoResourcePermissions.canDeleteDisk(otherDisk); // Assert expect(canIDeleteDisk).toBe(false); @@ -56,7 +56,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as ListRuntimeItem; // Act - const canIDeleteDisk = stateAndPermissionsProvider.canPauseResource(myRuntime); + const canIDeleteDisk = leoResourcePermissions.canPauseResource(myRuntime); // Assert expect(canIDeleteDisk).toBe(true); @@ -72,7 +72,7 @@ describe('environmentsPermissions', () => { } as DeepPartial as ListRuntimeItem; // Act - const canIDeleteDisk = stateAndPermissionsProvider.canPauseResource(otherRuntime); + const canIDeleteDisk = leoResourcePermissions.canPauseResource(otherRuntime); // Assert expect(canIDeleteDisk).toBe(false); @@ -130,7 +130,7 @@ describe('environmentsPermissions', () => { ] as { resource: DecoratedComputeResource; canDeleteApp: boolean }[])( 'returns proper boolean for app deletion', ({ resource, canDeleteApp }) => { - expect(stateAndPermissionsProvider.canDeleteApp(resource)).toBe(canDeleteApp); + expect(leoResourcePermissions.canDeleteApp(resource)).toBe(canDeleteApp); } ); @@ -210,7 +210,7 @@ describe('environmentsPermissions', () => { ] as { resource: App | PersistentDisk | Runtime; canDeleteResource: boolean; resourceType: string }[])( 'returns correct boolean for resource deletion', ({ resource, canDeleteResource, resourceType }) => { - expect(stateAndPermissionsProvider.canDeleteResource(resourceType, resource)).toBe(canDeleteResource); + expect(leoResourcePermissions.canDeleteResource(resourceType, resource)).toBe(canDeleteResource); } ); }); diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index ff61c117df..8523461faa 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -1,4 +1,7 @@ -import { DecoratedComputeResource, PermissionsAndStateProvider } from 'src/analysis/Environments/Environments.models'; +import { + DecoratedComputeResource, + LeoResourcePermissionsProvider, +} from 'src/analysis/Environments/Environments.models'; import { getCreatorForCompute, isResourceDeletable } from 'src/analysis/utils/resource-utils'; import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; @@ -6,8 +9,8 @@ import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser } from 'src/libs/state'; -const makeStateAndPermissionsProvider = (userEmailGetter: () => string): PermissionsAndStateProvider => { - return { +const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePermissionsProvider => { + return { canDeleteDisk: (disk: PersistentDisk) => { const currentUserEmail = userEmailGetter(); return disk.auditInfo.creator === currentUserEmail; @@ -32,4 +35,4 @@ const currentUserEmailGetter = (): string => { return getTerraUser().email!; }; -export const stateAndPermissionsProvider = makeStateAndPermissionsProvider(currentUserEmailGetter); +export const leoResourcePermissions = makePermissionsProvider(currentUserEmailGetter); From 2466b5a585a86511d5967ae4b1cdb3ba1a2953dd Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Mon, 5 Feb 2024 12:30:07 -0500 Subject: [PATCH 21/24] stronger type safety in test --- src/analysis/Environments/Environments.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index cdd2a2eedb..9906647a0e 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -79,11 +79,15 @@ const getMockLeoDiskProvider = (overrides?: Partial): LeoDiskPr const getEnvironmentsProps = (propsOverrides?: Partial): EnvironmentsProps => { const mockPermissions: LeoResourcePermissionsProvider = { - canDeleteDisk: jest.fn().mockReturnValue(true), - canPauseResource: jest.fn().mockReturnValue(true), - canDeleteApp: jest.fn().mockReturnValue(true), - canDeleteResource: jest.fn().mockReturnValue(true), + canDeleteDisk: jest.fn(), + canPauseResource: jest.fn(), + canDeleteApp: jest.fn(), + canDeleteResource: jest.fn(), }; + asMockedFn(mockPermissions.canDeleteDisk).mockReturnValue(true); + asMockedFn(mockPermissions.canPauseResource).mockReturnValue(true); + asMockedFn(mockPermissions.canDeleteApp).mockReturnValue(true); + asMockedFn(mockPermissions.canDeleteResource).mockReturnValue(true); const defaultProps: EnvironmentsProps = { nav: mockNav, @@ -547,6 +551,8 @@ describe('Environments', () => { }); // Assert + expect(screen.getAllByRole('row')).toHaveLength(6); + const tableRows: HTMLElement[] = screen.getAllByRole('row').slice(1); // skip header row const firstAppRow: HTMLElement = tableRows[0]; const actionColumnButton1 = within(firstAppRow).getByRole('button', { name: 'Delete' }); From b68ef1b962ae44a6a40562f0591f9106caf27a36 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Tue, 6 Feb 2024 10:54:55 -0500 Subject: [PATCH 22/24] cleanup --- src/analysis/Environments/Environments.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/analysis/Environments/Environments.test.ts b/src/analysis/Environments/Environments.test.ts index ec46caf7dc..5251233419 100644 --- a/src/analysis/Environments/Environments.test.ts +++ b/src/analysis/Environments/Environments.test.ts @@ -21,7 +21,6 @@ import { LeoAppProvider } from 'src/libs/ajax/leonardo/providers/LeoAppProvider' import { LeoDiskProvider } from 'src/libs/ajax/leonardo/providers/LeoDiskProvider'; import { LeoRuntimeProvider } from 'src/libs/ajax/leonardo/providers/LeoRuntimeProvider'; import { makeCompleteDate } from 'src/libs/utils'; -import { WorkspaceWrapper } from 'src/libs/workspace-utils'; import { leoResourcePermissions } from 'src/pages/EnvironmentsPage/environmentsPermissions'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; import { defaultAzureWorkspace, defaultGoogleWorkspace } from 'src/testing/workspace-fixtures'; From 60641fc1ac763a82973abf195b34bdeea9d14859 Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Wed, 7 Feb 2024 11:45:33 -0500 Subject: [PATCH 23/24] PR comments --- src/analysis/Environments/DeleteButton.ts | 4 +- .../Environments/Environments.models.ts | 4 +- src/analysis/utils/resource-utils.ts | 17 ++++----- src/libs/ajax/leonardo/models/disk-models.ts | 5 +++ .../environmentsPermissions.test.ts | 38 ++++++++++++------- .../environmentsPermissions.ts | 17 +++------ 6 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index d41f41678f..cc4fa5310a 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -20,9 +20,7 @@ export interface DeleteButtonProps { export const DeleteButton = (props: DeleteButtonProps): ReactNode => { const { resource, permissions, onClick } = props; - const resourceType = isApp(resource) ? 'app' : 'runtime'; - const isDeletable = - resourceType === 'app' ? permissions.canDeleteApp(resource) : permissions.canDeleteResource(resourceType, resource); + const isDeletable = isApp(resource) ? permissions.canDeleteApp(resource) : permissions.canDeleteResource(resource); return h( Link, diff --git a/src/analysis/Environments/Environments.models.ts b/src/analysis/Environments/Environments.models.ts index c43788b215..cc052590aa 100644 --- a/src/analysis/Environments/Environments.models.ts +++ b/src/analysis/Environments/Environments.models.ts @@ -20,8 +20,8 @@ export type DecoratedResource = DecoratedComputeResource | DiskWithWorkspace; export interface LeoResourcePermissionsProvider { canDeleteDisk: (disk: PersistentDisk) => boolean; canPauseResource: (resource: App | ListRuntimeItem) => boolean; - canDeleteApp: (resource: DecoratedComputeResource) => boolean; - canDeleteResource: (resourceType, resource: App | PersistentDisk | Runtime) => boolean; + canDeleteApp: (resource: App) => boolean; + canDeleteResource: (resource: App | PersistentDisk | Runtime) => boolean; } export type DeleteRuntimeProvider = Pick; diff --git a/src/analysis/utils/resource-utils.ts b/src/analysis/utils/resource-utils.ts index 133427acfd..ee1fb3d673 100644 --- a/src/analysis/utils/resource-utils.ts +++ b/src/analysis/utils/resource-utils.ts @@ -2,7 +2,7 @@ import _ from 'lodash/fp'; import { getAppStatusForDisplay } from 'src/analysis/utils/app-utils'; import { getDisplayRuntimeStatus } from 'src/analysis/utils/runtime-utils'; import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; -import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; +import { isPersistentDisk, PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { isRuntime, Runtime, runtimeStatuses } from 'src/libs/ajax/leonardo/models/runtime-models'; import * as Utils from 'src/libs/utils'; @@ -13,20 +13,17 @@ import * as Utils from 'src/libs/utils'; * https://github.com/DataBiosphere/leonardo/blob/e60c71a9e78b53196c2848cd22a752e22a2cf6f5/core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/diskModels.scala */ // TODO: stop using resourceType here when all types are defined.... -export const isResourceDeletable = (resourceType, resource: App | PersistentDisk | Runtime) => +export const isResourceDeletable = (resource: App | PersistentDisk | Runtime) => _.includes( _.lowerCase(resource?.status), - Utils.switchCase( - resourceType, - ['runtime', () => ['unknown', 'running', 'updating', 'error', 'stopping', 'stopped', 'starting']], - ['app', () => ['unspecified', 'running', 'error']], - ['disk', () => ['failed', 'ready']], + Utils.cond( + [isApp(resource), () => ['unspecified', 'running', 'error']], + [isRuntime(resource), () => ['unknown', 'running', 'updating', 'error', 'stopping', 'stopped', 'starting']], + [isPersistentDisk(resource), () => ['failed', 'ready']], [ Utils.DEFAULT, () => { - console.error( - `Cannot determine deletability; resource type ${resourceType} must be one of runtime, app or disk.` - ); + console.error('Cannot determine deletability; resource type must be one of runtime, app or disk.'); return undefined; }, ] diff --git a/src/libs/ajax/leonardo/models/disk-models.ts b/src/libs/ajax/leonardo/models/disk-models.ts index f8a1882445..a2a3bc2f2e 100644 --- a/src/libs/ajax/leonardo/models/disk-models.ts +++ b/src/libs/ajax/leonardo/models/disk-models.ts @@ -114,3 +114,8 @@ export interface AzurePdSelectOption { } export const azureDiskSizes: number[] = [32, 64, 128, 256, 512, 1024, 2048, 4095]; + +export const isPersistentDisk = (obj: any): obj is PersistentDisk => { + const castDisk = obj as PersistentDisk; + return castDisk && castDisk.diskType !== undefined; +}; diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts index 6053764971..d1d2f8c1b7 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts @@ -1,6 +1,5 @@ import { DeepPartial } from '@terra-ui-packages/core-utils'; import { asMockedFn } from '@terra-ui-packages/test-utils'; -import { DecoratedComputeResource } from 'src/analysis/Environments/Environments.models'; import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; @@ -87,6 +86,14 @@ describe('environmentsPermissions', () => { }, canDeleteApp: true, }, + { + resource: { + appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', + status: 'CREATING', + appType: 'GALAXY', + }, + canDeleteApp: false, + }, { resource: { appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', @@ -127,7 +134,7 @@ describe('environmentsPermissions', () => { }, canDeleteApp: true, }, - ] as { resource: DecoratedComputeResource; canDeleteApp: boolean }[])( + ] as { resource: App; canDeleteApp: boolean }[])( 'returns proper boolean for app deletion', ({ resource, canDeleteApp }) => { expect(leoResourcePermissions.canDeleteApp(resource)).toBe(canDeleteApp); @@ -140,27 +147,37 @@ describe('environmentsPermissions', () => { workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', status: 'Stopped', + runtimeConfig: { + machineType: 'machine', + persistentDiskId: 313, + region: null, + }, + cloudContext: 'AZURE', }, canDeleteResource: true, - resourceType: 'runtime', }, { resource: { workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', status: 'Creating', + runtimeConfig: { + machineType: 'machine', + persistentDiskId: 313, + region: null, + }, + cloudContext: 'AZURE', }, canDeleteResource: false, - resourceType: 'runtime', }, { resource: { workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', status: 'Creating', + diskType: 'abc123', }, canDeleteResource: false, - resourceType: 'disk', }, { resource: { @@ -169,7 +186,6 @@ describe('environmentsPermissions', () => { appType: 'GALAXY', }, canDeleteResource: true, - resourceType: 'app', }, { resource: { @@ -178,7 +194,6 @@ describe('environmentsPermissions', () => { appType: 'CROMWELL', }, canDeleteResource: true, - resourceType: 'app', }, { resource: { @@ -187,7 +202,6 @@ describe('environmentsPermissions', () => { appType: 'WORKFLOWS_APP', }, canDeleteResource: true, - resourceType: 'app', }, { resource: { @@ -196,7 +210,6 @@ describe('environmentsPermissions', () => { appType: 'CROMWELL_RUNNER_APP', }, canDeleteResource: true, - resourceType: 'app', }, { resource: { @@ -205,12 +218,11 @@ describe('environmentsPermissions', () => { appType: 'WDS', }, canDeleteResource: true, - resourceType: 'app', }, - ] as { resource: App | PersistentDisk | Runtime; canDeleteResource: boolean; resourceType: string }[])( + ] as { resource: App | PersistentDisk | Runtime; canDeleteResource: boolean }[])( 'returns correct boolean for resource deletion', - ({ resource, canDeleteResource, resourceType }) => { - expect(leoResourcePermissions.canDeleteResource(resourceType, resource)).toBe(canDeleteResource); + ({ resource, canDeleteResource }) => { + expect(leoResourcePermissions.canDeleteResource(resource)).toBe(canDeleteResource); } ); }); diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.ts b/src/pages/EnvironmentsPage/environmentsPermissions.ts index 8523461faa..6116a6d3ed 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.ts @@ -1,10 +1,7 @@ -import { - DecoratedComputeResource, - LeoResourcePermissionsProvider, -} from 'src/analysis/Environments/Environments.models'; +import { LeoResourcePermissionsProvider } from 'src/analysis/Environments/Environments.models'; import { getCreatorForCompute, isResourceDeletable } from 'src/analysis/utils/resource-utils'; import { cromwellAppToolLabels } from 'src/analysis/utils/tool-utils'; -import { App, isApp } from 'src/libs/ajax/leonardo/models/app-models'; +import { App } from 'src/libs/ajax/leonardo/models/app-models'; import { PersistentDisk } from 'src/libs/ajax/leonardo/models/disk-models'; import { ListRuntimeItem, Runtime } from 'src/libs/ajax/leonardo/models/runtime-models'; import { getTerraUser } from 'src/libs/state'; @@ -20,13 +17,11 @@ const makePermissionsProvider = (userEmailGetter: () => string): LeoResourcePerm const creator = getCreatorForCompute(resource); return currentUserEmail === creator; }, - canDeleteApp: (resource: DecoratedComputeResource) => { - if (isApp(resource)) { - return !Object.keys(cromwellAppToolLabels).includes(resource.appType); - } + canDeleteApp: (resource: App) => { + return isResourceDeletable(resource) && !Object.keys(cromwellAppToolLabels).includes(resource.appType); }, - canDeleteResource: (resourceType, resource: App | PersistentDisk | Runtime) => { - return isResourceDeletable(resourceType, resource); + canDeleteResource: (resource: App | PersistentDisk | Runtime) => { + return isResourceDeletable(resource); }, }; }; From 9125f5b757065283eb366bc0c1c4364dfd666cca Mon Sep 17 00:00:00 2001 From: Katrina Pierre Date: Wed, 7 Feb 2024 13:38:33 -0500 Subject: [PATCH 24/24] CI failures --- src/analysis/Environments/DeleteButton.ts | 2 +- src/analysis/modals/AnalysisModal.ts | 13 ++++--------- .../environmentsPermissions.test.ts | 14 +++++++++++++- .../state/useDeleteWorkspaceState.ts | 4 ++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/analysis/Environments/DeleteButton.ts b/src/analysis/Environments/DeleteButton.ts index cc4fa5310a..07de016db9 100644 --- a/src/analysis/Environments/DeleteButton.ts +++ b/src/analysis/Environments/DeleteButton.ts @@ -28,7 +28,7 @@ export const DeleteButton = (props: DeleteButtonProps): ReactNode => { disabled: !isDeletable, tooltip: Utils.cond( [isDeletable, () => 'Delete cloud environment'], - [!permissions.canDeleteApp(resource), () => 'Deleting not yet supported'], + [isApp(resource) && !permissions.canDeleteApp(resource), () => 'Deleting not yet supported'], [ Utils.DEFAULT, () => diff --git a/src/analysis/modals/AnalysisModal.ts b/src/analysis/modals/AnalysisModal.ts index 97052f64f1..37999030cb 100644 --- a/src/analysis/modals/AnalysisModal.ts +++ b/src/analysis/modals/AnalysisModal.ts @@ -144,14 +144,11 @@ export const AnalysisModal = withDisplayName('AnalysisModal')( Utils.cond( [doesCloudEnvForToolExist, onSuccess], [ - !doesCloudEnvForToolExist && !!currentRuntime && isResourceDeletable('runtime', currentRuntime), + !doesCloudEnvForToolExist && !!currentRuntime && isResourceDeletable(currentRuntime), () => setViewMode(environmentMode), ], [!doesCloudEnvForToolExist && !currentRuntime, () => setViewMode(environmentMode)], - [ - !doesCloudEnvForToolExist && !!currentRuntime && !isResourceDeletable('runtime', currentRuntime), - onSuccess, - ] + [!doesCloudEnvForToolExist && !!currentRuntime && !isResourceDeletable(currentRuntime), onSuccess] ), ], [environmentMode, onSuccess], @@ -368,9 +365,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')( : runtimeToolLabels.JupyterLab; const tool = toolLabel ? tools[toolLabel] : undefined; setCurrentToolObj(tool); - currentRuntime && - !isResourceDeletable('runtime', currentRuntime) && - currentRuntimeToolLabel !== toolLabel + currentRuntime && !isResourceDeletable(currentRuntime) && currentRuntimeToolLabel !== toolLabel ? onSuccess() : enterNextViewMode(tool, analysisMode); uploadFiles(files); @@ -494,7 +489,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')( ]), (isJupyterLab || isRStudio || isJupyter) && currentRuntime && - !isResourceDeletable('runtime', currentRuntime) && + !isResourceDeletable(currentRuntime) && currentRuntimeToolLabel !== toolLabel && div({ style: { backgroundColor: colors.warning(0.1), margin: '0.5rem', padding: '1rem' } }, [ h(WarningTitle, { iconSize: 16 }, [span({ style: { fontWeight: 600 } }, ['Environment Creation'])]), diff --git a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts index d1d2f8c1b7..d090be2621 100644 --- a/src/pages/EnvironmentsPage/environmentsPermissions.test.ts +++ b/src/pages/EnvironmentsPage/environmentsPermissions.test.ts @@ -175,7 +175,7 @@ describe('environmentsPermissions', () => { workspaceId: 'e5795cea-b98f-4b45-a92d-57173e4d1ea4', runtimeName: 'saturn-eb643cb0-e6f3-451c-88cd-0c79edd2df64', status: 'Creating', - diskType: 'abc123', + diskType: 'pd-standard', }, canDeleteResource: false, }, @@ -216,6 +216,18 @@ describe('environmentsPermissions', () => { appName: 'terra-app-3f07f6aa-6531-4151-824d-0ab8d0f19cd1', status: 'RUNNING', appType: 'WDS', + workspaceId: null, + cloudContext: { cloudProvider: 'AZURE', cloudResource: 'string' }, + kubernetesRuntimeConfig: { numNodes: 1, machineType: 'string', autoscalingEnabled: false }, + errors: [], + accessScope: null, + region: 'abc123', + proxyUrls: { + cbas: 'foo.bar', + }, + diskName: null, + auditInfo: { creator: 'foo', createdDate: 'bar', destroyedDate: null, dateAccessed: 'accessed' }, + labels: {}, }, canDeleteResource: true, }, diff --git a/src/workspaces/DeleteWorkspaceModal/state/useDeleteWorkspaceState.ts b/src/workspaces/DeleteWorkspaceModal/state/useDeleteWorkspaceState.ts index 29498fece9..059517e487 100644 --- a/src/workspaces/DeleteWorkspaceModal/state/useDeleteWorkspaceState.ts +++ b/src/workspaces/DeleteWorkspaceModal/state/useDeleteWorkspaceState.ts @@ -61,9 +61,9 @@ export const useDeleteWorkspaceState = (hookArgs: DeleteWorkspaceHookArgs): Dele ? await Ajax(signal).Runtimes.listV2WithWorkspace(workspaceInfo.workspaceId) : []; - const [deletableApps, nonDeletableApps] = _.partition((app) => isResourceDeletable('app', app), apps); + const [deletableApps, nonDeletableApps] = _.partition((app) => isResourceDeletable(app), apps); const [deletableRuntimes, nonDeletableRuntimes] = _.partition( - (runtime) => isResourceDeletable('runtime', runtime), + (runtime) => isResourceDeletable(runtime), currentRuntimesList ); return {