Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 'Only Mine' toggle for execution tasks list #335

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"@storybook/testing-library": "^0.0.9",
"@testing-library/jest-dom": "^5.5.0",
"@testing-library/react": "^10.0.3",
"@testing-library/react-hooks": "^7.0.2",
"@types/cheerio": "^0.22.2",
"@types/compression-webpack-plugin": "^9.1.1",
"@types/d3-shape": "^1.2.6",
Expand Down
12 changes: 5 additions & 7 deletions src/components/Entities/EntityExecutions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import { Typography } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import { contentMarginGridUnits } from 'common/layout';
import { fetchStates } from 'components/hooks/types';
import { WaitForData } from 'components/common/WaitForData';
import { ExecutionFilters } from 'components/Executions/ExecutionFilters';
import { useExecutionShowArchivedState } from 'components/Executions/filters/useExecutionArchiveState';
Expand All @@ -14,6 +13,7 @@ import { SortDirection } from 'models/AdminEntity/types';
import { ResourceIdentifier } from 'models/Common/types';
import { executionSortFields } from 'models/Execution/constants';
import { compact } from 'lodash';
import { useOnlyMyExecutionsFilterState } from 'components/Executions/filters/useOnlyMyExecutionsFilterState';
import { executionFilterGenerator } from './generators';

