Skip to content

Commit

Permalink
[AN-156] Broad Methods Repository: "workflows" to "methods" (#5098)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sam-schu
sam-schu authored Nov 1, 2024
1 parent ac94dfa commit b0f16cd
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 89 deletions.
2 changes: 1 addition & 1 deletion src/components/breadcrumbs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'))],

Expand Down
50 changes: 25 additions & 25 deletions src/pages/workflows/WorkflowList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}));

Expand Down Expand Up @@ -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 () => {
Expand All @@ -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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
);

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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
Expand All @@ -435,8 +435,8 @@ describe('workflows table', () => {
render(<WorkflowList queryParams={{ filter: 'test' }} />);
});

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);
Expand All @@ -457,7 +457,7 @@ describe('workflows table', () => {
render(<WorkflowList />);
});

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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -506,7 +506,7 @@ describe('workflows table', () => {
render(<WorkflowList queryParams={{ tab: 'public' }} />);
});

await user.click(screen.getByText('Workflow'));
await user.click(screen.getByText('Method'));

// Assert
checkOrder('sorting method', 'revali method 2', 'revali method', 'daruk method');
Expand Down Expand Up @@ -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();
});

Expand All @@ -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
Expand Down Expand Up @@ -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());
});
});

Expand Down
92 changes: 55 additions & 37 deletions src/pages/workflows/WorkflowList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TabKey, MethodDefinition[]>;

// This is based on the sort type from the FlexTable component
// When that component is converted to TypeScript, we should use its sort type
Expand All @@ -39,7 +47,7 @@ interface SortProperties {
}

interface NewQueryParams {
newTab?: string;
newTab?: TabKey;
newFilter?: string;
}

Expand Down Expand Up @@ -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<boolean>(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<GroupedWorkflows | null>();
const [workflows, setWorkflows] = useState<GroupedWorkflows | null | undefined>();

// Valid direction values are 'asc' and 'desc' (based on expected
// function signatures from the Sortable component used in this
Expand All @@ -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 },
Expand All @@ -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<TabKey, string> = { 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<TabKey, string> => {
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<TabKey, string> = { ...tabNames }; // (shallow) copy
for (const tabKey of tabKeys) {
tabDisplayNames[tabKey] += getCountString(tabKey);
}
return tabDisplayNames;
};

useOnMount(() => {
Expand All @@ -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 });
}
});

Expand All @@ -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 => {
Expand All @@ -187,25 +200,30 @@ export const WorkflowList = (props: WorkflowListProps) => {
return _.lowerCase(sortValue.toString());
};

const sortedWorkflows: MethodDefinition[] = _.flow<MethodDefinition[], MethodDefinition[], MethodDefinition[]>(
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;
const paginatedWorkflows: MethodDefinition[] = sortedWorkflows.slice(firstPageIndex, lastPageIndex);

return (
<FooterWrapper>
<TopBar title='Workflows' href=''>
<TopBar title='Broad Methods Repository' href=''>
{null /* no additional content to display in the top bar */}
</TopBar>
<TabBar
aria-label='workflows menu'
activeTab={tabName}
tabNames={Object.keys(tabs)}
displayNames={getTabDisplayNames(workflows, tabName)}
aria-label='methods list menu'
activeTab={selectedTab}
tabNames={tabKeys}
displayNames={getTabDisplayNames(workflows, selectedTab)}
getHref={(currentTab) => `${Nav.getLink('workflows')}${getUpdatedQuery({ newTab: currentTab })}`}
getOnClick={(currentTab) => (e) => {
e.preventDefault();
Expand All @@ -218,8 +236,8 @@ export const WorkflowList = (props: WorkflowListProps) => {
<div style={{ display: 'flex' }}>
<DelayedSearchInput
style={{ width: 500, display: 'flex', justifyContent: 'flex-start' }}
placeholder='SEARCH WORKFLOWS'
aria-label='Search workflows'
placeholder='SEARCH METHODS'
aria-label='Search methods'
onChange={(val) => updateQuery({ newFilter: val })}
value={filter}
/>
Expand All @@ -238,7 +256,7 @@ export const WorkflowList = (props: WorkflowListProps) => {
<AutoSizer>
{({ width, height }) => (
<FlexTable
aria-label={tabs[tabName]}
aria-label={tabNames[selectedTab]}
width={width}
height={height}
sort={sort as any /* necessary until FlexTable is converted to TS */}
Expand All @@ -257,7 +275,7 @@ export const WorkflowList = (props: WorkflowListProps) => {
// @ts-expect-error
<Paginator
filteredDataLength={sortedWorkflows.length}
unfilteredDataLength={workflows![tabName].length}
unfilteredDataLength={workflows![selectedTab].length}
pageNumber={pageNumber}
setPageNumber={setPageNumber}
itemsPerPage={itemsPerPage}
Expand Down Expand Up @@ -295,7 +313,7 @@ const getColumns = (
field: 'name',
headerRenderer: () => (
<WorkflowTableHeader sort={sort} field='name' onSort={onSort}>
Workflow
Method
</WorkflowTableHeader>
),
cellRenderer: ({ rowIndex }) => {
Expand Down Expand Up @@ -359,8 +377,8 @@ const getColumns = (
export const navPaths = [
{
name: 'workflows',
path: '/workflows',
path: '/methods',
component: WorkflowList,
title: 'Workflows',
title: 'Broad Methods Repository',
},
];
Loading

0 comments on commit b0f16cd

Please sign in to comment.