From f068ade7b58a62a9b29f371902633d737fec0ce4 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Mon, 4 Nov 2024 18:37:26 -0500 Subject: [PATCH 01/27] rename to PostMethodProvider --- packages/components/src/Link.test.tsx | 7 +++++ packages/components/src/buttons.test.tsx | 5 ++++ .../providers/CreateMethodProvider.test.ts | 4 +-- ...ethodProvider.ts => PostMethodProvider.ts} | 8 +++--- src/pages/workflows/WorkflowList.test.tsx | 6 ++--- src/pages/workflows/WorkflowList.tsx | 4 +-- .../workflow/common/WorkflowModal.test.tsx | 26 +++++++++---------- .../workflow/common/WorkflowModal.tsx | 8 +++--- 8 files changed, 40 insertions(+), 28 deletions(-) rename src/libs/ajax/methods/providers/{CreateMethodProvider.ts => PostMethodProvider.ts} (86%) diff --git a/packages/components/src/Link.test.tsx b/packages/components/src/Link.test.tsx index b16d55983d..93390f8a86 100644 --- a/packages/components/src/Link.test.tsx +++ b/packages/components/src/Link.test.tsx @@ -28,9 +28,11 @@ describe('Link', () => { it('has a light variant that is styled differently', () => { // Act render(); + // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); + // @ts-ignore const lightVariantStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -40,9 +42,11 @@ describe('Link', () => { it('can be styled with a different base color', () => { // Act render(); + // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render( 'blue'} />); + // @ts-ignore const baseColorStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -53,9 +57,11 @@ describe('Link', () => { it('is styled differently', () => { // Act render(); + // @ts-ignore const enabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); + // @ts-ignore const disabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -65,6 +71,7 @@ describe('Link', () => { it('has no hover style', () => { // Act render(); + // @ts-ignore const disabledHoverStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].hover; // Assert diff --git a/packages/components/src/buttons.test.tsx b/packages/components/src/buttons.test.tsx index 1181b954fd..c8e5c7fd64 100644 --- a/packages/components/src/buttons.test.tsx +++ b/packages/components/src/buttons.test.tsx @@ -31,9 +31,11 @@ describe.each([{ component: ButtonPrimary }, { component: ButtonSecondary }, { c it('is styled differently', () => { // Act render(); + // @ts-ignore const enabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); + // @ts-ignore const disabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -43,6 +45,7 @@ describe.each([{ component: ButtonPrimary }, { component: ButtonSecondary }, { c it('has no hover style', () => { // Act render(); + // @ts-ignore const disabledHoverStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].hover; // Assert @@ -56,9 +59,11 @@ describe('ButtonPrimary', () => { it('can be styled differently to indicate a dangerous action', () => { // Act render(); + // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); + // @ts-ignore const dangerStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert diff --git a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts b/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts index 3a097eb8cd..5fcae413ec 100644 --- a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts +++ b/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts @@ -1,6 +1,6 @@ import { Methods, MethodsAjaxContract } from 'src/libs/ajax/methods/Methods'; import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; -import { createMethodProvider } from 'src/libs/ajax/methods/providers/CreateMethodProvider'; +import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import { asMockedFn, partial } from 'src/testing/test-utils'; jest.mock('src/libs/ajax/methods/Methods'); @@ -37,7 +37,7 @@ describe('create method provider', () => { const signal = new window.AbortController().signal; // Act - const result = await createMethodProvider.create( + const result = await postMethodProvider.create( 'input-namespace', 'input-name', 'workflow input {}', diff --git a/src/libs/ajax/methods/providers/CreateMethodProvider.ts b/src/libs/ajax/methods/providers/PostMethodProvider.ts similarity index 86% rename from src/libs/ajax/methods/providers/CreateMethodProvider.ts rename to src/libs/ajax/methods/providers/PostMethodProvider.ts index e77ec47a00..5dbbdd0bbf 100644 --- a/src/libs/ajax/methods/providers/CreateMethodProvider.ts +++ b/src/libs/ajax/methods/providers/PostMethodProvider.ts @@ -2,8 +2,8 @@ import { AbortOption } from '@terra-ui-packages/data-client-core'; import { Methods } from 'src/libs/ajax/methods/Methods'; import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; -export interface CreateMethodProvider { - create: ( +export interface PostMethodProvider { + postMethod: ( namespace: string, name: string, wdl: string, @@ -14,8 +14,8 @@ export interface CreateMethodProvider { ) => Promise; } -export const createMethodProvider: CreateMethodProvider = { - create: async ( +export const postMethodProvider: PostMethodProvider = { + postMethod: async ( namespace: string, name: string, wdl: string, diff --git a/src/pages/workflows/WorkflowList.test.tsx b/src/pages/workflows/WorkflowList.test.tsx index 72147f9c5b..06a2d6fd1e 100644 --- a/src/pages/workflows/WorkflowList.test.tsx +++ b/src/pages/workflows/WorkflowList.test.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { Ajax, AjaxContract } from 'src/libs/ajax'; import { MethodsAjaxContract } from 'src/libs/ajax/methods/Methods'; import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; -import { createMethodProvider } from 'src/libs/ajax/methods/providers/CreateMethodProvider'; +import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import * as Nav from 'src/libs/nav'; import { getLink } from 'src/libs/nav'; import { notify } from 'src/libs/notifications'; @@ -950,7 +950,7 @@ describe('create workflow modal', () => { // Arrange asMockedFn(Ajax).mockImplementation(() => mockAjax([]) as AjaxContract); - jest.spyOn(createMethodProvider, 'create').mockResolvedValue(mockCreateMethodResponse); + jest.spyOn(postMethodProvider, 'create').mockResolvedValue(mockCreateMethodResponse); const user: UserEvent = userEvent.setup(); @@ -973,7 +973,7 @@ describe('create workflow modal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(createMethodProvider.create).toHaveBeenCalled(); + expect(postMethodProvider.create).toHaveBeenCalled(); expect(Nav.goToPath).toHaveBeenCalledWith('workflow-dashboard', { name: 'response-name', diff --git a/src/pages/workflows/WorkflowList.tsx b/src/pages/workflows/WorkflowList.tsx index 9fbeaee8f4..d7a631cd4a 100644 --- a/src/pages/workflows/WorkflowList.tsx +++ b/src/pages/workflows/WorkflowList.tsx @@ -10,7 +10,7 @@ import { TabBar } from 'src/components/tabBars'; import { FlexTable, HeaderCell, Paginator, Sortable, TooltipCell } from 'src/components/table'; import { TopBar } from 'src/components/TopBar'; import { Ajax } from 'src/libs/ajax'; -import { createMethodProvider } from 'src/libs/ajax/methods/providers/CreateMethodProvider'; +import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import * as Nav from 'src/libs/nav'; import { notify } from 'src/libs/notifications'; import { useCancellation, useOnMount } from 'src/libs/react-utils'; @@ -291,7 +291,7 @@ export const WorkflowList = (props: WorkflowListProps) => { setCreateWorkflowModalOpen(false)} /> diff --git a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx index 75b7848ce4..80932ca1ee 100644 --- a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx +++ b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx @@ -3,7 +3,7 @@ import userEvent, { UserEvent } from '@testing-library/user-event'; import _ from 'lodash/fp'; import React from 'react'; import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; -import { CreateMethodProvider } from 'src/libs/ajax/methods/providers/CreateMethodProvider'; +import { CreateMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import { WorkflowModal } from 'src/pages/workflows/workflow/common/WorkflowModal'; import { renderWithAppContexts as render } from 'src/testing/test-utils'; @@ -52,7 +52,7 @@ describe('WorkflowModal', () => { @@ -87,7 +87,7 @@ describe('WorkflowModal', () => { title='Create New Method' buttonActionName='Upload' defaultWdl='a' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -130,7 +130,7 @@ describe('WorkflowModal', () => { defaultNamespace=',' defaultName=',' defaultWdl='a' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -164,7 +164,7 @@ describe('WorkflowModal', () => { buttonActionName='Upload' defaultNamespace={longStringNamespace} defaultName={longStringName} - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -193,7 +193,7 @@ describe('WorkflowModal', () => { buttonActionName='Upload' defaultNamespace='a' defaultName='a' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -225,7 +225,7 @@ describe('WorkflowModal', () => { defaultNamespace='a' defaultName='a' defaultSynopsis={longSynopsis} - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -260,7 +260,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='old wdl' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -299,7 +299,7 @@ describe('WorkflowModal', () => { defaultDocumentation='test docs' defaultSynopsis='test synopsis' defaultSnapshotComment='test comment' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -350,7 +350,7 @@ describe('WorkflowModal', () => { defaultDocumentation='test docs' defaultSynopsis='test synopsis' defaultSnapshotComment='test comment' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -398,7 +398,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - createMethodProvider={errorCreateMethodProvider} + postMethodProvider={errorCreateMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -431,7 +431,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - createMethodProvider={thrownResponseCreateMethodProvider} + postMethodProvider={thrownResponseCreateMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -464,7 +464,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - createMethodProvider={successCreateMethodProvider} + postMethodProvider={successCreateMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> diff --git a/src/pages/workflows/workflow/common/WorkflowModal.tsx b/src/pages/workflows/workflow/common/WorkflowModal.tsx index 1cc3206490..95ecbd3523 100644 --- a/src/pages/workflows/workflow/common/WorkflowModal.tsx +++ b/src/pages/workflows/workflow/common/WorkflowModal.tsx @@ -5,7 +5,7 @@ import React, { useState } from 'react'; import Dropzone from 'src/components/Dropzone'; import ErrorView from 'src/components/ErrorView'; import { TextArea, TextInput, ValidatedInput } from 'src/components/input'; -import { CreateMethodProvider } from 'src/libs/ajax/methods/providers/CreateMethodProvider'; +import { PostMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import colors from 'src/libs/colors'; import { FormLabel } from 'src/libs/forms'; import * as Utils from 'src/libs/utils'; @@ -61,7 +61,7 @@ export interface WorkflowModalProps { * operation. The create function provided is called with the information * inputted into the modal. */ - createMethodProvider: CreateMethodProvider; + postMethodProvider: PostMethodProvider; /** * The function to be called with the namespace, name, and snapshot ID of the @@ -302,7 +302,7 @@ export const WorkflowModal = (props: WorkflowModalProps) => { defaultDocumentation, defaultSynopsis, defaultSnapshotComment, - createMethodProvider, + postMethodProvider, onSuccess, onDismiss, } = props; @@ -328,7 +328,7 @@ export const WorkflowModal = (props: WorkflowModalProps) => { namespace: createdWorkflowNamespace, name: createdWorkflowName, snapshotId: createdWorkflowSnapshotId, - } = await createMethodProvider.create(namespace, name, wdl, documentation, synopsis, snapshotComment); + } = await postMethodProvider.postMethod(namespace, name, wdl, documentation, synopsis, snapshotComment); onSuccess(createdWorkflowNamespace, createdWorkflowName, createdWorkflowSnapshotId); } catch (error) { setSubmissionError(error instanceof Response ? await error.text() : error); From 7d2648fecd84979431efcbe94c8be9daa3fd6ae6 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Wed, 6 Nov 2024 16:37:05 -0500 Subject: [PATCH 02/27] working clone method --- .../workflows/workflow/WorkflowWrapper.ts | 22 +++++++++++++++++++ src/workflows/SnapshotActionMenu.tsx | 11 ++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/pages/workflows/workflow/WorkflowWrapper.ts b/src/pages/workflows/workflow/WorkflowWrapper.ts index 39a1af4cc3..ef5d1d39a3 100644 --- a/src/pages/workflows/workflow/WorkflowWrapper.ts +++ b/src/pages/workflows/workflow/WorkflowWrapper.ts @@ -8,6 +8,7 @@ import FooterWrapper from 'src/components/FooterWrapper'; import { TabBar } from 'src/components/tabBars'; import { TopBar } from 'src/components/TopBar'; import { Ajax } from 'src/libs/ajax'; +import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import { makeExportWorkflowFromMethodsRepoProvider } from 'src/libs/ajax/workspaces/providers/ExportWorkflowToWorkspaceProvider'; import { ErrorCallback, withErrorReporting } from 'src/libs/error'; import * as Nav from 'src/libs/nav'; @@ -17,6 +18,7 @@ import * as Style from 'src/libs/style'; import * as Utils from 'src/libs/utils'; import { withBusyState } from 'src/libs/utils'; import { PermissionsModal } from 'src/pages/workflows/workflow/common/PermissionsModal'; +import { WorkflowModal } from 'src/pages/workflows/workflow/common/WorkflowModal'; import { Snapshot } from 'src/snapshots/Snapshot'; import DeleteSnapshotModal from 'src/workflows/modals/DeleteSnapshotModal'; import ExportWorkflowModal from 'src/workflows/modals/ExportWorkflowModal'; @@ -122,6 +124,7 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { const [snapshotNotFound, setSnapshotNotFound] = useState(false); const [exportingWorkflow, setExportingWorkflow] = useState(false); const [showDeleteModal, setShowDeleteModal] = useState(false); + const [showCloneModal, setShowCloneModal] = useState(false); const [busy, setBusy] = useState(false); const [permissionsModalOpen, setPermissionsModalOpen] = useState(false); @@ -239,6 +242,7 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { isSnapshotOwner, onEditPermissions: () => setPermissionsModalOpen(true), onDelete: () => setShowDeleteModal(true), + onClone: () => setShowCloneModal(true), }), ]), ] @@ -289,6 +293,24 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { setPermissionsModalOpen, refresh: loadSnapshot, }), + showCloneModal && + h(WorkflowModal, { + title: 'Clone method', + defaultName: name.concat('_copy'), + defaultWdl: snapshot?.payload, + defaultDocumentation: snapshot?.documentation, + defaultSynopsis: snapshot?.synopsis, + defaultSnapshotComment: snapshot?.snapshotComment, + buttonActionName: 'Clone method', + postMethodProvider, + onSuccess: (namespace: string, name: string, snapshotId: number) => + Nav.goToPath('workflow-dashboard', { + namespace, + name, + snapshotId, + }), + onDismiss: () => setShowCloneModal(false), + }), busy && spinnerOverlay, snapshotNotFound && h(NotFoundMessage, { subject: 'snapshot' }), snapshot && div({ style: { flex: 1, display: 'flex', flexDirection: 'column' } }, [children]), diff --git a/src/workflows/SnapshotActionMenu.tsx b/src/workflows/SnapshotActionMenu.tsx index 9c3306a024..c6b0da6d6e 100644 --- a/src/workflows/SnapshotActionMenu.tsx +++ b/src/workflows/SnapshotActionMenu.tsx @@ -24,16 +24,19 @@ export interface SnapshotActionMenuProps { /** The action to be performed if the "Delete snapshot" button is pressed. */ onDelete: () => void; + + /** The action to be performed if the "Clone snapshot" button is pressed. */ + onClone: () => void; } /** * A kebab (vertical three-dot) menu that displays buttons to perform actions on * a workflow snapshot. * - * Currently supported actions: edit permissions, delete snapshot + * Currently supported actions: edit permissions, delete snapshot, clone snapshot */ const SnapshotActionMenu = (props: SnapshotActionMenuProps): ReactNode => { - const { disabled, isSnapshotOwner, onEditPermissions, onDelete } = props; + const { disabled, isSnapshotOwner, onEditPermissions, onDelete, onClone } = props; const notSnapshotOwnerTooltip = 'You must be an owner of this snapshot'; @@ -48,6 +51,10 @@ const SnapshotActionMenu = (props: SnapshotActionMenuProps): ReactNode => { {makeMenuIcon('cog')} Edit snapshot permissions + + {makeMenuIcon('copy')} + Clone snapshot + Date: Wed, 6 Nov 2024 18:01:41 -0500 Subject: [PATCH 03/27] clone tests --- .../workflow/common/WorkflowModal.test.tsx | 110 +++++++++++++----- src/workflows/SnapshotActionMenu.test.tsx | 80 ++++++++++++- 2 files changed, 157 insertions(+), 33 deletions(-) diff --git a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx index 80932ca1ee..661fd1d2d3 100644 --- a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx +++ b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx @@ -3,7 +3,7 @@ import userEvent, { UserEvent } from '@testing-library/user-event'; import _ from 'lodash/fp'; import React from 'react'; import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; -import { CreateMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; +import { PostMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import { WorkflowModal } from 'src/pages/workflows/workflow/common/WorkflowModal'; import { renderWithAppContexts as render } from 'src/testing/test-utils'; @@ -28,20 +28,20 @@ const mockCreateMethodResponse: MethodResponse = { url: 'http://agora.dsde-dev.broadinstitute.org/api/v1/methods/sschu/response-test/1', }; -const errorCreateMethodProvider: CreateMethodProvider = { - create: jest.fn(() => { +const errorPostMethodProvider: PostMethodProvider = { + postMethod: jest.fn(() => { throw new Error('You have not yet risen to the status of Expert WDL Engineer.'); }), }; -const thrownResponseCreateMethodProvider: CreateMethodProvider = { - create: jest.fn(() => { +const thrownResponsePostMethodProvider: PostMethodProvider = { + postMethod: jest.fn(() => { throw new Response('You have not yet risen to the status of Expert WDL Engineer.'); }), }; -const successCreateMethodProvider: CreateMethodProvider = { - create: jest.fn().mockResolvedValue(mockCreateMethodResponse), +const successPostMethodProvider: PostMethodProvider = { + postMethod: jest.fn().mockResolvedValue(mockCreateMethodResponse), }; describe('WorkflowModal', () => { @@ -52,7 +52,7 @@ describe('WorkflowModal', () => { @@ -87,7 +87,7 @@ describe('WorkflowModal', () => { title='Create New Method' buttonActionName='Upload' defaultWdl='a' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -130,7 +130,7 @@ describe('WorkflowModal', () => { defaultNamespace=',' defaultName=',' defaultWdl='a' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -164,7 +164,7 @@ describe('WorkflowModal', () => { buttonActionName='Upload' defaultNamespace={longStringNamespace} defaultName={longStringName} - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -193,7 +193,7 @@ describe('WorkflowModal', () => { buttonActionName='Upload' defaultNamespace='a' defaultName='a' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -225,7 +225,7 @@ describe('WorkflowModal', () => { defaultNamespace='a' defaultName='a' defaultSynopsis={longSynopsis} - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -260,7 +260,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='old wdl' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={jest.fn()} onDismiss={jest.fn()} /> @@ -299,7 +299,7 @@ describe('WorkflowModal', () => { defaultDocumentation='test docs' defaultSynopsis='test synopsis' defaultSnapshotComment='test comment' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -318,8 +318,8 @@ describe('WorkflowModal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(successCreateMethodProvider.create).toHaveBeenCalledTimes(1); - expect(successCreateMethodProvider.create).toHaveBeenCalledWith( + expect(successPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); + expect(successPostMethodProvider.postMethod).toHaveBeenCalledWith( 'newnamespace', 'newname', 'workflow new {}', @@ -350,7 +350,7 @@ describe('WorkflowModal', () => { defaultDocumentation='test docs' defaultSynopsis='test synopsis' defaultSnapshotComment='test comment' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -369,8 +369,8 @@ describe('WorkflowModal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(successCreateMethodProvider.create).toHaveBeenCalledTimes(1); - expect(successCreateMethodProvider.create).toHaveBeenCalledWith( + expect(successPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); + expect(successPostMethodProvider.postMethod).toHaveBeenCalledWith( 'testnamespace', 'testname', 'workflow hi {}', @@ -398,7 +398,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - postMethodProvider={errorCreateMethodProvider} + postMethodProvider={errorPostMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -408,8 +408,8 @@ describe('WorkflowModal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(errorCreateMethodProvider.create).toHaveBeenCalledTimes(1); - expect(errorCreateMethodProvider.create).toHaveBeenCalledWith('namespace', 'name', 'a', '', '', ''); + expect(errorPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); + expect(errorPostMethodProvider.postMethod).toHaveBeenCalledWith('namespace', 'name', 'a', '', '', ''); expect(screen.getByText('You have not yet risen to the status of Expert WDL Engineer.')).toBeInTheDocument(); expect(mockOnSuccess).not.toHaveBeenCalled(); expect(mockOnDismiss).not.toHaveBeenCalled(); @@ -431,7 +431,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - postMethodProvider={thrownResponseCreateMethodProvider} + postMethodProvider={thrownResponsePostMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -441,8 +441,8 @@ describe('WorkflowModal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(thrownResponseCreateMethodProvider.create).toHaveBeenCalledTimes(1); - expect(thrownResponseCreateMethodProvider.create).toHaveBeenCalledWith('namespace', 'name', 'a', '', '', ''); + expect(thrownResponsePostMethodProvider.postMethod).toHaveBeenCalledTimes(1); + expect(thrownResponsePostMethodProvider.postMethod).toHaveBeenCalledWith('namespace', 'name', 'a', '', '', ''); expect(screen.getByText('You have not yet risen to the status of Expert WDL Engineer.')).toBeInTheDocument(); expect(mockOnSuccess).not.toHaveBeenCalled(); expect(mockOnDismiss).not.toHaveBeenCalled(); @@ -464,7 +464,7 @@ describe('WorkflowModal', () => { defaultNamespace='namespace' defaultName='name' defaultWdl='a' - postMethodProvider={successCreateMethodProvider} + postMethodProvider={successPostMethodProvider} onSuccess={mockOnSuccess} onDismiss={mockOnDismiss} /> @@ -474,8 +474,62 @@ describe('WorkflowModal', () => { await user.click(screen.getByRole('button', { name: 'Cancel' })); // Assert - expect(successCreateMethodProvider.create).not.toHaveBeenCalled(); + expect(successPostMethodProvider.postMethod).not.toHaveBeenCalled(); expect(mockOnSuccess).not.toHaveBeenCalled(); expect(mockOnDismiss).toHaveBeenCalled(); }); + + it('clone method modal', async () => { + // Arrange + const mockOnSuccess = jest.fn(); + const mockOnDismiss = jest.fn(); + + const user: UserEvent = userEvent.setup(); + + // Act + await act(async () => { + render( + + ); + }); + + // Assert + expect(screen.getByRole('textbox', { name: 'Name *' })).toHaveDisplayValue('groot-scientific-workflow_copy'); + expect(screen.getByTestId('wdl editor')).toHaveDisplayValue('workflow do-great-stuff {}'); + expect(screen.getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('I am Groot'); + expect(screen.getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue('I am Groot'); + expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue('I am Groot'); + + // user enters value for 'Namespace' text box + fireEvent.change(screen.getByRole('textbox', { name: 'Namespace *' }), { + target: { value: 'groot-test-namespace' }, + }); + + // Act + await user.click(screen.getByRole('button', { name: 'Clone method' })); + + // Assert + expect(successPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); + expect(successPostMethodProvider.postMethod).toHaveBeenCalledWith( + 'groot-test-namespace', + 'groot-scientific-workflow_copy', + 'workflow do-great-stuff {}', + 'I am Groot', + 'I am Groot', + 'I am Groot' + ); + expect(mockOnSuccess).toHaveBeenCalledWith('response-namespace', 'response-name', 1); + expect(mockOnDismiss).not.toHaveBeenCalled(); + }); }); diff --git a/src/workflows/SnapshotActionMenu.test.tsx b/src/workflows/SnapshotActionMenu.test.tsx index 657ee7dc9d..5e0de8713d 100644 --- a/src/workflows/SnapshotActionMenu.test.tsx +++ b/src/workflows/SnapshotActionMenu.test.tsx @@ -6,6 +6,7 @@ import SnapshotActionMenu from 'src/workflows/SnapshotActionMenu'; const mockOnDelete = jest.fn(); const mockOnEditPermissions = jest.fn(); +const mockOnClone = jest.fn(); describe('snapshot action menu', () => { it('honors the disabled prop', async () => { @@ -17,6 +18,7 @@ describe('snapshot action menu', () => { isSnapshotOwner onEditPermissions={mockOnEditPermissions} onDelete={mockOnDelete} + onClone={mockOnClone} /> ); }); @@ -34,7 +36,14 @@ describe('snapshot action menu', () => { // Act await act(async () => { - render(); + render( + + ); }); // Assert @@ -64,16 +73,29 @@ describe('snapshot action menu', () => { expect(deleteSnapshotButton).toBeInTheDocument(); expect(deleteSnapshotButton).toHaveAttribute('aria-disabled', 'false'); expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + + // Act + const cloneSnapshotButton = screen.getByRole('button', { name: 'Clone snapshot' }); + await user.pointer({ target: cloneSnapshotButton }); + + // Assert + expect(cloneSnapshotButton).toBeInTheDocument(); + expect(cloneSnapshotButton).toHaveAttribute('aria-disabled', 'false'); }); - it('renders the menu buttons but disables the proper ones if you are not the snapshot owner', async () => { + it('renders the menu buttons based on snapshot ownership', async () => { // Arrange const user: UserEvent = userEvent.setup(); // Act await act(async () => { render( - + ); }); @@ -104,6 +126,14 @@ describe('snapshot action menu', () => { expect(deleteSnapshotButton).toBeInTheDocument(); expect(deleteSnapshotButton).toHaveAttribute('aria-disabled', 'true'); expect(screen.getByRole('tooltip')).toBeInTheDocument(); + + // Act + const cloneSnapshotButton = screen.getByRole('button', { name: 'Clone snapshot' }); + await user.pointer({ target: cloneSnapshotButton }); + + // Assert + expect(cloneSnapshotButton).toBeInTheDocument(); + expect(cloneSnapshotButton).toHaveAttribute('aria-disabled', 'false'); }); }); @@ -114,7 +144,14 @@ describe('snapshot action menu edit snapshot permissions button', () => { // Act await act(async () => { - render(); + render( + + ); }); await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); @@ -133,7 +170,14 @@ describe('snapshot action menu delete snapshot button', () => { // Act await act(async () => { - render(); + render( + + ); }); await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); @@ -145,3 +189,29 @@ describe('snapshot action menu delete snapshot button', () => { expect(mockOnEditPermissions).not.toHaveBeenCalled(); }); }); + +describe('snapshot action menu clone snapshot button', () => { + it('closes and calls the onClone callback when clicked', async () => { + // Arrange + const user: UserEvent = userEvent.setup(); + + // Act + await act(async () => { + render( + + ); + }); + + await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); + await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + + // Assert + expect(screen.queryByRole('button', { name: 'Clone snapshot' })).not.toBeInTheDocument(); + expect(mockOnClone).toHaveBeenCalled(); + }); +}); From df47b02dc540da80d67c0aab6ba2738f0bdfb4d5 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Wed, 6 Nov 2024 18:03:58 -0500 Subject: [PATCH 04/27] revert unintentional changes --- packages/components/src/Link.test.tsx | 7 ------- packages/components/src/buttons.test.tsx | 5 ----- 2 files changed, 12 deletions(-) diff --git a/packages/components/src/Link.test.tsx b/packages/components/src/Link.test.tsx index 93390f8a86..b16d55983d 100644 --- a/packages/components/src/Link.test.tsx +++ b/packages/components/src/Link.test.tsx @@ -28,11 +28,9 @@ describe('Link', () => { it('has a light variant that is styled differently', () => { // Act render(); - // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); - // @ts-ignore const lightVariantStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -42,11 +40,9 @@ describe('Link', () => { it('can be styled with a different base color', () => { // Act render(); - // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render( 'blue'} />); - // @ts-ignore const baseColorStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -57,11 +53,9 @@ describe('Link', () => { it('is styled differently', () => { // Act render(); - // @ts-ignore const enabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); - // @ts-ignore const disabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -71,7 +65,6 @@ describe('Link', () => { it('has no hover style', () => { // Act render(); - // @ts-ignore const disabledHoverStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].hover; // Assert diff --git a/packages/components/src/buttons.test.tsx b/packages/components/src/buttons.test.tsx index c8e5c7fd64..1181b954fd 100644 --- a/packages/components/src/buttons.test.tsx +++ b/packages/components/src/buttons.test.tsx @@ -31,11 +31,9 @@ describe.each([{ component: ButtonPrimary }, { component: ButtonSecondary }, { c it('is styled differently', () => { // Act render(); - // @ts-ignore const enabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); - // @ts-ignore const disabledStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert @@ -45,7 +43,6 @@ describe.each([{ component: ButtonPrimary }, { component: ButtonSecondary }, { c it('has no hover style', () => { // Act render(); - // @ts-ignore const disabledHoverStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].hover; // Assert @@ -59,11 +56,9 @@ describe('ButtonPrimary', () => { it('can be styled differently to indicate a dangerous action', () => { // Act render(); - // @ts-ignore const defaultStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; render(); - // @ts-ignore const dangerStyle = (Clickable as jest.MockedFunction).mock.lastCall[0].style; // Assert From fff2d1f6b13f4978bae090727508d10642d3978f Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Wed, 6 Nov 2024 18:22:05 -0500 Subject: [PATCH 05/27] fix tests --- src/libs/ajax/methods/providers/CreateMethodProvider.test.ts | 2 +- src/pages/methods/WorkflowList.test.tsx | 4 ++-- src/pages/methods/workflow-details/WorkflowWrapper.ts | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts b/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts index 5fcae413ec..03d9d80fef 100644 --- a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts +++ b/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts @@ -37,7 +37,7 @@ describe('create method provider', () => { const signal = new window.AbortController().signal; // Act - const result = await postMethodProvider.create( + const result = await postMethodProvider.postMethod( 'input-namespace', 'input-name', 'workflow input {}', diff --git a/src/pages/methods/WorkflowList.test.tsx b/src/pages/methods/WorkflowList.test.tsx index 406353fa8f..1cbc335e2e 100644 --- a/src/pages/methods/WorkflowList.test.tsx +++ b/src/pages/methods/WorkflowList.test.tsx @@ -950,7 +950,7 @@ describe('create workflow modal', () => { // Arrange asMockedFn(Ajax).mockImplementation(() => mockAjax([]) as AjaxContract); - jest.spyOn(postMethodProvider, 'create').mockResolvedValue(mockCreateMethodResponse); + jest.spyOn(postMethodProvider, 'postMethod').mockResolvedValue(mockCreateMethodResponse); const user: UserEvent = userEvent.setup(); @@ -973,7 +973,7 @@ describe('create workflow modal', () => { await user.click(screen.getByRole('button', { name: 'Upload' })); // Assert - expect(postMethodProvider.create).toHaveBeenCalled(); + expect(postMethodProvider.postMethod).toHaveBeenCalled(); expect(Nav.goToPath).toHaveBeenCalledWith('workflow-dashboard', { name: 'response-name', diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index d1acd2269c..4e949dab72 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -20,6 +20,7 @@ import * as Utils from 'src/libs/utils'; import { withBusyState } from 'src/libs/utils'; import DeleteSnapshotModal from 'src/workflows/methods/modals/DeleteSnapshotModal'; import { PermissionsModal } from 'src/workflows/methods/modals/PermissionsModal'; +import { WorkflowModal } from 'src/workflows/methods/modals/WorkflowModal'; import SnapshotActionMenu from 'src/workflows/methods/SnapshotActionMenu'; import ExportWorkflowModal from 'src/workflows/modals/ExportWorkflowModal'; import { isGoogleWorkspace, WorkspaceInfo, WorkspaceWrapper } from 'src/workspaces/utils'; From b0a87bb6e9acf3ef7c528fd6d08a7ea052179e63 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Wed, 6 Nov 2024 18:39:52 -0500 Subject: [PATCH 06/27] cleanup --- ...eMethodProvider.test.ts => PostMethodProvider.test.ts} | 4 ++-- src/workflows/methods/SnapshotActionMenu.test.tsx | 6 ++++-- src/workflows/methods/modals/WorkflowModal.test.tsx | 8 +++++++- 3 files changed, 13 insertions(+), 5 deletions(-) rename src/libs/ajax/methods/providers/{CreateMethodProvider.test.ts => PostMethodProvider.test.ts} (95%) diff --git a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts b/src/libs/ajax/methods/providers/PostMethodProvider.test.ts similarity index 95% rename from src/libs/ajax/methods/providers/CreateMethodProvider.test.ts rename to src/libs/ajax/methods/providers/PostMethodProvider.test.ts index 03d9d80fef..3eae5f66cc 100644 --- a/src/libs/ajax/methods/providers/CreateMethodProvider.test.ts +++ b/src/libs/ajax/methods/providers/PostMethodProvider.test.ts @@ -29,8 +29,8 @@ const mockMethodResponse: MethodResponse = { url: 'http://agora.dsde-dev.broadinstitute.org/api/v1/methods/sschu/response-test/1', }; -describe('create method provider', () => { - it('handles create call', async () => { +describe('post method provider', () => { + it('handles post call', async () => { // Arrange const methodsMock = mockMethodsNeeds(); asMockedFn(methodsMock.postMethod).mockResolvedValue(mockMethodResponse); diff --git a/src/workflows/methods/SnapshotActionMenu.test.tsx b/src/workflows/methods/SnapshotActionMenu.test.tsx index 5e08087c09..8af4517359 100644 --- a/src/workflows/methods/SnapshotActionMenu.test.tsx +++ b/src/workflows/methods/SnapshotActionMenu.test.tsx @@ -30,7 +30,7 @@ describe('snapshot action menu', () => { expect(snapshotActionMenu).toHaveAttribute('aria-disabled'); }); - it('renders and enables the menu buttons if you are the snapshot owner', async () => { + it('renders and enables correct menu buttons if you are the snapshot owner', async () => { // Arrange const user: UserEvent = userEvent.setup(); @@ -79,11 +79,12 @@ describe('snapshot action menu', () => { await user.pointer({ target: cloneSnapshotButton }); // Assert + // clone option is always enabled irrespective of snapshot ownership expect(cloneSnapshotButton).toBeInTheDocument(); expect(cloneSnapshotButton).toHaveAttribute('aria-disabled', 'false'); }); - it('renders the menu buttons based on snapshot ownership', async () => { + it('renders and enables correct menu buttons if you are NOT the snapshot owner', async () => { // Arrange const user: UserEvent = userEvent.setup(); @@ -132,6 +133,7 @@ describe('snapshot action menu', () => { await user.pointer({ target: cloneSnapshotButton }); // Assert + // clone option is always enabled irrespective of snapshot ownership expect(cloneSnapshotButton).toBeInTheDocument(); expect(cloneSnapshotButton).toHaveAttribute('aria-disabled', 'false'); }); diff --git a/src/workflows/methods/modals/WorkflowModal.test.tsx b/src/workflows/methods/modals/WorkflowModal.test.tsx index 56953f1d17..9db32e8076 100644 --- a/src/workflows/methods/modals/WorkflowModal.test.tsx +++ b/src/workflows/methods/modals/WorkflowModal.test.tsx @@ -479,7 +479,7 @@ describe('WorkflowModal', () => { expect(mockOnDismiss).toHaveBeenCalled(); }); - it('clone method modal', async () => { + it('displays clone method modal and calls correct function on submit', async () => { // Arrange const mockOnSuccess = jest.fn(); const mockOnDismiss = jest.fn(); @@ -505,12 +505,18 @@ describe('WorkflowModal', () => { }); // Assert + expect(screen.getByRole('textbox', { name: 'Namespace *' })).toHaveDisplayValue(''); expect(screen.getByRole('textbox', { name: 'Name *' })).toHaveDisplayValue('groot-scientific-workflow_copy'); expect(screen.getByTestId('wdl editor')).toHaveDisplayValue('workflow do-great-stuff {}'); expect(screen.getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('I am Groot'); expect(screen.getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue('I am Groot'); expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue('I am Groot'); + const cloneMethodButton = screen.getByRole('button', { name: 'Clone method' }); + + // Assert + expect(cloneMethodButton).toHaveAttribute('aria-disabled', 'true'); + // user enters value for 'Namespace' text box fireEvent.change(screen.getByRole('textbox', { name: 'Namespace *' }), { target: { value: 'groot-test-namespace' }, From 2f6cc483173db89e6dfca7a7f8bf910d0f1a9fef Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 11:22:12 -0500 Subject: [PATCH 07/27] use snapshot in modal --- src/pages/methods/workflow-details/WorkflowWrapper.ts | 4 ++-- src/workflows/methods/modals/WorkflowModal.test.tsx | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index 4e949dab72..690e4c2ae0 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -295,13 +295,13 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { }), showCloneModal && h(WorkflowModal, { - title: 'Clone method', + title: 'Clone snapshot', defaultName: name.concat('_copy'), defaultWdl: snapshot?.payload, defaultDocumentation: snapshot?.documentation, defaultSynopsis: snapshot?.synopsis, defaultSnapshotComment: snapshot?.snapshotComment, - buttonActionName: 'Clone method', + buttonActionName: 'Clone snapshot', postMethodProvider, onSuccess: (namespace: string, name: string, snapshotId: number) => Nav.goToPath('workflow-dashboard', { diff --git a/src/workflows/methods/modals/WorkflowModal.test.tsx b/src/workflows/methods/modals/WorkflowModal.test.tsx index 9db32e8076..738cc47e6b 100644 --- a/src/workflows/methods/modals/WorkflowModal.test.tsx +++ b/src/workflows/methods/modals/WorkflowModal.test.tsx @@ -490,13 +490,13 @@ describe('WorkflowModal', () => { await act(async () => { render( { expect(screen.getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue('I am Groot'); expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue('I am Groot'); - const cloneMethodButton = screen.getByRole('button', { name: 'Clone method' }); + const cloneMethodButton = screen.getByRole('button', { name: 'Clone snapshot' }); // Assert expect(cloneMethodButton).toHaveAttribute('aria-disabled', 'true'); @@ -523,7 +523,7 @@ describe('WorkflowModal', () => { }); // Act - await user.click(screen.getByRole('button', { name: 'Clone method' })); + await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); // Assert expect(successPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); From bb2920a896b48b8de59e92475fe21799d13869c2 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 15:09:57 -0500 Subject: [PATCH 08/27] additional tests --- .../workflow-details/WorkflowWrapper.test.tsx | 83 ++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index 4a8ac33b60..5a9b90c7db 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -22,6 +22,14 @@ jest.mock('src/libs/notifications'); type NavExports = typeof import('src/libs/nav'); +type WDLEditorExports = typeof import('src/workflows/methods/WDLEditor'); +jest.mock('src/workflows/methods/WDLEditor', (): WDLEditorExports => { + const mockWDLEditorModule = jest.requireActual('src/workflows/methods/WDLEditor.mock'); + return { + WDLEditor: mockWDLEditorModule.MockWDLEditor, + }; +}); + jest.mock( 'src/libs/nav', (): NavExports => ({ @@ -37,9 +45,9 @@ const mockSnapshot: Snapshot = { managers: ['hello@world.org'], name: 'testname', createDate: '2024-09-04T15:37:57Z', - documentation: '', + documentation: 'mock documentation', entityType: 'Workflow', - snapshotComment: '', + snapshotComment: 'mock snapshot comment', snapshotId: 1, namespace: 'testnamespace', payload: @@ -407,6 +415,77 @@ describe('workflows container', () => { expect(goToPath).toHaveBeenCalledWith('workflows'); }); + it('renders the clone snapshot modal when the corresponding button is pressed', async () => { + // Arrange + mockAjax(); + + // set the user's email + jest.spyOn(userStore, 'get').mockImplementation(jest.fn().mockReturnValue(mockUserState('hello@world.org'))); + + const user: UserEvent = userEvent.setup(); + + // Act + await act(async () => { + render( + + ); + }); + + await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); + await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + + const dialog = screen.getByRole('dialog', { name: /clone snapshot/i }); + + screen.logTestingPlaygroundURL(); + + // Assert + expect(dialog).toBeInTheDocument(); + expect(within(dialog).getByRole('textbox', { name: 'Namespace *' })).toHaveDisplayValue(''); + expect(within(dialog).getByRole('textbox', { name: 'Name *' })).toHaveDisplayValue('testname_copy'); + expect(within(dialog).getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('mock documentation'); + expect(within(dialog).getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue(''); + expect(within(dialog).getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue( + 'mock snapshot comment' + ); + expect(within(dialog).getByTestId('wdl editor')).toHaveDisplayValue(mockSnapshot.payload.toString()); + }); + + it('hides the clone snapshot modal when it is dismissed', async () => { + // Arrange + mockAjax(); + + // set the user's email + jest.spyOn(userStore, 'get').mockImplementation(jest.fn().mockReturnValue(mockUserState('hello@world.org'))); + + const user: UserEvent = userEvent.setup(); + + // Act + await act(async () => { + render( + + ); + }); + + await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); + await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + // Assert + const dialog = screen.queryByRole('dialog', { name: /clone snapshot/i }); + + expect(dialog).not.toBeInTheDocument(); + }); + it('hides the delete snapshot modal and displays a loading spinner when the deletion is confirmed', async () => { // Arrange mockAjax({ From 32184284989f03e93d4de4d906bc63cb28f3834a Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 15:13:18 -0500 Subject: [PATCH 09/27] remove playground --- src/pages/methods/workflow-details/WorkflowWrapper.test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index 5a9b90c7db..6c2f5e7247 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -441,8 +441,6 @@ describe('workflows container', () => { const dialog = screen.getByRole('dialog', { name: /clone snapshot/i }); - screen.logTestingPlaygroundURL(); - // Assert expect(dialog).toBeInTheDocument(); expect(within(dialog).getByRole('textbox', { name: 'Namespace *' })).toHaveDisplayValue(''); From 5b6e01a8243c2a1168619e3887afe64cc4a33687 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 16:41:47 -0500 Subject: [PATCH 10/27] reset store --- src/pages/methods/workflow-details/WorkflowWrapper.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index 690e4c2ae0..6f4ef7b333 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -303,12 +303,17 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { defaultSnapshotComment: snapshot?.snapshotComment, buttonActionName: 'Clone snapshot', postMethodProvider, - onSuccess: (namespace: string, name: string, snapshotId: number) => + onSuccess: (namespace: string, name: string, snapshotId: number) => { + // there is an interesting situation where if a user has the same namespace and name for the cloned method + // as the original method, instead of creating a new method Agora will create a new snapshot of the original method. + // Hence, to ensure the data is correct in the UI reset the cached snapshot list store and then load the page. + snapshotsListStore.reset(); Nav.goToPath('workflow-dashboard', { namespace, name, snapshotId, - }), + }); + }, onDismiss: () => setShowCloneModal(false), }), busy && spinnerOverlay, From 6c01892797dd7996eb65b8b1e1f867753f43a379 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 16:45:59 -0500 Subject: [PATCH 11/27] add comment --- src/pages/methods/workflow-details/WorkflowWrapper.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index 6f4ef7b333..5d31f56673 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -304,9 +304,11 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { buttonActionName: 'Clone snapshot', postMethodProvider, onSuccess: (namespace: string, name: string, snapshotId: number) => { - // there is an interesting situation where if a user has the same namespace and name for the cloned method - // as the original method, instead of creating a new method Agora will create a new snapshot of the original method. - // Hence, to ensure the data is correct in the UI reset the cached snapshot list store and then load the page. + // when the user has owner permissions on the original method, there is an interesting situation where + // if the user types in the same namespace and name for the cloned method as the original method, + // instead of creating a new method Agora will create a new snapshot of the original method. + // Hence, to ensure the data is correct in the UI we reset the cached snapshot list store and then load the page. + // (Note: this behaviour is same as in Firecloud UI) snapshotsListStore.reset(); Nav.goToPath('workflow-dashboard', { namespace, From 69ef5da59c0afd05fc3f8813e82cc94e05ef8a05 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Nov 2024 16:29:12 -0500 Subject: [PATCH 12/27] wip --- src/libs/ajax/methods/Methods.ts | 17 ++++++++++++----- src/libs/ajax/methods/methods-models.ts | 8 ++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/libs/ajax/methods/Methods.ts b/src/libs/ajax/methods/Methods.ts index a31e26cc14..2433275332 100644 --- a/src/libs/ajax/methods/Methods.ts +++ b/src/libs/ajax/methods/Methods.ts @@ -4,6 +4,7 @@ import * as qs from 'qs'; import { authOpts } from 'src/auth/auth-session'; import { fetchAgora, fetchOrchestration, fetchRawls } from 'src/libs/ajax/ajax-common'; import { + CreateSnapshotRequest, MethodConfigACL, MethodDefinition, MethodQuery, @@ -60,6 +61,17 @@ export const Methods = (signal?: AbortSignal) => ({ return res.json(); }, + createSnapshot: async ( + payload: CreateSnapshotRequest, + redactPreviousSnapshot: boolean + ): Promise => { + const res = await fetchOrchestration( + `api/${root}?redact=${redactPreviousSnapshot}`, + _.mergeAll([authOpts(), jsonBody(payload), { signal, method: 'POST' }]) + ); + return res.json(); + }, + configs: async () => { const res = await fetchAgora(`${root}/configurations`, _.merge(authOpts(), { signal })); return res.json(); @@ -78,11 +90,6 @@ export const Methods = (signal?: AbortSignal) => ({ return res.json(); }, - allConfigs: async () => { - const res = await fetchAgora(`methods/${namespace}/${name}/configurations`, _.merge(authOpts(), { signal })); - return res.json(); - }, - toWorkspace: async (workspace, config: any = {}) => { const res = await fetchRawls( `workspaces/${workspace.namespace}/${workspace.name}/methodconfigs`, diff --git a/src/libs/ajax/methods/methods-models.ts b/src/libs/ajax/methods/methods-models.ts index ed0ef38f15..f0b85f221a 100644 --- a/src/libs/ajax/methods/methods-models.ts +++ b/src/libs/ajax/methods/methods-models.ts @@ -42,6 +42,14 @@ export interface MethodQuery { entityType: string; } +/** Type for Orchestration's create method snapshot schema */ +export interface CreateSnapshotRequest { + synopsis?: string; + snapshotComment?: string; + documentation?: string; + payload: string; +} + /** * Type for Orchestration's MethodResponse schema. * From 7373ed9885b53eae7a9e12f87d0a594ad5a716d1 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 14 Nov 2024 14:37:55 -0800 Subject: [PATCH 13/27] PR feedback --- .../methods/workflow-details/WorkflowWrapper.test.tsx | 4 +--- src/pages/methods/workflow-details/WorkflowWrapper.ts | 7 +++---- src/workflows/methods/SnapshotActionMenu.test.tsx | 1 - src/workflows/methods/modals/WorkflowModal.test.tsx | 9 ++++++--- src/workflows/methods/modals/WorkflowModal.tsx | 7 ++++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index 6c2f5e7247..6452f23c27 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -447,9 +447,7 @@ describe('workflows container', () => { expect(within(dialog).getByRole('textbox', { name: 'Name *' })).toHaveDisplayValue('testname_copy'); expect(within(dialog).getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('mock documentation'); expect(within(dialog).getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue(''); - expect(within(dialog).getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue( - 'mock snapshot comment' - ); + expect(within(dialog).getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue(''); expect(within(dialog).getByTestId('wdl editor')).toHaveDisplayValue(mockSnapshot.payload.toString()); }); diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index 5d31f56673..3e6a45df40 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -297,10 +297,9 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { h(WorkflowModal, { title: 'Clone snapshot', defaultName: name.concat('_copy'), - defaultWdl: snapshot?.payload, - defaultDocumentation: snapshot?.documentation, - defaultSynopsis: snapshot?.synopsis, - defaultSnapshotComment: snapshot?.snapshotComment, + defaultWdl: snapshot!.payload, + defaultDocumentation: snapshot!.documentation, + defaultSynopsis: snapshot!.synopsis, buttonActionName: 'Clone snapshot', postMethodProvider, onSuccess: (namespace: string, name: string, snapshotId: number) => { diff --git a/src/workflows/methods/SnapshotActionMenu.test.tsx b/src/workflows/methods/SnapshotActionMenu.test.tsx index 8af4517359..2eb2489960 100644 --- a/src/workflows/methods/SnapshotActionMenu.test.tsx +++ b/src/workflows/methods/SnapshotActionMenu.test.tsx @@ -76,7 +76,6 @@ describe('snapshot action menu', () => { // Act const cloneSnapshotButton = screen.getByRole('button', { name: 'Clone snapshot' }); - await user.pointer({ target: cloneSnapshotButton }); // Assert // clone option is always enabled irrespective of snapshot ownership diff --git a/src/workflows/methods/modals/WorkflowModal.test.tsx b/src/workflows/methods/modals/WorkflowModal.test.tsx index 738cc47e6b..a3839f60ea 100644 --- a/src/workflows/methods/modals/WorkflowModal.test.tsx +++ b/src/workflows/methods/modals/WorkflowModal.test.tsx @@ -495,7 +495,6 @@ describe('WorkflowModal', () => { defaultWdl='workflow do-great-stuff {}' defaultDocumentation='I am Groot' defaultSynopsis='I am Groot' - defaultSnapshotComment='I am Groot' buttonActionName='Clone snapshot' postMethodProvider={successPostMethodProvider} onSuccess={mockOnSuccess} @@ -510,7 +509,7 @@ describe('WorkflowModal', () => { expect(screen.getByTestId('wdl editor')).toHaveDisplayValue('workflow do-great-stuff {}'); expect(screen.getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('I am Groot'); expect(screen.getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue('I am Groot'); - expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue('I am Groot'); + expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue(''); const cloneMethodButton = screen.getByRole('button', { name: 'Clone snapshot' }); @@ -521,6 +520,10 @@ describe('WorkflowModal', () => { fireEvent.change(screen.getByRole('textbox', { name: 'Namespace *' }), { target: { value: 'groot-test-namespace' }, }); + // user enters value for 'Snapshot comment' text box + fireEvent.change(screen.getByRole('textbox', { name: 'Snapshot comment' }), { + target: { value: "Groot's brand new snapshot" }, + }); // Act await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); @@ -533,7 +536,7 @@ describe('WorkflowModal', () => { 'workflow do-great-stuff {}', 'I am Groot', 'I am Groot', - 'I am Groot' + "Groot's brand new snapshot" ); expect(mockOnSuccess).toHaveBeenCalledWith('response-namespace', 'response-name', 1); expect(mockOnDismiss).not.toHaveBeenCalled(); diff --git a/src/workflows/methods/modals/WorkflowModal.tsx b/src/workflows/methods/modals/WorkflowModal.tsx index 6b70893e53..ce6c9cd60b 100644 --- a/src/workflows/methods/modals/WorkflowModal.tsx +++ b/src/workflows/methods/modals/WorkflowModal.tsx @@ -57,9 +57,10 @@ export interface WorkflowModalProps { defaultSnapshotComment?: string; /** - * Provides a function to make an API call to perform the create method - * operation. The create function provided is called with the information - * inputted into the modal. + * Provides a function to make an API call to perform the post method + * operation. The postMethod function provided is called with the information + * inputted into the modal. This provider is used for both "create new method" + * and "clone method snapshot" functionality. */ postMethodProvider: PostMethodProvider; From 98f4bb819507975bf99cd66ae2f47af048eb2930 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Fri, 15 Nov 2024 11:40:54 -0800 Subject: [PATCH 14/27] change naming --- .../workflow-details/WorkflowWrapper.test.tsx | 12 +++++------ .../workflow-details/WorkflowWrapper.ts | 4 ++-- .../methods/SnapshotActionMenu.test.tsx | 10 +++++----- src/workflows/methods/SnapshotActionMenu.tsx | 20 +++++++++---------- .../methods/modals/WorkflowModal.test.tsx | 10 +++++----- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index 6452f23c27..4caf83ec14 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -415,7 +415,7 @@ describe('workflows container', () => { expect(goToPath).toHaveBeenCalledWith('workflows'); }); - it('renders the clone snapshot modal when the corresponding button is pressed', async () => { + it('renders the save as new method modal when the corresponding button is pressed', async () => { // Arrange mockAjax(); @@ -437,9 +437,9 @@ describe('workflows container', () => { }); await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); - await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + await user.click(screen.getByRole('button', { name: 'Save as' })); - const dialog = screen.getByRole('dialog', { name: /clone snapshot/i }); + const dialog = screen.getByRole('dialog', { name: /create new method/i }); // Assert expect(dialog).toBeInTheDocument(); @@ -451,7 +451,7 @@ describe('workflows container', () => { expect(within(dialog).getByTestId('wdl editor')).toHaveDisplayValue(mockSnapshot.payload.toString()); }); - it('hides the clone snapshot modal when it is dismissed', async () => { + it('hides the save as new method modal when it is dismissed', async () => { // Arrange mockAjax(); @@ -473,11 +473,11 @@ describe('workflows container', () => { }); await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); - await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + await user.click(screen.getByRole('button', { name: 'Save as' })); await user.click(screen.getByRole('button', { name: 'Cancel' })); // Assert - const dialog = screen.queryByRole('dialog', { name: /clone snapshot/i }); + const dialog = screen.queryByRole('dialog', { name: /save as/i }); expect(dialog).not.toBeInTheDocument(); }); diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index 3e6a45df40..fde26f4541 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -295,12 +295,12 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { }), showCloneModal && h(WorkflowModal, { - title: 'Clone snapshot', + title: 'Create new method', defaultName: name.concat('_copy'), defaultWdl: snapshot!.payload, defaultDocumentation: snapshot!.documentation, defaultSynopsis: snapshot!.synopsis, - buttonActionName: 'Clone snapshot', + buttonActionName: 'Create new method', postMethodProvider, onSuccess: (namespace: string, name: string, snapshotId: number) => { // when the user has owner permissions on the original method, there is an interesting situation where diff --git a/src/workflows/methods/SnapshotActionMenu.test.tsx b/src/workflows/methods/SnapshotActionMenu.test.tsx index 2eb2489960..079537f3f8 100644 --- a/src/workflows/methods/SnapshotActionMenu.test.tsx +++ b/src/workflows/methods/SnapshotActionMenu.test.tsx @@ -75,7 +75,7 @@ describe('snapshot action menu', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); // Act - const cloneSnapshotButton = screen.getByRole('button', { name: 'Clone snapshot' }); + const cloneSnapshotButton = screen.getByRole('button', { name: 'Save as' }); // Assert // clone option is always enabled irrespective of snapshot ownership @@ -128,7 +128,7 @@ describe('snapshot action menu', () => { expect(screen.getByRole('tooltip')).toBeInTheDocument(); // Act - const cloneSnapshotButton = screen.getByRole('button', { name: 'Clone snapshot' }); + const cloneSnapshotButton = screen.getByRole('button', { name: 'Save as' }); await user.pointer({ target: cloneSnapshotButton }); // Assert @@ -191,7 +191,7 @@ describe('snapshot action menu delete snapshot button', () => { }); }); -describe('snapshot action menu clone snapshot button', () => { +describe('snapshot action menu save as button', () => { it('closes and calls the onClone callback when clicked', async () => { // Arrange const user: UserEvent = userEvent.setup(); @@ -209,10 +209,10 @@ describe('snapshot action menu clone snapshot button', () => { }); await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); - await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + await user.click(screen.getByRole('button', { name: 'Save as' })); // Assert - expect(screen.queryByRole('button', { name: 'Clone snapshot' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Save as' })).not.toBeInTheDocument(); expect(mockOnClone).toHaveBeenCalled(); }); }); diff --git a/src/workflows/methods/SnapshotActionMenu.tsx b/src/workflows/methods/SnapshotActionMenu.tsx index c6b0da6d6e..16b8f809e4 100644 --- a/src/workflows/methods/SnapshotActionMenu.tsx +++ b/src/workflows/methods/SnapshotActionMenu.tsx @@ -42,18 +42,9 @@ const SnapshotActionMenu = (props: SnapshotActionMenuProps): ReactNode => { const menuContent = ( <> - - {makeMenuIcon('cog')} - Edit snapshot permissions - {makeMenuIcon('copy')} - Clone snapshot + Save as { {makeMenuIcon('trash')} Delete snapshot + + {makeMenuIcon('cog')} + Edit snapshot permissions + ); diff --git a/src/workflows/methods/modals/WorkflowModal.test.tsx b/src/workflows/methods/modals/WorkflowModal.test.tsx index a3839f60ea..d691deacb0 100644 --- a/src/workflows/methods/modals/WorkflowModal.test.tsx +++ b/src/workflows/methods/modals/WorkflowModal.test.tsx @@ -479,7 +479,7 @@ describe('WorkflowModal', () => { expect(mockOnDismiss).toHaveBeenCalled(); }); - it('displays clone method modal and calls correct function on submit', async () => { + it('displays save as new method modal and calls correct function on submit', async () => { // Arrange const mockOnSuccess = jest.fn(); const mockOnDismiss = jest.fn(); @@ -490,12 +490,12 @@ describe('WorkflowModal', () => { await act(async () => { render( { expect(screen.getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue('I am Groot'); expect(screen.getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue(''); - const cloneMethodButton = screen.getByRole('button', { name: 'Clone snapshot' }); + const cloneMethodButton = screen.getByRole('button', { name: 'Create new method' }); // Assert expect(cloneMethodButton).toHaveAttribute('aria-disabled', 'true'); @@ -526,7 +526,7 @@ describe('WorkflowModal', () => { }); // Act - await user.click(screen.getByRole('button', { name: 'Clone snapshot' })); + await user.click(screen.getByRole('button', { name: 'Create new method' })); // Assert expect(successPostMethodProvider.postMethod).toHaveBeenCalledTimes(1); From 47afaffb2aabfc168b940448ca95501b1e8d0e0a Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Fri, 15 Nov 2024 11:48:12 -0800 Subject: [PATCH 15/27] test change --- .../methods/workflow-details/WorkflowWrapper.test.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index 4caf83ec14..a1420e3ab4 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -474,12 +474,17 @@ describe('workflows container', () => { await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); await user.click(screen.getByRole('button', { name: 'Save as' })); - await user.click(screen.getByRole('button', { name: 'Cancel' })); // Assert - const dialog = screen.queryByRole('dialog', { name: /save as/i }); + const dialog1 = screen.queryByRole('dialog', { name: /create new method/i }); + expect(dialog1).toBeInTheDocument(); - expect(dialog).not.toBeInTheDocument(); + // Act + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + // Assert + const dialog2 = screen.queryByRole('dialog', { name: /create new method/i }); + expect(dialog2).not.toBeInTheDocument(); }); it('hides the delete snapshot modal and displays a loading spinner when the deletion is confirmed', async () => { From 52d1106b97d1b4c488be8747f423da7591c62491 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Fri, 15 Nov 2024 15:44:27 -0800 Subject: [PATCH 16/27] add test from PR feedback --- .../workflow-details/WorkflowWrapper.test.tsx | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx index a1420e3ab4..79853ac6b7 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx +++ b/src/pages/methods/workflow-details/WorkflowWrapper.test.tsx @@ -6,10 +6,12 @@ import React from 'react'; import * as breadcrumbs from 'src/components/breadcrumbs'; import { Ajax, AjaxContract } from 'src/libs/ajax'; import { MethodsAjaxContract } from 'src/libs/ajax/methods/Methods'; -import { Snapshot } from 'src/libs/ajax/methods/methods-models'; +import { MethodResponse, Snapshot } from 'src/libs/ajax/methods/methods-models'; +import { postMethodProvider } from 'src/libs/ajax/methods/providers/PostMethodProvider'; import * as ExportWorkflowToWorkspaceProvider from 'src/libs/ajax/workspaces/providers/ExportWorkflowToWorkspaceProvider'; import { errorWatcher } from 'src/libs/error.mock'; import { goToPath } from 'src/libs/nav'; +import * as Nav from 'src/libs/nav'; import { forwardRefWithName } from 'src/libs/react-utils'; import { snapshotsListStore, snapshotStore, TerraUser, TerraUserState, userStore } from 'src/libs/state'; import { WorkflowsContainer, wrapWorkflows } from 'src/pages/methods/workflow-details/WorkflowWrapper'; @@ -259,6 +261,21 @@ const snapshotStoreInitialValue = { url: '', }; +const mockCloneSnapshotResponse: MethodResponse = { + name: 'testname_copy', + createDate: '2024-11-15T15:41:38Z', + documentation: 'mock documentation', + synopsis: '', + entityType: 'Workflow', + snapshotComment: 'groot-new-snapshot', + snapshotId: 1, + namespace: 'groot-new-namespace', + payload: + // eslint-disable-next-line no-template-curly-in-string + 'task echo_files {\\n String? input1\\n String? input2\\n String? input3\\n \\n output {\\n String out = read_string(stdout())\\n }\\n\\n command {\\n echo \\"result: ${input1} ${input2} ${input3}\\"\\n }\\n\\n runtime {\\n docker: \\"ubuntu:latest\\"\\n }\\n}\\n\\nworkflow echo_strings {\\n call echo_files\\n}', + url: 'http://agora.dsde-dev.broadinstitute.org/api/v1/methods/groot-new-namespace/testname_copy/1', +}; + describe('workflow wrapper', () => { it('displays the method not found page if a method does not exist or the user does not have access', async () => { // Arrange @@ -451,6 +468,68 @@ describe('workflows container', () => { expect(within(dialog).getByTestId('wdl editor')).toHaveDisplayValue(mockSnapshot.payload.toString()); }); + it('calls right provider with expected arguments when snapshot is cloned', async () => { + // Arrange + mockAjax(); + + jest.spyOn(postMethodProvider, 'postMethod').mockResolvedValue(mockCloneSnapshotResponse); + + const user: UserEvent = userEvent.setup(); + + // Act + await act(async () => { + render( + + ); + }); + + await user.click(screen.getByRole('button', { name: 'Snapshot action menu' })); + await user.click(screen.getByRole('button', { name: 'Save as' })); + + const dialog = screen.getByRole('dialog', { name: /create new method/i }); + + // Assert + expect(dialog).toBeInTheDocument(); + expect(within(dialog).getByRole('textbox', { name: 'Namespace *' })).toHaveDisplayValue(''); + expect(within(dialog).getByRole('textbox', { name: 'Name *' })).toHaveDisplayValue('testname_copy'); + expect(within(dialog).getByRole('textbox', { name: 'Documentation' })).toHaveDisplayValue('mock documentation'); + expect(within(dialog).getByRole('textbox', { name: 'Synopsis (80 characters max)' })).toHaveDisplayValue(''); + expect(within(dialog).getByRole('textbox', { name: 'Snapshot comment' })).toHaveDisplayValue(''); + expect(within(dialog).getByTestId('wdl editor')).toHaveDisplayValue(mockSnapshot.payload.toString()); + + // Act + fireEvent.change(screen.getByRole('textbox', { name: 'Namespace *' }), { + target: { value: 'groot-new-namespace' }, + }); + fireEvent.change(screen.getByRole('textbox', { name: 'Snapshot comment' }), { + target: { value: 'groot-new-snapshot' }, + }); + + await user.click(screen.getByRole('button', { name: 'Create new method' })); + + // Assert + expect(postMethodProvider.postMethod).toHaveBeenCalled(); + expect(postMethodProvider.postMethod).toHaveBeenCalledWith( + 'groot-new-namespace', + 'testname_copy', + mockSnapshot.payload, + 'mock documentation', + '', + 'groot-new-snapshot' + ); + + expect(Nav.goToPath).toHaveBeenCalledWith('workflow-dashboard', { + name: 'testname_copy', + namespace: 'groot-new-namespace', + snapshotId: 1, + }); + }); + it('hides the save as new method modal when it is dismissed', async () => { // Arrange mockAjax(); From 6ab8ab7082374a6069b2d782de751a6d0a645b5b Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Mon, 18 Nov 2024 15:38:09 -0500 Subject: [PATCH 17/27] wip; before refactor of WorkflowModal --- .../methods/providers/EditMethodProvider.ts | 42 +++++++++++++++++++ .../workflow-details/WorkflowWrapper.ts | 22 ++++++++++ src/workflows/methods/SnapshotActionMenu.tsx | 16 ++++++- 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/libs/ajax/methods/providers/EditMethodProvider.ts diff --git a/src/libs/ajax/methods/providers/EditMethodProvider.ts b/src/libs/ajax/methods/providers/EditMethodProvider.ts new file mode 100644 index 0000000000..d41858ad38 --- /dev/null +++ b/src/libs/ajax/methods/providers/EditMethodProvider.ts @@ -0,0 +1,42 @@ +import { AbortOption } from '@terra-ui-packages/data-client-core'; +import { Methods } from 'src/libs/ajax/methods/Methods'; +import { MethodResponse } from 'src/libs/ajax/methods/methods-models'; + +export interface EditMethodProvider { + createNewSnapshot: ( + namespace: string, + name: string, + snapshotId: number, + redactPreviousSnapshot: boolean, + synopsis: string, + documentation: string, + wdl: string, + snapshotComment: string, + options?: AbortOption + ) => Promise; +} + +export const editMethodProvider: EditMethodProvider = { + createNewSnapshot: async ( + namespace: string, + name: string, + snapshotId: number, + redactPreviousSnapshot: boolean, + synopsis: string, + documentation: string, + wdl: string, + snapshotComment: string, + options: AbortOption = {} + ): Promise => { + const { signal } = options; + + const payload = { + synopsis, + snapshotComment, + documentation, + payload: wdl, + }; + + return await Methods(signal).method(namespace, name, snapshotId).createSnapshot(payload, redactPreviousSnapshot); + }, +}; diff --git a/src/pages/methods/workflow-details/WorkflowWrapper.ts b/src/pages/methods/workflow-details/WorkflowWrapper.ts index fde26f4541..e9991e22c9 100644 --- a/src/pages/methods/workflow-details/WorkflowWrapper.ts +++ b/src/pages/methods/workflow-details/WorkflowWrapper.ts @@ -125,6 +125,7 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { const [exportingWorkflow, setExportingWorkflow] = useState(false); const [showDeleteModal, setShowDeleteModal] = useState(false); const [showCloneModal, setShowCloneModal] = useState(false); + const [showEditMethodModal, setShowEditMethodModal] = useState(false); const [busy, setBusy] = useState(false); const [permissionsModalOpen, setPermissionsModalOpen] = useState(false); @@ -243,6 +244,7 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { onEditPermissions: () => setPermissionsModalOpen(true), onDelete: () => setShowDeleteModal(true), onClone: () => setShowCloneModal(true), + onEdit: () => setShowEditMethodModal(true), }), ]), ] @@ -317,6 +319,26 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { }, onDismiss: () => setShowCloneModal(false), }), + showEditMethodModal && + h(WorkflowModal, { + title: 'Edit method', + defaultNamespace: namespace, + defaultName: name, + defaultWdl: snapshot!.payload, + defaultDocumentation: snapshot!.documentation, + defaultSynopsis: snapshot!.synopsis, + buttonActionName: 'Create new snapshot', + postMethodProvider, // editMethodProvider ???? + onSuccess: (namespace: string, name: string, snapshotId: number) => { + snapshotsListStore.reset(); + Nav.goToPath('workflow-dashboard', { + namespace, + name, + snapshotId, + }); + }, + onDismiss: () => setShowEditMethodModal(false), + }), busy && spinnerOverlay, snapshotNotFound && h(NotFoundMessage, { subject: 'snapshot' }), snapshot && div({ style: { flex: 1, display: 'flex', flexDirection: 'column' } }, [children]), diff --git a/src/workflows/methods/SnapshotActionMenu.tsx b/src/workflows/methods/SnapshotActionMenu.tsx index 16b8f809e4..b6c7543816 100644 --- a/src/workflows/methods/SnapshotActionMenu.tsx +++ b/src/workflows/methods/SnapshotActionMenu.tsx @@ -25,8 +25,11 @@ export interface SnapshotActionMenuProps { /** The action to be performed if the "Delete snapshot" button is pressed. */ onDelete: () => void; - /** The action to be performed if the "Clone snapshot" button is pressed. */ + /** The action to be performed if the "Save as" button is pressed. */ onClone: () => void; + + /** The action to be performed if the "Edit" button is pressed. */ + onEdit: () => void; } /** @@ -36,7 +39,7 @@ export interface SnapshotActionMenuProps { * Currently supported actions: edit permissions, delete snapshot, clone snapshot */ const SnapshotActionMenu = (props: SnapshotActionMenuProps): ReactNode => { - const { disabled, isSnapshotOwner, onEditPermissions, onDelete, onClone } = props; + const { disabled, isSnapshotOwner, onEditPermissions, onDelete, onClone, onEdit } = props; const notSnapshotOwnerTooltip = 'You must be an owner of this snapshot'; @@ -46,6 +49,15 @@ const SnapshotActionMenu = (props: SnapshotActionMenuProps): ReactNode => { {makeMenuIcon('copy')} Save as + + {makeMenuIcon('edit')} + Edit + Date: Tue, 19 Nov 2024 17:21:09 -0500 Subject: [PATCH 18/27] refactored WorkflowModal --- src/pages/methods/WorkflowList.tsx | 4 +- .../workflow-details/WorkflowWrapper.ts | 17 +- .../methods/modals/BaseWorkflowModal.tsx | 173 +++++++++++++++ ...st.tsx => CreateNewWorkflowModal.test.tsx} | 28 +-- ...owModal.tsx => CreateNewWorkflowModal.tsx} | 201 ++---------------- .../methods/modals/EditWorkflowModal.tsx | 160 ++++++++++++++ 6 files changed, 381 insertions(+), 202 deletions(-) create mode 100644 src/workflows/methods/modals/BaseWorkflowModal.tsx rename src/workflows/methods/modals/{WorkflowModal.test.tsx => CreateNewWorkflowModal.test.tsx} (97%) rename src/workflows/methods/modals/{WorkflowModal.tsx => CreateNewWorkflowModal.tsx} (56%) create mode 100644 src/workflows/methods/modals/EditWorkflowModal.tsx diff --git a/src/pages/methods/WorkflowList.tsx b/src/pages/methods/WorkflowList.tsx index 0b14155651..a99b610631 100644 --- a/src/pages/methods/WorkflowList.tsx +++ b/src/pages/methods/WorkflowList.tsx @@ -18,7 +18,7 @@ import { useCancellation, useOnMount } from 'src/libs/react-utils'; import { getTerraUser } from 'src/libs/state'; import * as Utils from 'src/libs/utils'; import { withBusyState } from 'src/libs/utils'; -import { WorkflowModal } from 'src/workflows/methods/modals/WorkflowModal'; +import { CreateNewWorkflowModal } from 'src/workflows/methods/modals/CreateNewWorkflowModal'; // Note: The first tab key in this array will determine the default tab selected // if the tab query parameter is not present or has an invalid value (and when @@ -288,7 +288,7 @@ export const WorkflowList = (props: WorkflowListProps) => { )} {createWorkflowModalOpen && ( - { refresh: loadSnapshot, }), showCloneModal && - h(WorkflowModal, { + h(CreateNewWorkflowModal, { title: 'Create new method', defaultName: name.concat('_copy'), defaultWdl: snapshot!.payload, @@ -320,15 +322,16 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { onDismiss: () => setShowCloneModal(false), }), showEditMethodModal && - h(WorkflowModal, { - title: 'Edit method', - defaultNamespace: namespace, - defaultName: name, + h(EditWorkflowModal, { + title: 'Edit', + namespace, + name, + snapshotId: snapshot!.snapshotId, defaultWdl: snapshot!.payload, defaultDocumentation: snapshot!.documentation, defaultSynopsis: snapshot!.synopsis, buttonActionName: 'Create new snapshot', - postMethodProvider, // editMethodProvider ???? + editMethodProvider, onSuccess: (namespace: string, name: string, snapshotId: number) => { snapshotsListStore.reset(); Nav.goToPath('workflow-dashboard', { diff --git a/src/workflows/methods/modals/BaseWorkflowModal.tsx b/src/workflows/methods/modals/BaseWorkflowModal.tsx new file mode 100644 index 0000000000..ac20248636 --- /dev/null +++ b/src/workflows/methods/modals/BaseWorkflowModal.tsx @@ -0,0 +1,173 @@ +import { Clickable, useUniqueId } from '@terra-ui-packages/components'; +import { readFileAsText } from '@terra-ui-packages/core-utils'; +import React, { useState } from 'react'; +import Dropzone from 'src/components/Dropzone'; +import { TextArea, TextInput, ValidatedInput } from 'src/components/input'; +import colors from 'src/libs/colors'; +import { FormLabel } from 'src/libs/forms'; +import * as Utils from 'src/libs/utils'; +import { WDLEditor } from 'src/workflows/methods/WDLEditor'; + +export interface BaseWorkflowModalProps { + /** The title to be shown at the top of the modal. */ + title: string; + + /** The text to be shown on the primary button of the modal. */ + buttonActionName: string; + + /** + * The default value to be prefilled in the WDL input. If not present, the + * input will initially be blank. + */ + defaultWdl?: string; + + /** + * The default value to be prefilled in the documentation input. If not + * present, the input will initially be blank. + */ + defaultDocumentation?: string; + + /** + * The default value to be prefilled in the synopsis input. If not present, + * the input will initially be blank. + */ + defaultSynopsis?: string; + + /** + * The default value to be prefilled in the snapshot comment input. If not + * present, the input will initially be blank. + */ + defaultSnapshotComment?: string; + + /** + * The function to be called with the namespace, name, and snapshot ID of the + * created method snapshot after the user presses the primary modal button and + * the triggered operation successfully completes. + */ + onSuccess: (namespace: string, name: string, snapshotId: number) => void; + + /** + * Called when the underlying modal is dismissed (e.g., when the Cancel button + * is pressed or the user clicks outside the modal). + */ + onDismiss: () => void; +} + +interface SynopsisSnapshotSectionProps { + synopsis: string; + snapshotComment: string; + setWorkflowSynopsis: (value: string) => void; + setSnapshotComment: (value: string) => void; + errors: any; +} + +interface WdlBoxSectionProps { + wdlPayload: string; + setWdlPayload: (value: string) => void; +} + +interface DocumentationSectionProps { + documentation: string; + setWorkflowDocumentation: (value: string) => void; +} + +export const baseWorkflowModalConstraints = { + synopsis: { + length: { maximum: 80 }, + }, + wdl: { + presence: { allowEmpty: false }, + }, +}; + +const uploadWdl = async (wdlFile, setWdlPayload) => { + const rawWdl = await readFileAsText(wdlFile); + setWdlPayload(rawWdl); +}; + +export const SynopsisSnapshotSection = (props: SynopsisSnapshotSectionProps) => { + const { synopsis, snapshotComment, setWorkflowSynopsis, setSnapshotComment, errors } = props; + const [synopsisModified, setSynopsisModified] = useState(false); + + const synopsisInputId = useUniqueId(); + const snapshotCommentInputId = useUniqueId(); + + return ( +
+
+
+ Synopsis (80 characters max) + { + setWorkflowSynopsis(v); + setSynopsisModified(true); + }, + }} + error={Utils.summarizeErrors((synopsisModified || synopsis) && errors?.synopsis)} + /> +
+
+
+
+ Snapshot comment +
+ +
+
+ ); +}; + +export const WdlBoxSection = (props: WdlBoxSectionProps) => { + const { wdlPayload, setWdlPayload } = props; + + const wdlLabelId = useUniqueId(); + + return ( + <> +
+ + WDL + + uploadWdl(wdlFile[0], setWdlPayload)} + > + {({ openUploader }) => ( + openUploader()} + > + Load from file + + )} + +
+
+ +
+ + ); +}; + +export const DocumentationSection = (props: DocumentationSectionProps) => { + const { documentation, setWorkflowDocumentation } = props; + + const documentationInputId = useUniqueId(); + + return ( +
+ Documentation +