const useStyles = makeStyles((theme: Theme) => ({
Expand Down Expand Up @@ -42,6 +42,7 @@ export const EntityExecutions: React.FC<EntityExecutionsProps> = ({
const styles = useStyles();
const filtersState = useWorkflowExecutionFiltersState();
const archivedFilter = useExecutionShowArchivedState();
const onlyMyExecutionsFilterState = useOnlyMyExecutionsFilterState({});

const sort = {
key: executionSortFields.createdAt,
Expand All @@ -57,7 +58,9 @@ export const EntityExecutions: React.FC<EntityExecutionsProps> = ({
...baseFilters,
...filtersState.appliedFilters,
archivedFilter.getFilter(),
onlyMyExecutionsFilterState.getFilter(),
]);

const executions = useWorkflowExecutions(
{ domain, project },
{
Expand All @@ -71,12 +74,6 @@ export const EntityExecutions: React.FC<EntityExecutionsProps> = ({
executions.value = executions.value.filter((item) => chartIds.includes(item.id.name));
}

/** Don't render component until finish fetching user profile */
const lastIndex = filtersState.filters.length - 1;
if (filtersState.filters[lastIndex].status !== fetchStates.LOADED) {
return null;
}

return (
<>
<Typography className={styles.header} variant="h6">
Expand All @@ -89,6 +86,7 @@ export const EntityExecutions: React.FC<EntityExecutionsProps> = ({
clearCharts={clearCharts}
showArchived={archivedFilter.showArchived}
onArchiveFilterChange={archivedFilter.setShowArchived}
onlyMyExecutionsFilterState={onlyMyExecutionsFilterState}
/>
</div>
<WaitForData {...executions}>
Expand Down
6 changes: 0 additions & 6 deletions src/components/Entities/EntityExecutionsBarChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,6 @@ export const EntityExecutionsBarChart: React.FC<EntityExecutionsBarChartProps> =
[onToggle],
);

/** Don't render component until finish fetching user profile */
const lastIndex = filtersState.filters.length - 1;
if (filtersState.filters[lastIndex].status !== fetchStates.LOADED) {
return null;
}

return (
<WaitForData {...executions}>
<Typography className={styles.header} variant="h6">
Expand Down
50 changes: 42 additions & 8 deletions src/components/Executions/ExecutionFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ const useStyles = makeStyles((theme: Theme) => ({
},
}));

interface OnlyMyExecutionsFilterState {
onlyMyExecutionsValue: boolean;
isFilterDisabled: boolean;
onOnlyMyExecutionsFilterChange: (filterOnlyMyExecutions: boolean) => void;
}

export interface ExecutionFiltersProps {
filters: (FilterState | BooleanFilterState)[];
chartIds?: string[];
clearCharts?: () => void;
showArchived?: boolean;
onArchiveFilterChange?: (showArchievedItems: boolean) => void;
onlyMyExecutionsFilterState?: OnlyMyExecutionsFilterState;
}

const RenderFilter: React.FC<{ filter: FilterState }> = ({ filter }) => {
const searchFilterState = filter as SearchFilterState;
switch (filter.type) {
Expand All @@ -51,13 +66,14 @@ const RenderFilter: React.FC<{ filter: FilterState }> = ({ filter }) => {
* This allows for the consuming code to have direct access to the
* current filters without relying on complicated callback arrangements
*/
export const ExecutionFilters: React.FC<{
filters: (FilterState | BooleanFilterState)[];
chartIds?: string[];
clearCharts?: () => void;
showArchived?: boolean;
onArchiveFilterChange?: (showArchievedItems: boolean) => void;
}> = ({ filters, chartIds, clearCharts, showArchived, onArchiveFilterChange }) => {
export const ExecutionFilters: React.FC<ExecutionFiltersProps> = ({
filters,
chartIds,
clearCharts,
showArchived,
onArchiveFilterChange,
onlyMyExecutionsFilterState,
}) => {
const styles = useStyles();

filters = filters.map((filter) => {
Expand Down Expand Up @@ -111,14 +127,32 @@ export const ExecutionFilters: React.FC<{
buttonText="Clear Manually Selected Executions"
onReset={clearCharts}
key="charts"
data-testId="clear-charts"
/>
)}
{!!onlyMyExecutionsFilterState && (
<FormGroup>
<FormControlLabel
control={
<Checkbox
checked={onlyMyExecutionsFilterState.onlyMyExecutionsValue}
disabled={onlyMyExecutionsFilterState.isFilterDisabled}
onChange={(_, checked) =>
onlyMyExecutionsFilterState.onOnlyMyExecutionsFilterChange(checked)
}
/>
}
className={styles.checkbox}
label="Only my executions"
/>
</FormGroup>
)}
{!!onArchiveFilterChange && (
<FormGroup>
<FormControlLabel
control={
<Checkbox
value={showArchived}
checked={showArchived}
onChange={(_, checked) => onArchiveFilterChange(checked)}
/>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ stories.add('Workflow executions - all', () => (
clearCharts={action('clearCharts')}
showArchived={false}
onArchiveFilterChange={action('onArchiveFilterChange')}
onlyMyExecutionsFilterState={{
isFilterDisabled: false,
onlyMyExecutionsValue: false,
onOnlyMyExecutionsFilterChange: action('onOnlyMyExecutionsFilterChange'),
}}
/>
));
stories.add('Workflow executions - minimal', () => (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { loadedFetchable } from 'components/hooks/__mocks__/fetchableData';
import { useUserProfile } from 'components/hooks/useUserProfile';
import { useOnlyMyExecutionsFilterState } from 'components/Executions/filters/useOnlyMyExecutionsFilterState';
import { UserProfile } from 'models/Common/types';
import { FetchableData } from 'components/hooks/types';
import { renderHook } from '@testing-library/react-hooks';

jest.mock('components/hooks/useUserProfile');

describe('useOnlyMyExecutionsFilterState', () => {
const mockUseRemoteLiteralMap = useUserProfile as jest.Mock<FetchableData<UserProfile | null>>;
mockUseRemoteLiteralMap.mockReturnValue(loadedFetchable(null, jest.fn()));

describe.each`
isFilterDisabled | initialValue | expected
${undefined} | ${undefined} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: false }}
${false} | ${undefined} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: false }}
${true} | ${undefined} | ${{ isFilterDisabled: true, onlyMyExecutionsValue: false }}
${undefined} | ${false} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: false }}
${undefined} | ${true} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: true }}
${false} | ${false} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: false }}
${false} | ${true} | ${{ isFilterDisabled: false, onlyMyExecutionsValue: true }}
${true} | ${false} | ${{ isFilterDisabled: true, onlyMyExecutionsValue: false }}
${true} | ${true} | ${{ isFilterDisabled: true, onlyMyExecutionsValue: true }}
`('for each case', ({ isFilterDisabled, initialValue, expected }) => {
Comment on lines +14 to +25
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially wrote this up as individual tests, but thought this may be a good use case to digest them as a table. I have the original code handy, so wouldn't be a problem to revert, if it's just my taste :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any particular taste, but curious how it will look like if one of them fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anrusina only failed cases will show up, like in this screenshot, test case title is customized on line 26, and I just stringified expected to show value instead of [Object Object]
Screen Shot 2022-03-21 at 5 42 53 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

it(`should return ${JSON.stringify(
expected,
)} when called with isFilterDisabled = ${isFilterDisabled} and initialValue = ${initialValue}`, () => {
const { result } = renderHook(() =>
useOnlyMyExecutionsFilterState({ isFilterDisabled, initialValue }),
);
expect(result.current).toEqual(expect.objectContaining(expected));
});
});
});
2 changes: 0 additions & 2 deletions src/components/Executions/filters/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ export type FilterStateType = 'single' | 'multi' | 'search' | 'boolean';
export interface FilterState {
active: boolean;
button: FilterButtonState;
hidden?: boolean;
label: string;
type: FilterStateType;
status?: string;
getFilter: () => FilterOperation[];
onReset: () => void;
onChange?: (value) => void;
Expand Down
46 changes: 0 additions & 46 deletions src/components/Executions/filters/useCurrentUserOnlyFilterState.ts

This file was deleted.

2 changes: 0 additions & 2 deletions src/components/Executions/filters/useExecutionFiltersState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FilterState } from './types';
import { useMultiFilterState } from './useMultiFilterState';
import { useSearchFilterState } from './useSearchFilterState';
import { useSingleFilterState } from './useSingleFilterState';
import { useCurrentUserOnlyFilterState } from './useCurrentUserOnlyFilterState';

export interface ExecutionFiltersState {
appliedFilters: FilterOperation[];
Expand Down Expand Up @@ -57,7 +56,6 @@ export function useWorkflowExecutionFiltersState() {
label: filterLabels.duration,
queryStateKey: 'duration',
}),
useCurrentUserOnlyFilterState(),
]);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { useState } from 'react';
import { FilterOperation, FilterOperationName } from 'models/AdminEntity/types';
import { useUserProfile } from 'components/hooks/useUserProfile';

interface OnlyMyExecutionsFilterState {
onlyMyExecutionsValue: boolean;
isFilterDisabled: boolean;
onOnlyMyExecutionsFilterChange: (newValue: boolean) => void;
getFilter: () => FilterOperation | null;
}

interface OnlyMyExecutionsFilterStateProps {
isFilterDisabled?: boolean;
initialValue?: boolean;
}

/**
* Allows to filter executions by Current User Id
*/
export function useOnlyMyExecutionsFilterState({
isFilterDisabled,
initialValue,
}: OnlyMyExecutionsFilterStateProps): OnlyMyExecutionsFilterState {
const profile = useUserProfile();
const userId = profile.value?.subject ? profile.value.subject : '';
const [onlyMyExecutionsValue, setOnlyMyExecutionsValue] = useState<boolean>(
initialValue ?? false,
);

const getFilter = (): FilterOperation | null => {
if (!onlyMyExecutionsValue) {
return null;
}

return {
key: 'user',
value: userId,
operation: FilterOperationName.EQ,
};
};

return {
onlyMyExecutionsValue,
isFilterDisabled: isFilterDisabled ?? false,
onOnlyMyExecutionsFilterChange: setOnlyMyExecutionsValue,
getFilter,
};
}
Loading