Skip to content

Commit

Permalink
fix(sqllab): Sort db selector options by the API order (apache#28749)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored May 29, 2024
1 parent a67b0ed commit 453a645
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ const fakeDatabaseApiResult = {
],
};

const fakeDatabaseApiResultInReverseOrder = {
...fakeDatabaseApiResult,
ids: [2, 1],
result: [...fakeDatabaseApiResult.result].reverse(),
};

const fakeSchemaApiResult = {
count: 2,
result: ['information_schema', 'public'],
Expand All @@ -168,7 +174,8 @@ const fakeFunctionNamesApiResult = {
function_names: [],
};

const databaseApiRoute = 'glob:*/api/v1/database/?*';
const databaseApiRoute =
'glob:*/api/v1/database/?*order_column:database_name,order_direction*';
const catalogApiRoute = 'glob:*/api/v1/database/*/catalogs/?*';
const schemaApiRoute = 'glob:*/api/v1/database/*/schemas/?*';
const tablesApiRoute = 'glob:*/api/v1/database/*/tables/*';
Expand Down Expand Up @@ -241,6 +248,30 @@ test('Should database select display options', async () => {
expect(await screen.findByText('test-mysql')).toBeInTheDocument();
});

test('should display options in order of the api response', async () => {
fetchMock.get(databaseApiRoute, fakeDatabaseApiResultInReverseOrder, {
overwriteRoutes: true,
});
const props = createProps();
render(<DatabaseSelector {...props} db={undefined} />, {
useRedux: true,
store,
});
const select = screen.getByRole('combobox', {
name: 'Select database or type to search databases',
});
expect(select).toBeInTheDocument();
userEvent.click(select);
const options = await screen.findAllByRole('option');

expect(options[0]).toHaveTextContent(
`${fakeDatabaseApiResultInReverseOrder.result[0].id}`,
);
expect(options[1]).toHaveTextContent(
`${fakeDatabaseApiResultInReverseOrder.result[1].id}`,
);
});

test('Should fetch the search keyword when total count exceeds initial options', async () => {
fetchMock.get(
databaseApiRoute,
Expand Down
26 changes: 23 additions & 3 deletions superset-frontend/src/components/DatabaseSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { ReactNode, useState, useMemo, useEffect, useRef } from 'react';
import React, {
ReactNode,
useState,
useMemo,
useEffect,
useRef,
useCallback,
} from 'react';
import { styled, SupersetClient, t } from '@superset-ui/core';
import type { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import rison from 'rison';
import { AsyncSelect, Select } from 'src/components';
import Label from 'src/components/Label';
Expand Down Expand Up @@ -124,6 +132,10 @@ const SelectLabel = ({
const EMPTY_CATALOG_OPTIONS: CatalogOption[] = [];
const EMPTY_SCHEMA_OPTIONS: SchemaOption[] = [];

interface AntdLabeledValueWithOrder extends AntdLabeledValue {
order: number;
}

export default function DatabaseSelector({
db,
formMode = false,
Expand Down Expand Up @@ -153,6 +165,11 @@ export default function DatabaseSelector({
const schemaRef = useRef(schema);
schemaRef.current = schema;
const { addSuccessToast } = useToasts();
const sortComparator = useCallback(
(itemA: AntdLabeledValueWithOrder, itemB: AntdLabeledValueWithOrder) =>
itemA.order - itemB.order,
[],
);

const loadDatabases = useMemo(
() =>
Expand All @@ -165,7 +182,7 @@ export default function DatabaseSelector({
totalCount: number;
}> => {
const queryParams = rison.encode({
order_columns: 'database_name',
order_column: 'database_name',
order_direction: 'asc',
page,
page_size: pageSize,
Expand All @@ -191,7 +208,8 @@ export default function DatabaseSelector({
if (result.length === 0) {
if (onEmptyResults) onEmptyResults(search);
}
const options = result.map((row: DatabaseObject) => ({

const options = result.map((row: DatabaseObject, order: number) => ({
label: (
<SelectLabel
backend={row.backend}
Expand All @@ -203,6 +221,7 @@ export default function DatabaseSelector({
database_name: row.database_name,
backend: row.backend,
allow_multi_catalog: row.allow_multi_catalog,
order,
}));

return {
Expand Down Expand Up @@ -346,6 +365,7 @@ export default function DatabaseSelector({
placeholder={t('Select database or type to search databases')}
disabled={!isDatabaseSelectEnabled || readOnly}
options={loadDatabases}
sortComparator={sortComparator}
/>,
null,
);
Expand Down

0 comments on commit 453a645

Please sign in to comment.