Skip to content

Commit

Permalink
fix(tekton): fix #947: Reset pagination when changing any filter
Browse files Browse the repository at this point in the history
Signed-off-by: Christoph Jerolimov <[email protected]>
  • Loading branch information
christoph-jerolimov committed Jan 30, 2024
1 parent ad50c44 commit 605f2f4
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 137 deletions.
19 changes: 2 additions & 17 deletions plugins/shared-react/src/types/pipeline/computedStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export enum TerminatedReasons {
}

export enum ComputedStatus {
All = 'All',
Cancelling = 'Cancelling',
Succeeded = 'Succeeded',
Failed = 'Failed',
Expand All @@ -14,7 +15,7 @@ export enum ComputedStatus {
Cancelled = 'Cancelled',
Pending = 'Pending',
Idle = 'Idle',
Other = '-',
Other = 'Other',
}

export enum SucceedConditionReason {
Expand Down Expand Up @@ -45,19 +46,3 @@ export type TaskStatusTypes = {
Failed: number;
Skipped: number;
};

export const computedStatus: { [key: string]: string | ComputedStatus } = {
All: '',
Cancelling: ComputedStatus.Cancelling,
Succeeded: ComputedStatus.Succeeded,
Failed: ComputedStatus.Failed,
Running: ComputedStatus.Running,
'In Progress': ComputedStatus['In Progress'],
FailedToStart: ComputedStatus.FailedToStart,
PipelineNotStarted: ComputedStatus.PipelineNotStarted,
Skipped: ComputedStatus.Skipped,
Cancelled: ComputedStatus.Cancelled,
Pending: ComputedStatus.Pending,
Idle: ComputedStatus.Idle,
Other: ComputedStatus.Other,
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BrowserRouter } from 'react-router-dom';

import { render } from '@testing-library/react';

import { computedStatus } from '@janus-idp/shared-react';
import { ComputedStatus } from '@janus-idp/shared-react';

import { mockKubernetesPlrResponse } from '../../__fixtures__/1-pipelinesData';
import { TektonResourcesContext } from '../../hooks/TektonResourcesContext';
Expand Down Expand Up @@ -35,7 +35,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: ['ocp'],
setSelectedCluster: () => {},
selectedStatus: '',
selectedStatus: ComputedStatus.All,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand All @@ -59,7 +59,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: [],
setSelectedCluster: () => {},
selectedStatus: '',
selectedStatus: ComputedStatus.All,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: [],
setSelectedCluster: () => {},
selectedStatus: '',
selectedStatus: ComputedStatus.All,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down Expand Up @@ -116,7 +116,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: [],
setSelectedCluster: () => {},
selectedStatus: '',
selectedStatus: ComputedStatus.All,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down Expand Up @@ -146,7 +146,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [{ message: '403 - forbidden' }],
clusters: ['ocp'],
setSelectedCluster: () => {},
selectedStatus: '',
selectedStatus: ComputedStatus.All,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down Expand Up @@ -175,7 +175,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: ['ocp'],
setSelectedCluster: () => {},
selectedStatus: computedStatus.Succeeded,
selectedStatus: ComputedStatus.Succeeded,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down Expand Up @@ -206,7 +206,7 @@ describe('PipelineRunList', () => {
selectedClusterErrors: [],
clusters: ['ocp'],
setSelectedCluster: () => {},
selectedStatus: computedStatus.Cancelled,
selectedStatus: ComputedStatus.Cancelled,
setSelectedStatus: () => {},
setIsExpanded: () => {},
};
Expand Down
99 changes: 55 additions & 44 deletions plugins/tekton/src/components/PipelineRunList/PipelineRunList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '@material-ui/core';

import {
computedStatus,
ComputedStatus,
PipelineRunKind,
pipelineRunStatus,
} from '@janus-idp/shared-react';
Expand Down Expand Up @@ -91,36 +91,61 @@ const PipelineRunList = () => {
watchResourcesData,
selectedClusterErrors,
clusters,
selectedCluster,
selectedStatus,
} = React.useContext(TektonResourcesContext);
const [search, setSearch] = React.useState<string>('');
const [order, setOrder] = React.useState<Order>('desc');
const [orderBy, setOrderBy] = React.useState<string>('status.startTime');
const [orderById, setOrderById] = React.useState<string>('startTime'); // 2 columns have the same field
const [page, setPage] = React.useState<number>(0);
const [rowsPerPage, setRowsPerPage] = React.useState<number>(5);
const [filteredPipelineRuns, setFilteredPipelineRuns] = React.useState<
PipelineRunKind[] | undefined
>();

const totalPlrs = watchResourcesData?.pipelineruns?.data?.map(d => ({
...d,
id: d.metadata.uid,
}));

const pipelineRunsData = React.useMemo(
() =>
selectedStatus === computedStatus.All
? totalPlrs
: // eslint-disable-next-line consistent-return
totalPlrs?.filter(plr => {
if (pipelineRunStatus(plr) === selectedStatus) {
return plr;
}
}),
[selectedStatus, totalPlrs],
);

const pipelineRuns = filteredPipelineRuns || pipelineRunsData;
// Jump to first page when cluster, status and search filter changed
const updateStateOnFilterChanges = React.useRef(false);
React.useEffect(() => {
if (updateStateOnFilterChanges.current) {
setPage(0);
} else {
updateStateOnFilterChanges.current = true;
}
}, [selectedCluster, selectedStatus, search]);

const allPipelineRuns = React.useMemo(() => {
const plrs =
watchResourcesData?.pipelineruns?.data?.map(d => ({
...d,
id: d.metadata.uid,
})) ?? [];
return plrs as PipelineRunKind[];
}, [watchResourcesData]);

const filteredPipelineRuns = React.useMemo(() => {
let plrs = allPipelineRuns;

if (selectedStatus && selectedStatus !== ComputedStatus.All) {
plrs = plrs.filter(plr => pipelineRunStatus(plr) === selectedStatus);
}

if (search) {
const f = search.toUpperCase();
plrs = plrs.filter((plr: PipelineRunKind) => {
const n = plr.metadata?.name?.toUpperCase();
return n?.includes(f);
});
}

plrs = plrs.sort(getComparator(order, orderBy, orderById));

return plrs;
}, [allPipelineRuns, selectedStatus, search, order, orderBy, orderById]);

const visibleRows = React.useMemo(() => {
return filteredPipelineRuns.slice(
page * rowsPerPage,
page * rowsPerPage + rowsPerPage,
);
}, [filteredPipelineRuns, page, rowsPerPage]);

const handleRequestSort = (
_event: React.MouseEvent<unknown>,
Expand All @@ -147,19 +172,12 @@ const PipelineRunList = () => {
// Avoid a layout jump when reaching the last page with empty rows.
const emptyRows =
page > 0
? Math.max(0, (1 + page) * rowsPerPage - (pipelineRuns?.length ?? 0))
? Math.max(
0,
(1 + page) * rowsPerPage - (filteredPipelineRuns.length ?? 0),
)
: 0;

const visibleRows = React.useMemo(
() =>
pipelineRuns
?.sort(getComparator(order, orderBy, orderById))
.slice(
page * rowsPerPage,
page * rowsPerPage + rowsPerPage,
) as PipelineRunKind[],
[pipelineRuns, order, orderBy, orderById, page, rowsPerPage],
);
const classes = useStyles();

const allErrors: ClusterErrors = [
Expand All @@ -186,10 +204,7 @@ const PipelineRunList = () => {
<Typography variant="h5" component="h2">
Pipeline Runs
</Typography>
<PipelineRunListSearchBar
pipelineRuns={pipelineRunsData}
onSearch={setFilteredPipelineRuns}
/>
<PipelineRunListSearchBar value={search} onChange={setSearch} />
</Toolbar>
<Table
aria-labelledby="Pipeline Runs"
Expand All @@ -206,11 +221,7 @@ const PipelineRunList = () => {
<TableBody>
<PipelineRunTableBody rows={visibleRows} />
{emptyRows > 0 && (
<TableRow
style={{
height: 33 * emptyRows,
}}
>
<TableRow style={{ height: 55 * emptyRows }}>
<TableCell colSpan={PipelineRunColumnHeader.length} />
</TableRow>
)}
Expand All @@ -221,7 +232,7 @@ const PipelineRunList = () => {
{ value: 10, label: '10 rows' },
{ value: 25, label: '25 rows' },
]}
count={pipelineRuns?.length ?? 0}
count={filteredPipelineRuns.length}
rowsPerPage={rowsPerPage}
page={page}
onPageChange={handleChangePage}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import { fireEvent, render, screen } from '@testing-library/react';

import { PipelineRunListSearchBar } from './PipelineRunListSearchBar';

jest.mock('react-use/lib/useDebounce', () => {
return jest.fn(fn => fn);
});

describe('PipelineRunListSearchBar', () => {
test('renders PipelineRunListSearchBar component', () => {
const { getByPlaceholderText } = render(
<PipelineRunListSearchBar onSearch={() => {}} />,
<PipelineRunListSearchBar value="" onChange={() => {}} />,
);

screen.logTestingPlaygroundURL();
Expand All @@ -20,28 +16,29 @@ describe('PipelineRunListSearchBar', () => {
});

test('handles search input change', () => {
const { getByPlaceholderText } = render(
<PipelineRunListSearchBar onSearch={() => {}} />,
const onChange = jest.fn();
const { getByPlaceholderText, getByTestId } = render(
<PipelineRunListSearchBar value="" onChange={onChange} />,
);
const searchInput = getByPlaceholderText('Search');
const clearButton = getByTestId('clear-search');
expect(clearButton.getAttribute('disabled')).toBe(''); // disabled

fireEvent.change(searchInput, { target: { value: 'example' } });

expect((searchInput as HTMLInputElement).value).toBe('example');
expect(onChange).toHaveBeenCalledWith('example');
});

test('clears search input', () => {
const { getByPlaceholderText, getByTestId } = render(
<PipelineRunListSearchBar onSearch={() => {}} />,
const onChange = jest.fn();
const { getByTestId } = render(
<PipelineRunListSearchBar value="example" onChange={onChange} />,
);
const searchInput = getByPlaceholderText('Search');

fireEvent.change(searchInput, { target: { value: 'example' } });

expect((searchInput as HTMLInputElement).value).toBe('example');
const clearButton = getByTestId('clear-search');
expect(clearButton.getAttribute('disabled')).toBe(null); // not disabled

fireEvent.click(getByTestId('clear-search'));

expect((searchInput as HTMLInputElement).value).toBe('');
expect(onChange).toHaveBeenCalledWith('');
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import useDebounce from 'react-use/lib/useDebounce';

import {
FormControl,
Expand All @@ -11,11 +10,9 @@ import {
import Clear from '@material-ui/icons/Clear';
import Search from '@material-ui/icons/Search';

import { PipelineRunKind } from '@janus-idp/shared-react';

type PipelineRunListSearchBarProps = {
pipelineRuns?: PipelineRunKind[];
onSearch: React.Dispatch<React.SetStateAction<PipelineRunKind[] | undefined>>;
value: string;
onChange: (filter: string) => void;
};

const useStyles = makeStyles({
Expand All @@ -26,40 +23,19 @@ const useStyles = makeStyles({
});

export const PipelineRunListSearchBar = ({
pipelineRuns,
onSearch,
value,
onChange,
}: PipelineRunListSearchBarProps) => {
const [search, setSearch] = React.useState<string>('');
const classes = useStyles();

const searchByName = () => {
const filteredPipelineRuns =
pipelineRuns && search
? pipelineRuns.filter((plr: PipelineRunKind) => {
const s = search.toUpperCase();
const n = plr.metadata?.name?.toUpperCase();
return n?.includes(s);
})
: undefined;
onSearch(filteredPipelineRuns);
};

useDebounce(
() => {
searchByName();
},
100,
[search],
);

return (
<FormControl className={classes.formControl}>
<Input
aria-label="search"
placeholder="Search"
autoComplete="off"
onChange={event => setSearch(event.target.value)}
value={search}
onChange={event => onChange(event.target.value)}
value={value}
startAdornment={
<InputAdornment position="start">
<Search />
Expand All @@ -69,9 +45,9 @@ export const PipelineRunListSearchBar = ({
<InputAdornment position="end">
<IconButton
aria-label="clear search"
onClick={() => setSearch('')}
onClick={() => onChange('')}
edge="end"
disabled={search.length === 0}
disabled={!value}
data-testid="clear-search"
>
<Clear />
Expand Down
Loading

0 comments on commit 605f2f4

Please sign in to comment.