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

Paging improvement #13943

Merged
merged 7 commits into from
Nov 10, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Users flow should work properly', () => {
it('Search for bot user', () => {
interceptURL(
'GET',
`/api/v1/search/query?q=*${searchBotText}***isBot:false&from=0&size=15&index=user_search_index`,
`/api/v1/search/query?q=*${searchBotText}***isBot:false&from=0&size=25&index=user_search_index`,
'searchUser'
);
cy.get('[data-testid="searchbar"]')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* limitations under the License.
*/

import { Col } from 'antd';
import { Col, Row, Switch, Typography } from 'antd';
import { ColumnsType } from 'antd/lib/table';
import { AxiosError } from 'axios';
import { isUndefined } from 'lodash';
Expand All @@ -23,6 +23,8 @@ import RichTextEditorPreviewer from '../../components/common/RichTextEditor/Rich
import Table from '../../components/common/Table/Table';
import {
getDataModelDetailsPath,
INITIAL_PAGING_VALUE,
PAGE_SIZE_BASE,
pagingObject,
} from '../../constants/constants';
import { Include } from '../../generated/type/include';
Expand All @@ -35,10 +37,11 @@ import { showErrorToast } from '../../utils/ToastUtils';
import NextPrevious from '../common/NextPrevious/NextPrevious';
import { NextPreviousProps } from '../common/NextPrevious/NextPrevious.interface';

const DataModelTable = ({ showDeleted }: { showDeleted?: boolean }) => {
const DataModelTable = () => {
const { t } = useTranslation();
const { fqn } = useParams<{ fqn: string }>();
const [dataModels, setDataModels] = useState<Array<ServicePageData>>();
const [showDeleted, setShowDeleted] = useState(false);
const {
currentPage,
pageSize,
Expand Down Expand Up @@ -94,7 +97,6 @@ const DataModelTable = ({ showDeleted }: { showDeleted?: boolean }) => {
setIsLoading(true);
const { data, paging: resPaging } = await getDataModels({
service: fqn,
fields: 'owner,tags,followers',
limit: pageSize,
include: showDeleted ? Include.Deleted : Include.NonDeleted,
...pagingData,
Expand Down Expand Up @@ -122,12 +124,32 @@ const DataModelTable = ({ showDeleted }: { showDeleted?: boolean }) => {
handlePageChange(currentPage);
};

const handleShowDeletedChange = (checked: boolean) => {
setShowDeleted(checked);
handlePageChange(INITIAL_PAGING_VALUE);
handlePageSizeChange(PAGE_SIZE_BASE);
};

useEffect(() => {
fetchDashboardsDataModel();
}, [pageSize, showDeleted]);

return (
<>
<Row gutter={[0, 16]}>
<Col className="p-t-sm p-x-lg" span={24}>
<Row justify="end">
<Col>
<Switch
checked={showDeleted}
data-testid="show-deleted"
onClick={handleShowDeletedChange}
/>
<Typography.Text className="m-l-xs">
{t('label.deleted')}
</Typography.Text>{' '}
</Col>
</Row>
</Col>
<Col className="p-x-lg" data-testid="table-container" span={24}>
<Table
bordered
Expand Down Expand Up @@ -155,7 +177,7 @@ const DataModelTable = ({ showDeleted }: { showDeleted?: boolean }) => {
/>
)}
</Col>
</>
</Row>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { AxiosError } from 'axios';
import QueryString from 'qs';
import React, { ReactNode, useEffect, useMemo, useState } from 'react';
import { useHistory, useLocation, useParams } from 'react-router-dom';
import { PAGE_SIZE } from '../../../constants/constants';
import { ERROR_PLACEHOLDER_TYPE } from '../../../enums/common.enum';
import { SearchIndex } from '../../../enums/search.enum';
import { TestCase } from '../../../generated/tests/testCase';
Expand Down Expand Up @@ -67,6 +66,7 @@ export const TestCases = ({ summaryPanel }: { summaryPanel: ReactNode }) => {
handlePageSizeChange,
paging,
handlePagingChange,
showPagination,
} = usePaging();

const handleSearchParam = (
Expand Down Expand Up @@ -127,7 +127,7 @@ export const TestCases = ({ summaryPanel }: { summaryPanel: ReactNode }) => {
try {
const response = await searchQuery({
pageNumber: page,
pageSize: PAGE_SIZE,
pageSize: pageSize,
searchIndex: SearchIndex.TEST_CASE,
query: searchValue,
fetchSource: false,
Expand All @@ -154,6 +154,7 @@ export const TestCases = ({ summaryPanel }: { summaryPanel: ReactNode }) => {
}, [] as TestCase[]);

setTestCase(testSuites);
handlePageChange(page);
handlePagingChange({ total: response.hits.total.value ?? 0 });
} catch (error) {
setTestCase([]);
Expand Down Expand Up @@ -232,6 +233,7 @@ export const TestCases = ({ summaryPanel }: { summaryPanel: ReactNode }) => {
afterDeleteAction={fetchTestCases}
isLoading={isLoading}
pagingData={pagingData}
showPagination={showPagination}
testCases={testCase}
onTestCaseResultUpdate={handleStatusSubmit}
onTestUpdate={handleTestCaseUpdate}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('TestCases component', () => {
expect(mockSearchQuery).toHaveBeenCalledWith({
fetchSource: false,
pageNumber: 1,
pageSize: 10,
pageSize: 15,
query: 'sale',
searchIndex: 'test_case_search_index',
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
/*
* Copyright 2023 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Col, Row, Switch, Typography } from 'antd';
import { AxiosError } from 'axios';
import { t } from 'i18next';
import { isEmpty } from 'lodash';
import QueryString from 'qs';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { useHistory, useLocation, useParams } from 'react-router-dom';
import {
INITIAL_PAGING_VALUE,
PAGE_SIZE,
} from '../../../../constants/constants';
import { SearchIndex } from '../../../../enums/search.enum';
import { DatabaseSchema } from '../../../../generated/entity/data/databaseSchema';
import { Include } from '../../../../generated/type/include';
import { Paging } from '../../../../generated/type/paging';
import { usePaging } from '../../../../hooks/paging/usePaging';
import { getDatabaseSchemas } from '../../../../rest/databaseAPI';
import { searchQuery } from '../../../../rest/searchAPI';
import { schemaTableColumns } from '../../../../utils/DatabaseDetails.utils';
import { showErrorToast } from '../../../../utils/ToastUtils';
import ErrorPlaceHolder from '../../../common/ErrorWithPlaceholder/ErrorPlaceHolder';
import NextPrevious from '../../../common/NextPrevious/NextPrevious';
import { PagingHandlerParams } from '../../../common/NextPrevious/NextPrevious.interface';
import Searchbar from '../../../common/SearchBarComponent/SearchBar.component';
import Table from '../../../common/Table/Table';

export const DatabaseSchemaTable = () => {
const { fqn } = useParams<{ fqn: string }>();
const history = useHistory();
const location = useLocation();
const [schemas, setSchemas] = useState<DatabaseSchema[]>([]);
const [isLoading, setIsLoading] = useState(true);
const [showDeletedSchemas, setShowDeletedSchemas] = useState<boolean>(false);
const searchValue = useMemo(() => {
const param = location.search;
const searchData = QueryString.parse(
param.startsWith('?') ? param.substring(1) : param
);

return searchData.schema as string | undefined;
}, [location.search]);
const {
currentPage,
handlePageChange,
pageSize,
handlePageSizeChange,
paging,
handlePagingChange,
showPagination,
} = usePaging();

const fetchDatabaseSchema = useCallback(
Copy link
Contributor

@mgorsk1 mgorsk1 Nov 10, 2023

Choose a reason for hiding this comment

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

wouldn't it simplify this component if fetchDatabaseSchema and searchSchema are replaced by single function that uses same query but query_text is:

  • * when search box is empty and
  • (name.keyword:*${searchValue}*) OR (description.keyword:*${searchValue}*) when search box is not empty

it would:

  • centralize the way data is fetched (and from where it's fetched) and follow DRY approach
  • keep just one logic of pagination

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @mgorsk1 Thanks for the comment and suggestion. We were following listing from DB calls and searching through ES at all the places. I will certainly take your feedback and see if we can club them in near future.
cc: @harshach

async (params?: Partial<Paging>) => {
if (isEmpty(fqn)) {
return;
}

try {
setIsLoading(true);
const { data, paging } = await getDatabaseSchemas({
databaseName: fqn,
limit: pageSize,
after: params?.after,
before: params?.before,
include: showDeletedSchemas ? Include.Deleted : Include.NonDeleted,
fields: ['owner', 'usageSummary'],
});

setSchemas(data);
handlePagingChange(paging);
} catch (error) {
showErrorToast(error);
} finally {
setIsLoading(false);
}
},
[pageSize, fqn, showDeletedSchemas]
);

const searchSchema = async (
searchValue: string,
pageNumber = INITIAL_PAGING_VALUE
) => {
setIsLoading(true);
try {
const response = await searchQuery({
query: `(name.keyword:*${searchValue}*) OR (description.keyword:*${searchValue}*)`,
pageNumber,
pageSize: PAGE_SIZE,
queryFilter: {
query: {
bool: {
must: [{ term: { 'database.fullyQualifiedName': fqn } }],
},
},
},
searchIndex: SearchIndex.DATABASE_SCHEMA,
includeDeleted: showDeletedSchemas,
trackTotalHits: true,
});
const data = response.hits.hits.map((schema) => schema._source);
const total = response.hits.total.value;
setSchemas(data);
handlePagingChange({ total });
} catch (error) {
showErrorToast(error as AxiosError);
} finally {
setIsLoading(false);
}
};

const handleShowDeletedSchemas = useCallback((value: boolean) => {
setShowDeletedSchemas(value);
handlePageChange(INITIAL_PAGING_VALUE);
}, []);

const handleSchemaPageChange = useCallback(
({ currentPage, cursorType }: PagingHandlerParams) => {
if (cursorType) {
fetchDatabaseSchema({ [cursorType]: paging[cursorType] });
}
handlePageChange(currentPage);
},
[paging, fetchDatabaseSchema]
);

const onSchemaSearch = (value: string) => {
history.push({
search: QueryString.stringify({
schema: isEmpty(value) ? undefined : value,
}),
});
if (value) {
searchSchema(value);
} else {
fetchDatabaseSchema();
}
};

useEffect(() => {
fetchDatabaseSchema();
}, [fqn, pageSize, showDeletedSchemas]);

return (
<Row gutter={[16, 16]}>
<Col span={12}>
<Searchbar
removeMargin
placeholder={t('label.search-for-type', {
type: t('label.schema'),
})}
searchValue={searchValue}
typingInterval={500}
onSearch={onSchemaSearch}
/>
</Col>
<Col className="flex items-center justify-end" span={12}>
<Switch
checked={showDeletedSchemas}
data-testid="show-deleted"
onClick={handleShowDeletedSchemas}
/>
<Typography.Text className="m-l-xs">
{t('label.deleted')}
</Typography.Text>{' '}
</Col>
<Col span={24}>
<Table
bordered
columns={schemaTableColumns}
data-testid="database-databaseSchemas"
dataSource={schemas}
loading={isLoading}
locale={{
emptyText: <ErrorPlaceHolder className="m-y-md" />,
}}
pagination={false}
rowKey="id"
size="small"
/>
</Col>
<Col span={24}>
{showPagination && (
<NextPrevious
currentPage={currentPage}
pageSize={pageSize}
paging={paging}
pagingHandler={handleSchemaPageChange}
onShowSizeChange={handlePageSizeChange}
/>
)}
</Col>
</Row>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const AddDataProductModal = ({

return (
<Modal
centered
cancelText={t('label.cancel')}
className="add-data-product-modal"
closable={false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ const AddDomainForm = ({
required: false,
placeholder: t('label.icon-url'),
type: FieldTypes.TEXT,
helperText: t('message.govern-url-size-message'),
props: {
'data-testid': 'icon-url',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const ExploreSearchCard: React.FC<ExploreSearchCardProps> = forwardRef<
if (source.style?.iconURL) {
return (
<img
className="align-middle m-r-xs"
className="align-middle m-r-xs object-contain"
data-testid="icon"
height={24}
src={source.style.iconURL}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ const GlossaryHeader = ({
if ((selectedData as GlossaryTerm).style?.iconURL) {
return (
<img
className="align-middle"
className="align-middle object-contain"
data-testid="icon"
height={36}
src={(selectedData as GlossaryTerm).style?.iconURL}
Expand Down
Loading
Loading