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

Pagination button at DatabaseDetails page doesn't work #13925

Closed
dechoma opened this issue Nov 9, 2023 · 1 comment · Fixed by #13943
Closed

Pagination button at DatabaseDetails page doesn't work #13925

dechoma opened this issue Nov 9, 2023 · 1 comment · Fixed by #13943
Assignees

Comments

@dechoma
Copy link
Contributor

dechoma commented Nov 9, 2023

Affected module
UI

Describe the bug
Clicking on the pagination button, [Previous/Next], at the DatabaseDetails page does not trigger any action, so it doesn't allow the user to paginate through the database schemas list.

pagination_error.mov

To Reproduce

Ingest 20 database schemas under a single db, and try to paginate through them via the Previous/Next button from the UI

Expected behavior
Paginate button Previous/Next should allow the user to paginate through the database schema list.

Version:

  • OS: iOS
  • Python version:
  • OpenMetadata version: 1.2.0
  • OpenMetadata Ingestion package version: openmetadata-ingestion[docker]==1.2.0.1

Additional context
It seems that in the DatabaseDetailsPage.tsx file, this line: Boolean(searchSchema) is setting the isNumberBased flag in the NextPrevious component to true.

 const searchSchema = async (
    searchValue: string,
    pageNumber = INITIAL_PAGING_VALUE
  ) => {.....}

const databaseTable = useMemo(
    () =>
      getDatabaseSchemaTable(
        schemaData,
        schemaDataLoading,
        databaseSchemaPaging,
        currentPage,
        databaseSchemaPagingHandler,
        Boolean(searchSchema)
      ),
    [
      schemaData,
      schemaDataLoading,
      databaseSchemaPaging,
      currentPage,
      databaseSchemaPagingHandler,
      searchSchema,
    ]
  );
export const getDatabaseSchemaTable = (
  schemaData: DatabaseSchema[],
  schemaDataLoading: boolean,
  databaseSchemaPaging: Paging,
  currentPage: number,
  databaseSchemaPagingHandler: NextPreviousProps['pagingHandler'],
  isNumberBased = false
) => {
  return (
    <Col span={24}>
      <Space className="w-full m-b-md" direction="vertical" size="middle">
        <Table
          bordered
          columns={schemaTableColumns}
          data-testid="database-databaseSchemas"
          dataSource={schemaData}
          loading={schemaDataLoading}
          locale={{
            emptyText: <ErrorPlaceHolder className="m-y-md" />,
          }}
          pagination={false}
          rowKey="id"
          size="small"
        />
        {databaseSchemaPaging.total > PAGE_SIZE && (
          <NextPrevious
            currentPage={currentPage}
            isNumberBased={isNumberBased}
            pageSize={PAGE_SIZE}
            paging={databaseSchemaPaging}
            pagingHandler={databaseSchemaPagingHandler}
          />
        )}
      </Space>
    </Col>

When the isNumberBased flag is set to true in the NextPrevious pagination component, then cursorType is not passed to pagingHandler.

  const onNextHandler = () => {
    if (isNumberBased) {
      pagingHandler({ currentPage: currentPage + 1 });
    } else {
      pagingHandler({
        cursorType: CursorType.AFTER,
        currentPage: currentPage + 1,
      });
    }
  };

When no cursorType type object is passed to databaseSchemaPagingHandler, the databaseSchemaPagingHandler logic is disabled.

  const databaseSchemaPagingHandler = ({
    cursorType,
    currentPage,
  }: PagingHandlerParams) => {
    if (cursorType) {
      if (isString(cursorType)) {
        const pagingString = `&${cursorType}=${
          databaseSchemaPaging[cursorType as keyof typeof databaseSchemaPaging]
        }`;
        setSchemaDataLoading(true);
        fetchDatabaseSchemas(pagingString).finally(() => {
          setSchemaDataLoading(false);
        });
        setCurrentPage(currentPage);
      } else {
        setCurrentPage(cursorType);
        searchValue && searchSchema(searchValue, cursorType);
      }
    }
  };
@harshach
Copy link
Collaborator

harshach commented Nov 9, 2023

Thanks @dechoma for filing the issue.
@chirag-madlani @karanh37 can we take a look for 1.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants