diff --git a/src/components/breadcrumbs.js b/src/components/breadcrumbs.js index 83dcac5654..23853849d4 100644 --- a/src/components/breadcrumbs.js +++ b/src/components/breadcrumbs.js @@ -17,7 +17,7 @@ export const breadcrumbElement = (child, href) => { export const commonPaths = { datasetList: () => [breadcrumbElement('Datasets', Nav.getLink('library-datasets'))], - workflowList: () => [breadcrumbElement('Workflows', Nav.getLink('workflows'))], + workflowList: () => [breadcrumbElement('Methods', Nav.getLink('workflows'))], workspaceList: () => [breadcrumbElement('Workspaces', Nav.getLink('workspaces'))], diff --git a/src/pages/workflows/WorkflowList.test.tsx b/src/pages/workflows/WorkflowList.test.tsx index 6f9f60f4b4..72147f9c5b 100644 --- a/src/pages/workflows/WorkflowList.test.tsx +++ b/src/pages/workflows/WorkflowList.test.tsx @@ -19,7 +19,7 @@ jest.mock('src/libs/ajax'); jest.mock('src/libs/notifications'); jest.mock('src/libs/nav', () => ({ ...jest.requireActual('src/libs/nav'), - getLink: jest.fn(() => '#workflows'), + getLink: jest.fn(() => '#methods'), goToPath: jest.fn(), })); @@ -211,11 +211,11 @@ describe('workflows table', () => { }); // Assert - expect(screen.getByPlaceholderText('SEARCH WORKFLOWS')).toBeInTheDocument(); - expect(screen.getByText('My Workflows (0)')).toBeInTheDocument(); - expect(screen.getByText('Public Workflows (0)')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('SEARCH METHODS')).toBeInTheDocument(); + expect(screen.getByText('My Methods (0)')).toBeInTheDocument(); + expect(screen.getByText('Public Methods (0)')).toBeInTheDocument(); - expect(screen.queryByText('Featured Workflows')).not.toBeInTheDocument(); + expect(screen.queryByText('Featured Methods')).not.toBeInTheDocument(); }); it('renders the workflows table with method information', async () => { @@ -235,7 +235,7 @@ describe('workflows table', () => { const headers: HTMLElement[] = within(table).getAllByRole('columnheader'); expect(headers).toHaveLength(4); - expect(headers[0]).toHaveTextContent('Workflow'); + expect(headers[0]).toHaveTextContent('Method'); expect(headers[1]).toHaveTextContent('Synopsis'); expect(headers[2]).toHaveTextContent('Owners'); expect(headers[3]).toHaveTextContent('Snapshots'); @@ -269,7 +269,7 @@ describe('workflows table', () => { expect(screen.queryByText('daruk method')).not.toBeInTheDocument(); }); - it('displays only my workflows in the my workflows tab', async () => { + it('displays only my workflows in the my methods tab', async () => { // Arrange asMockedFn(Ajax).mockImplementation( () => mockAjax([darukMethod, revaliMethod, revaliPrivateMethod]) as AjaxContract @@ -307,7 +307,7 @@ describe('workflows table', () => { expect(screen.queryByText('ganon private method')).not.toBeInTheDocument(); }); - it('displays only public workflows in the public workflows tab', async () => { + it('displays only public workflows in the public methods tab', async () => { // Arrange asMockedFn(Ajax).mockImplementation( () => mockAjax([darukMethod, revaliMethod, ganonPrivateMethod]) as AjaxContract @@ -343,8 +343,8 @@ describe('workflows table', () => { }); // Assert - expect(screen.getByText(`My Workflows (${myMethodsCount})`)).toBeInTheDocument(); - expect(screen.getByText(`Public Workflows (${publicMethodsCount})`)).toBeInTheDocument(); + expect(screen.getByText(`My Methods (${myMethodsCount})`)).toBeInTheDocument(); + expect(screen.getByText(`Public Methods (${publicMethodsCount})`)).toBeInTheDocument(); } ); @@ -365,10 +365,10 @@ describe('workflows table', () => { // Assert // currently selected tab - count based on filter - expect(screen.getByText('My Workflows (1)')).toBeInTheDocument(); + expect(screen.getByText('My Methods (1)')).toBeInTheDocument(); // other tab - count not based on filter - expect(screen.getByText('Public Workflows (3)')).toBeInTheDocument(); + expect(screen.getByText('Public Methods (3)')).toBeInTheDocument(); }); it('filters workflows by namespace', async () => { @@ -422,7 +422,7 @@ describe('workflows table', () => { // Arrange asMockedFn(Ajax).mockImplementation(() => mockAjax([darukMethod, revaliMethod]) as AjaxContract); - // set the user's email to ensure there are no my workflows + // set the user's email to ensure there are no my methods jest.spyOn(userStore, 'get').mockImplementation(jest.fn().mockReturnValue(mockUserState('test@test.com'))); // should be called to switch tabs @@ -435,8 +435,8 @@ describe('workflows table', () => { render(); }); - await user.click(screen.getByText('Public Workflows (2)')); - await user.click(screen.getByText('My Workflows (0)')); + await user.click(screen.getByText('Public Methods (2)')); + await user.click(screen.getByText('My Methods (0)')); // Assert expect(navHistoryReplace).toHaveBeenCalledTimes(2); @@ -457,7 +457,7 @@ describe('workflows table', () => { render(); }); - fireEvent.change(screen.getByPlaceholderText('SEARCH WORKFLOWS'), { target: { value: 'mysearch' } }); + fireEvent.change(screen.getByPlaceholderText('SEARCH METHODS'), { target: { value: 'mysearch' } }); await act(() => delay(300)); // debounced search // Assert @@ -478,7 +478,7 @@ describe('workflows table', () => { expect(screen.getByRole('searchbox')).toHaveProperty('value', 'testfilter'); }); - it('sorts by workflow ascending by default', async () => { + it('sorts by method ascending by default', async () => { // Arrange asMockedFn(Ajax).mockImplementation( () => mockAjax([sortingMethod, darukMethod, revaliMethod, revaliMethod2]) as AjaxContract @@ -493,7 +493,7 @@ describe('workflows table', () => { checkOrder('daruk method', 'revali method', 'revali method 2', 'sorting method'); }); - it('sorts by workflow descending', async () => { + it('sorts by method descending', async () => { // Arrange asMockedFn(Ajax).mockImplementation( () => mockAjax([sortingMethod, darukMethod, revaliMethod, revaliMethod2]) as AjaxContract @@ -506,7 +506,7 @@ describe('workflows table', () => { render(); }); - await user.click(screen.getByText('Workflow')); + await user.click(screen.getByText('Method')); // Assert checkOrder('sorting method', 'revali method 2', 'revali method', 'daruk method'); @@ -721,14 +721,14 @@ describe('workflows table', () => { expect(screen.getByText('11 - 13 of 13')).toBeInTheDocument(); // Act - await user.click(screen.getByText('Public Workflows (13)')); + await user.click(screen.getByText('Public Methods (13)')); // Assert // Note: the total workflow count (13) is actually still from // the previous tab just in the test because Nav cannot be // mocked properly to actually switch tabs when the public - // workflows tab is clicked + // methods tab is clicked expect(screen.getByText('1 - 10 of 13')).toBeInTheDocument(); }); @@ -753,7 +753,7 @@ describe('workflows table', () => { expect(screen.getByText('11 - 13 of 13')).toBeInTheDocument(); // Act - fireEvent.change(screen.getByPlaceholderText('SEARCH WORKFLOWS'), { target: { value: 'method' } }); + fireEvent.change(screen.getByPlaceholderText('SEARCH METHODS'), { target: { value: 'method' } }); await act(() => delay(300)); // debounced search // Assert @@ -907,11 +907,11 @@ describe('workflows table', () => { // tabs should not display method counts because their // true values are not known - expect(screen.getByText('My Workflows')).toBeInTheDocument(); - expect(screen.getByText('Public Workflows')).toBeInTheDocument(); + expect(screen.getByText('My Methods')).toBeInTheDocument(); + expect(screen.getByText('Public Methods')).toBeInTheDocument(); expect(screen.getByText('Nothing to display')).toBeInTheDocument(); - expect(notify).toHaveBeenCalledWith('error', 'Error loading workflows', expect.anything()); + expect(notify).toHaveBeenCalledWith('error', 'Error loading methods', expect.anything()); }); }); diff --git a/src/pages/workflows/WorkflowList.tsx b/src/pages/workflows/WorkflowList.tsx index 9bc8d55ce3..9fbeaee8f4 100644 --- a/src/pages/workflows/WorkflowList.tsx +++ b/src/pages/workflows/WorkflowList.tsx @@ -20,15 +20,23 @@ import { withBusyState } from 'src/libs/utils'; import { WorkflowModal } from 'src/pages/workflows/workflow/common/WorkflowModal'; import { MethodDefinition } from 'src/pages/workflows/workflow-utils'; +// 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 +// clicking on that tab, the tab query parameter will not be used in the URL) +const tabKeys = ['mine', 'public'] as const; +type TabKey = (typeof tabKeys)[number]; // 'mine' | 'public' + +// Custom type guard +const isTabKey = (val: any): val is TabKey => _.includes(val, tabKeys); + +const defaultTabKey: TabKey = tabKeys[0]; + /** * Represents a list of method definitions grouped into two - * categories — My Workflows and Public Workflows — corresponding + * categories — My Methods and Public Methods — corresponding * to the tabs above the workflows table. */ -interface GroupedWorkflows { - mine: MethodDefinition[]; - public: MethodDefinition[]; -} +type GroupedWorkflows = Record; // This is based on the sort type from the FlexTable component // When that component is converted to TypeScript, we should use its sort type @@ -39,7 +47,7 @@ interface SortProperties { } interface NewQueryParams { - newTab?: string; + newTab?: TabKey; newFilter?: string; } @@ -80,14 +88,16 @@ interface WorkflowListProps { // TODO: consider wrapping query updates in useEffect export const WorkflowList = (props: WorkflowListProps) => { const { queryParams = {} } = props; - const { tab = 'mine', filter = '', ...query } = queryParams; + const { tab: queryTab, filter = '', ...query } = queryParams; + + const selectedTab: TabKey = isTabKey(queryTab) ? queryTab : defaultTabKey; const signal: AbortSignal = useCancellation(); const [busy, setBusy] = useState(false); // workflows is undefined while the method definitions are still loading; // it is null if there is an error while loading - const [workflows, setWorkflows] = useState(); + const [workflows, setWorkflows] = useState(); // Valid direction values are 'asc' and 'desc' (based on expected // function signatures from the Sortable component used in this @@ -99,9 +109,9 @@ export const WorkflowList = (props: WorkflowListProps) => { const [pageNumber, setPageNumber] = useState(1); const [itemsPerPage, setItemsPerPage] = useState(25); - const getTabQueryName = (newTab: string | undefined): string | undefined => (newTab === 'mine' ? undefined : newTab); + const getTabQueryName = (newTab: TabKey): TabKey | undefined => (newTab === defaultTabKey ? undefined : newTab); - const getUpdatedQuery = ({ newTab = tab, newFilter = filter }: NewQueryParams): string => { + const getUpdatedQuery = ({ newTab = selectedTab, newFilter = filter }: NewQueryParams): string => { // Note: setting undefined so that falsy values don't show up at all return qs.stringify( { ...query, tab: getTabQueryName(newTab), filter: newFilter || undefined }, @@ -123,27 +133,30 @@ export const WorkflowList = (props: WorkflowListProps) => { setSort(newSort); }; - const tabName: string = tab || 'mine'; - const tabs = { mine: 'My Workflows', public: 'Public Workflows' }; + const tabNames: Record = { mine: 'My Methods', public: 'Public Methods' }; - const getTabDisplayNames = (workflows: GroupedWorkflows | null | undefined, currentTabName: string) => { - const getCountString = (tabName: keyof GroupedWorkflows): string => { + const getTabDisplayNames = ( + workflows: GroupedWorkflows | null | undefined, + selectedTab: TabKey + ): Record => { + const getCountString = (tab: TabKey): string => { if (workflows == null) { return ''; } - // Only the current tab's workflow count reflects the search + // Only the currently selected tab's workflow count reflects the search // filter (since the filter is cleared when switching tabs) - if (tabName === currentTabName) { + if (tab === selectedTab) { return ` (${sortedWorkflows.length})`; } - return ` (${workflows[tabName].length})`; + return ` (${workflows[tab].length})`; }; - return { - mine: `My Workflows${getCountString('mine')}`, - public: `Public Workflows${getCountString('public')}`, - }; + const tabDisplayNames: Record = { ...tabNames }; // (shallow) copy + for (const tabKey of tabKeys) { + tabDisplayNames[tabKey] += getCountString(tabKey); + } + return tabDisplayNames; }; useOnMount(() => { @@ -160,7 +173,7 @@ export const WorkflowList = (props: WorkflowListProps) => { }); } catch (error) { setWorkflows(null); - notify('error', 'Error loading workflows', { detail: error instanceof Response ? await error.text() : error }); + notify('error', 'Error loading methods', { detail: error instanceof Response ? await error.text() : error }); } }); @@ -174,7 +187,7 @@ export const WorkflowList = (props: WorkflowListProps) => { snapshotId, }); - // Gets the sort key of a method definition based on the currently + // Get the sort key of a method definition based on the currently // selected sort field such that numeric fields are sorted numerically // and other fields are sorted as case-insensitive strings const getSortKey = ({ [sort.field]: sortValue }: MethodDefinition): number | string => { @@ -187,10 +200,15 @@ export const WorkflowList = (props: WorkflowListProps) => { return _.lowerCase(sortValue.toString()); }; - const sortedWorkflows: MethodDefinition[] = _.flow( + const sortedWorkflows: MethodDefinition[] = _.flow< + // filter input type: MethodDefinition[] | undefined (extra [] are because the inputs are viewed as a rest parameter) + (MethodDefinition[] | undefined)[], + MethodDefinition[], // filter output type / orderBy input type + MethodDefinition[] // final result type + >( _.filter(({ namespace, name }: MethodDefinition) => Utils.textMatch(filter, `${namespace}/${name}`)), _.orderBy([getSortKey], [sort.direction]) - )(workflows?.[tabName]); + )(workflows?.[selectedTab]); const firstPageIndex: number = (pageNumber - 1) * itemsPerPage; const lastPageIndex: number = firstPageIndex + itemsPerPage; @@ -198,14 +216,14 @@ export const WorkflowList = (props: WorkflowListProps) => { return ( - + {null /* no additional content to display in the top bar */} `${Nav.getLink('workflows')}${getUpdatedQuery({ newTab: currentTab })}`} getOnClick={(currentTab) => (e) => { e.preventDefault(); @@ -218,8 +236,8 @@ export const WorkflowList = (props: WorkflowListProps) => {
updateQuery({ newFilter: val })} value={filter} /> @@ -238,7 +256,7 @@ export const WorkflowList = (props: WorkflowListProps) => { {({ width, height }) => ( { // @ts-expect-error ( - Workflow + Method ), cellRenderer: ({ rowIndex }) => { @@ -359,8 +377,8 @@ const getColumns = ( export const navPaths = [ { name: 'workflows', - path: '/workflows', + path: '/methods', component: WorkflowList, - title: 'Workflows', + title: 'Broad Methods Repository', }, ]; diff --git a/src/pages/workflows/workflow/WorkflowSummary.test.ts b/src/pages/workflows/workflow/WorkflowSummary.test.ts index 1b9b872780..5d7256353b 100644 --- a/src/pages/workflows/workflow/WorkflowSummary.test.ts +++ b/src/pages/workflows/workflow/WorkflowSummary.test.ts @@ -81,7 +81,7 @@ describe('WorkflowSummary Component', () => { expect(screen.queryAllByText('a fake snapshot')); }); - it('shows Private for private workflows', () => { + it('shows Private for private methods', () => { // ** ARRANGE ** jest.spyOn(snapshotStore, 'get').mockImplementation( jest.fn().mockReturnValue({ @@ -128,7 +128,7 @@ describe('WorkflowSummary Component', () => { expect(screen.queryByText('Synopsis')).not.toBeInTheDocument(); }); - it('shows expected workflow information when collapsable is open', () => { + it('shows expected method information when collapsable is open', () => { // ** ARRANGE ** jest.spyOn(snapshotStore, 'get').mockImplementation( jest.fn().mockReturnValue({ diff --git a/src/pages/workflows/workflow/WorkflowSummary.ts b/src/pages/workflows/workflow/WorkflowSummary.ts index be6319b4fd..1992c784f0 100644 --- a/src/pages/workflows/workflow/WorkflowSummary.ts +++ b/src/pages/workflows/workflow/WorkflowSummary.ts @@ -28,7 +28,7 @@ export const BaseWorkflowSummary = () => { public: isPublic, snapshotComment, } = useStore(snapshotStore); - const persistenceId = `workflows/${namespace}/${name}/dashboard`; + const persistenceId = `methods/${namespace}/${name}/dashboard`; const [importUrlCopied, setImportUrlCopied] = useState(); const importUrl = `${ getConfig().orchestrationUrlRoot @@ -116,7 +116,6 @@ const WorkflowSummary = _.flow( forwardRefWithName('WorkflowSummary'), wrapWorkflows({ breadcrumbs: () => breadcrumbs.commonPaths.workflowList(), - title: 'Workflows', activeTab: 'dashboard', }) )(() => { @@ -126,7 +125,7 @@ const WorkflowSummary = _.flow( export const navPaths = [ { name: 'workflow-dashboard', - path: '/workflows/:namespace/:name/:snapshotId?', + path: '/methods/:namespace/:name/:snapshotId?', component: (props) => h(WorkflowSummary, { ...props, tabName: 'dashboard' }), title: ({ name }) => `${name} - Dashboard`, }, diff --git a/src/pages/workflows/workflow/WorkflowWdl.ts b/src/pages/workflows/workflow/WorkflowWdl.ts index 0303ac3e5f..a23b318141 100644 --- a/src/pages/workflows/workflow/WorkflowWdl.ts +++ b/src/pages/workflows/workflow/WorkflowWdl.ts @@ -37,7 +37,6 @@ const WorkflowWdl = _.flow( forwardRefWithName('WorkflowWdl'), wrapWorkflows({ breadcrumbs: () => breadcrumbs.commonPaths.workflowList(), - title: 'Workflows', activeTab: 'wdl', }) )(() => { @@ -47,7 +46,7 @@ const WorkflowWdl = _.flow( export const navPaths = [ { name: 'workflow-wdl', - path: '/workflows/:namespace/:name/:snapshotId/wdl', + path: '/methods/:namespace/:name/:snapshotId/wdl', component: (props) => h(WorkflowWdl, { ...props, tabName: 'wdl' }), title: ({ name }) => `${name} - WDL`, }, diff --git a/src/pages/workflows/workflow/WorkflowWrapper.test.tsx b/src/pages/workflows/workflow/WorkflowWrapper.test.tsx index 48987f52e3..d420d08858 100644 --- a/src/pages/workflows/workflow/WorkflowWrapper.test.tsx +++ b/src/pages/workflows/workflow/WorkflowWrapper.test.tsx @@ -27,7 +27,7 @@ jest.mock( (): NavExports => ({ ...jest.requireActual('src/libs/nav'), getLink: jest.fn((name, pathParams?) => - name === 'workflow-dashboard' ? `#workflows/${pathParams!.namespace}/${pathParams!.name}` : `#${name}` + name === 'workflow-dashboard' ? `#methods/${pathParams!.namespace}/${pathParams!.name}` : `#${name}` ), goToPath: jest.fn(), }) @@ -164,7 +164,6 @@ const MockWrappedWorkflowComponent = _.flow( forwardRefWithName('MockWrappedWorkflowComponent'), wrapWorkflows({ breadcrumbs: () => breadcrumbs.commonPaths.workflowList(), - title: 'Methods', activeTab: 'dashboard', }) )(() => { @@ -307,6 +306,8 @@ describe('workflow wrapper', () => { const returnToMethodsListButton = screen.getByRole('link', { name: 'Return to Methods List' }); expect(returnToMethodsListButton).toBeInTheDocument(); + + // mock link path based on internal nav path name expect(returnToMethodsListButton).toHaveAttribute('href', '#workflows'); }); @@ -397,7 +398,7 @@ describe('workflows container', () => { Ajax().Methods.method(mockDeleteSnapshot.namespace, mockDeleteSnapshot.name, mockDeleteSnapshot.snapshotId).delete ).toHaveBeenCalled(); - expect(window.history.replaceState).toHaveBeenCalledWith({}, '', '#workflows/methodnamespace/testname'); + expect(window.history.replaceState).toHaveBeenCalledWith({}, '', '#methods/methodnamespace/testname'); expect(snapshotStore.get()).toEqual(snapshotStoreInitialValue); diff --git a/src/pages/workflows/workflow/WorkflowWrapper.ts b/src/pages/workflows/workflow/WorkflowWrapper.ts index 1bb7aca73d..39a1af4cc3 100644 --- a/src/pages/workflows/workflow/WorkflowWrapper.ts +++ b/src/pages/workflows/workflow/WorkflowWrapper.ts @@ -27,7 +27,6 @@ import * as WorkspaceUtils from 'src/workspaces/utils'; export interface WrapWorkflowOptions { breadcrumbs: (props: { name: string; namespace: string }) => ReactNode[]; activeTab?: string; - title: string | ((props: { name: string; namespace: string }) => string); } interface WorkflowWrapperProps extends PropsWithChildren { @@ -89,7 +88,7 @@ export const wrapWorkflows = (opts: WrapWorkflowOptions) => { }); return h(FooterWrapper, [ - h(TopBar, { title: 'Workflows', href: Nav.getLink('workflows') }, [ + h(TopBar, { title: 'Broad Methods Repository', href: Nav.getLink('workflows') }, [ div({ style: Style.breadcrumb.breadcrumb }, [ div(breadcrumbs(props)), div({ style: Style.breadcrumb.textUnderBreadcrumb }, [`${namespace}/${name}`]), @@ -176,9 +175,9 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { await Ajax(signal).Methods.method(namespace, name, selectedSnapshot).delete(); // Replace the current history entry linking to the method details page of a - // specific snapshot, like /#workflows/sschu/echo-strings-test/29, with an + // specific snapshot, like /#methods/sschu/echo-strings-test/29, with an // entry with the corresponding link without the snapshot ID, like - // /#workflows/sschu/echo-strings-test + // /#methods/sschu/echo-strings-test // This way, if the user presses the back button after deleting a // method snapshot, they will be automatically redirected to the most recent // snapshot that still exists of the same method @@ -202,7 +201,7 @@ export const WorkflowsContainer = (props: WorkflowContainerProps) => { h( TabBar, { - 'aria-label': 'workflow menu', + 'aria-label': 'method details menu', activeTab: tabName, tabNames: ['dashboard', 'wdl'], getHref: (currentTab) => diff --git a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx index e4edc76993..75b7848ce4 100644 --- a/src/pages/workflows/workflow/common/WorkflowModal.test.tsx +++ b/src/pages/workflows/workflow/common/WorkflowModal.test.tsx @@ -84,7 +84,7 @@ describe('WorkflowModal', () => { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render( { await act(async () => { render(