Skip to content

Commit

Permalink
fix(tekton): fix janus-idp#947: Reset pagination when changing any fi…
Browse files Browse the repository at this point in the history 
…lter (janus-idp#1140)

Signed-off-by: Christoph Jerolimov <jerolimov+github@redhat.com>
  • Loading branch information
@christoph-jerolimov
christoph-jerolimov authored Jan 31, 2024
1 parent 03a678d commit 5a1a2f4
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 146 deletions.
19 changes: 2 additions & 17 deletions plugins/shared-react/src/types/pipeline/computedStatus.ts
View file
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,
};
17 changes: 8 additions & 9 deletions plugins/shared-react/src/utils/pipeline/pipeline.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ describe('getRunStatusColor should handle ComputedStatus values', () => {
it('should expect all but PipelineNotStarted to produce a non-default result', () => {
// Verify that we cover colour states for all the ComputedStatus values
const failCase = 'PipelineNotStarted';
const defaultCase = getRunStatusColor(ComputedStatus[failCase]);
const allOtherStatuses = Object.keys(ComputedStatus)
.filter(
status =>
status !== failCase &&
ComputedStatus[status as keyof typeof ComputedStatus] !==
ComputedStatus.Other,
)
.map(status => ComputedStatus[status as keyof typeof ComputedStatus]);
const defaultCase = getRunStatusColor(failCase);
const allStatuses = Object.keys(ComputedStatus) as ComputedStatus[];
const allOtherStatuses = allStatuses.filter(
status =>
status !== ComputedStatus.All &&
status !== ComputedStatus.Other &&
status !== failCase,
);

expect(allOtherStatuses).not.toHaveLength(0);
allOtherStatuses.forEach(statusValue => {
Expand Down
16 changes: 8 additions & 8 deletions plugins/tekton/src/components/PipelineRunList/PipelineRunList.test.tsx
View file
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
View file
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
29 changes: 13 additions & 16 deletions plugins/tekton/src/components/PipelineRunList/PipelineRunListSearchBar.test.tsx
View file
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('');
});
});
Loading

0 comments on commit 5a1a2f4

Please sign in to comment.