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

Limit last page to the last existing page in the table #1121

Merged
merged 1 commit into from
Feb 19, 2024
Merged
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
6 changes: 2 additions & 4 deletions frontend/src/components/pages/topics/Tab.Consumers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,17 @@ import { editQuery } from '../../../utils/queryHelper';
type TopicConsumersProps = { topic: Topic };

export const TopicConsumers: FC<TopicConsumersProps> = observer(({ topic }) => {
const paginationParams = usePaginationParams(uiState.topicSettings.consumerPageSize);

let consumers = api.topicConsumers.get(topic.topicName);
const isLoading = consumers === null;

if(isLoading) {
return DefaultSkeleton;
}

if (!consumers) {
consumers = [];
}

const paginationParams = usePaginationParams(uiState.topicSettings.consumerPageSize, consumers.length);

return (
<DataTable<TopicConsumer>
data={consumers}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ export class TopicMessageView extends Component<TopicMessageViewProps> {
}

MessageTable = observer(() => {
const paginationParams = usePaginationParams(uiState.topicSettings.searchParams.pageSize)
const paginationParams = usePaginationParams(uiState.topicSettings.searchParams.pageSize, this.messageSource.data.length)
const [showPreviewSettings, setShowPreviewSettings] = React.useState(false);

const tsFormat = uiState.topicSettings.previewTimestamps;
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/pages/topics/Tab.Partitions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ type TopicPartitionsProps = {
};

export const TopicPartitions: FC<TopicPartitionsProps> = observer(({topic}) => {
const paginationParams = usePaginationParams(uiState.topicSettings.partitionPageSize);

let partitions = api.topicPartitions.get(topic.topicName);
if (partitions === undefined) return DefaultSkeleton;
if (partitions === null) partitions = []; // todo: show the error (if one was reported);
Expand All @@ -43,6 +41,8 @@ export const TopicPartitions: FC<TopicPartitionsProps> = observer(({topic}) => {
</Alert>
);

const paginationParams = usePaginationParams(uiState.topicSettings.partitionPageSize, partitions.length);

return (
<>
{warning}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/pages/topics/Topic.List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class TopicList extends PageComponent {
export default TopicList;

const TopicsTable: FC<{ topics: Topic[], onDelete: (record: Topic) => void }> = ({ topics, onDelete }) => {
const paginationParams = usePaginationParams(uiSettings.topicList.pageSize)
const paginationParams = usePaginationParams(uiSettings.topicList.pageSize, topics.length)

return (
<DataTable<Topic>
Expand Down
22 changes: 17 additions & 5 deletions frontend/src/hooks/usePaginationParams.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,34 @@ jest.mock('react-router-dom', () => ({
describe('usePaginationParams', () => {
it('returns default values when URL parameters are absent', () => {
(useLocation as jest.Mock).mockReturnValue({ search: '' });
const { result } = renderHook(() => usePaginationParams());
// Assuming a total data length of 100 for testing
const totalDataLength = 100;
const { result } = renderHook(() => usePaginationParams(10, totalDataLength));
expect(result.current.pageSize).toBe(10);
expect(result.current.pageIndex).toBe(0);
});

it('parses pageSize and pageIndex from URL parameters', () => {
(useLocation as jest.Mock).mockReturnValue({ search: '?pageSize=5&page=2' });
const { result } = renderHook(() => usePaginationParams());
expect(result.current.pageSize).toBe(5);
(useLocation as jest.Mock).mockReturnValue({ search: '?pageSize=20&page=2' });
const totalDataLength = 100;
const { result } = renderHook(() => usePaginationParams(10, totalDataLength));
expect(result.current.pageSize).toBe(20);
expect(result.current.pageIndex).toBe(2);
});

it('uses defaultPageSize when pageSize is not in URL', () => {
(useLocation as jest.Mock).mockReturnValue({ search: '?page=3' });
const { result } = renderHook(() => usePaginationParams(15));
const totalDataLength = 150;
const { result } = renderHook(() => usePaginationParams(15, totalDataLength));
expect(result.current.pageSize).toBe(15);
expect(result.current.pageIndex).toBe(3);
});

it('adjusts pageIndex if it exceeds calculated totalPages', () => {
(useLocation as jest.Mock).mockReturnValue({ search: '?pageSize=10&page=20' }); // pageIndex is out of range
const totalDataLength = 50; // Only 5 pages available
const { result } = renderHook(() => usePaginationParams(10, totalDataLength));
expect(result.current.pageSize).toBe(10);
expect(result.current.pageIndex).toBe(4);
});
});
17 changes: 12 additions & 5 deletions frontend/src/hooks/usePaginationParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,29 @@ import { useLocation } from 'react-router-dom';
* 'pageIndex' defaults to 0 if not present in the URL.
*
* @param {number} [defaultPageSize=10] - The default number of items per page if not specified in the URL.
* @param {number} totalDataLength - The total length of the data to paginate over.
* @returns {{ pageSize: number; pageIndex: number }} An object containing the pageSize and pageIndex.
*
* @example
* // In a component using react-router
* const { pageSize, pageIndex } = usePaginationParams(20);
*/
const usePaginationParams = (defaultPageSize: number = 10): { pageSize: number; pageIndex: number } => {
const usePaginationParams = (defaultPageSize: number = 10, totalDataLength: number): { pageSize: number; pageIndex: number } => {
const { search} = useLocation();

return useMemo(() => {
const searchParams = new URLSearchParams(search)
const searchParams = new URLSearchParams(search);
const pageSize = searchParams.has('pageSize') ? Number(searchParams.get('pageSize')) : defaultPageSize;
const pageIndex = searchParams.has('page') ? Number(searchParams.get('page')) : 0;
const totalPages = Math.ceil(totalDataLength / pageSize);

const boundedPageIndex = Math.max(0, Math.min(pageIndex, totalPages - 1));

return {
pageSize: searchParams.has('pageSize') ? Number(searchParams.get('pageSize')) : defaultPageSize,
pageIndex: searchParams.has('page') ? Number(searchParams.get('page')) : 0,
pageSize,
pageIndex: boundedPageIndex,
}
}, [search, defaultPageSize])
}, [search, defaultPageSize, totalDataLength])
};

export default usePaginationParams;
Loading