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

[wip] feat: Add pagination to the table selector when adding a dataset #20525

Closed
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
7 changes: 5 additions & 2 deletions superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -681,15 +681,18 @@ const Select = (
}
}, [isLoading, loading]);

const clearCache = () => fetchedQueries.current.clear();
const clearCache = useCallback(() => {
fetchedQueries.current.clear();
setSelectOptions(initialOptions);
}, [initialOptions]);

useImperativeHandle(
ref,
() => ({
...(ref.current as HTMLInputElement),
clearCache,
}),
[ref],
[ref, clearCache],
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const getTableMockFunction = async () =>
{ label: 'table_c', value: 'table_c' },
{ label: 'table_d', value: 'table_d' },
],
tableLength: 4,
},
} as any);

Expand Down
91 changes: 55 additions & 36 deletions superset-frontend/src/components/TableSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import React, {
FunctionComponent,
useState,
ReactNode,
useMemo,
useEffect,
useCallback,
useRef,
} from 'react';
import { SelectValue } from 'antd/lib/select';

Expand All @@ -36,6 +37,7 @@ import RefreshLabel from 'src/components/RefreshLabel';
import CertifiedBadge from 'src/components/CertifiedBadge';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { SelectRef } from '../Select/Select';

const TableSelectorWrapper = styled.div`
${({ theme }) => `
Expand Down Expand Up @@ -178,8 +180,8 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
>(undefined);
const [refresh, setRefresh] = useState(0);
const [previousRefresh, setPreviousRefresh] = useState(0);
const [loadingTables, setLoadingTables] = useState(false);
const { addSuccessToast } = useToasts();
const tableSelectRef = useRef<SelectRef>(null);

useEffect(() => {
// reset selections
Expand All @@ -204,22 +206,30 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
}
}, [tableOptions, tableValue, tableSelectMode]);

useEffect(() => {
if (currentDatabase && currentSchema) {
setLoadingTables(true);
const encodedSchema = encodeURIComponent(currentSchema);
const forceRefresh = refresh !== previousRefresh;
// TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes.
const endpoint = encodeURI(
`/superset/tables/${currentDatabase.id}/${encodedSchema}/undefined/${forceRefresh}/`,
);
const fetchTables = useCallback(
async (search: string, page: number, pageSize: number) => {
if (currentDatabase && currentSchema) {
const encodedSchema = encodeURIComponent(currentSchema);
const forceRefresh = refresh !== previousRefresh;

let endpoint = encodeURI(
`/superset/tables/${currentDatabase.id}/${encodedSchema}/undefined/`,
);

if (tableSelectMode === 'single') {
endpoint = encodeURI(
`/superset/tables/${currentDatabase.id}/${encodedSchema}/${
search || 'undefined'
}/${forceRefresh}/false/${page}/${pageSize}`,
);
}

if (previousRefresh !== refresh) {
setPreviousRefresh(refresh);
}
if (forceRefresh) {
setPreviousRefresh(refresh);
}

SupersetClient.get({ endpoint })
.then(({ json }) => {
try {
const { json } = await SupersetClient.get({ endpoint });
const options: TableOption[] = json.options.map((table: Table) => {
const option: TableOption = {
value: table.value,
Expand All @@ -232,18 +242,30 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({

onTablesLoad?.(json.options);
setTableOptions(options);
setLoadingTables(false);
if (forceRefresh) addSuccessToast('List updated');
})
.catch(() => {
setLoadingTables(false);

if (forceRefresh) {
addSuccessToast('List updated');
}

return {
data: options,
totalCount: json.tableLength,
};
} catch {
handleError(t('There was an error loading the tables'));
});
}
}
}

return {
data: [],
totalCount: 0,
};
},
// We are using the refresh state to re-trigger the query
// previousRefresh should be out of dependencies array
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [currentDatabase, currentSchema, onTablesLoad, setTableOptions, refresh]);
[currentDatabase, currentSchema, onTablesLoad, setTableOptions, refresh],
);

function renderSelectRow(select: ReactNode, refreshBtn: ReactNode) {
return (
Expand All @@ -269,14 +291,21 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
}
};

const clearTableSelection = () => {
tableSelectRef?.current?.clearCache();
internalTableChange(tableSelectMode === 'multiple' ? [] : undefined);
};

const internalDbChange = (db: DatabaseObject) => {
clearTableSelection();
setCurrentDatabase(db);
if (onDbChange) {
onDbChange(db);
}
};

const internalSchemaChange = (schema?: string) => {
clearTableSelection();
setCurrentSchema(schema);
if (onSchemaChange) {
onSchemaChange(schema);
Expand Down Expand Up @@ -306,15 +335,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
);
}

const handleFilterOption = useMemo(
() => (search: string, option: TableOption) => {
const searchValue = search.trim().toLowerCase();
const { text } = option;
return text.toLowerCase().includes(searchValue);
},
[],
);

function renderTableSelect() {
const disabled =
(currentSchema && !formMode && readOnly) ||
Expand All @@ -328,18 +348,17 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({

const select = (
<Select
ref={tableSelectRef}
ariaLabel={t('Select table or type table name')}
disabled={disabled}
filterOption={handleFilterOption}
header={header}
labelInValue
lazyLoading={false}
loading={loadingTables}
name="select-table"
onChange={(options: TableOption | TableOption[]) =>
internalTableChange(options)
}
options={tableOptions}
options={fetchTables}
placeholder={t('Select table or type table name')}
showSearch
mode={tableSelectMode}
Expand Down
20 changes: 9 additions & 11 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
)
query_context_generation_description = (
"The query context generation represents whether the query_context"
"is user generated or not so that it does not update user modfied"
"is user generated or not so that it does not update user modified"
"state."
)
cache_timeout_description = (
Expand All @@ -100,12 +100,12 @@
)
datasource_id_description = (
"The id of the dataset/datasource this new chart will use. "
"A complete datasource identification needs `datasouce_id` "
"A complete datasource identification needs `datasource_id` "
"and `datasource_type`."
)
datasource_uid_description = (
"The uid of the dataset/datasource this new chart will use. "
"A complete datasource identification needs `datasouce_uid` "
"A complete datasource identification needs `datasource_uid` "
)
datasource_type_description = (
"The type of dataset/datasource identified on `datasource_id`."
Expand All @@ -117,7 +117,7 @@
form_data_description = (
"Form data from the Explore controls used to form the chart's data query."
)
description_markeddown_description = "Sanitized HTML version of the chart description."
description_markdown_description = "Sanitized HTML version of the chart description."
owners_name_description = "Name of an owner of the chart."
certified_by_description = "Person or group that has certified this chart"
certification_details_description = "Details of the certification"
Expand Down Expand Up @@ -158,9 +158,7 @@ class ChartEntityResponseSchema(Schema):
cache_timeout = fields.Integer(description=cache_timeout_description)
changed_on = fields.String(description=changed_on_description)
description = fields.String(description=description_description)
description_markeddown = fields.String(
description=description_markeddown_description
)
description_markeddown = fields.String(description=description_markdown_description)
form_data = fields.Dict(description=form_data_description)
slice_url = fields.String(description=slice_url_description)
certified_by = fields.String(description=certified_by_description)
Expand Down Expand Up @@ -911,7 +909,7 @@ class AnnotationLayerSchema(Schema):
)
overrides = fields.Dict(
keys=fields.String(
desciption="Name of property to be overridden",
description="Name of property to be overridden",
validate=validate.OneOf(
choices=("granularity", "time_grain_sqla", "time_range", "time_shift"),
),
Expand Down Expand Up @@ -1291,7 +1289,7 @@ class ChartDataResponseResult(Schema):
allow_none=False,
)
stacktrace = fields.String(
desciption="Stacktrace if there was an error",
description="Stacktrace if there was an error",
allow_none=True,
)
rowcount = fields.Integer(
Expand All @@ -1310,10 +1308,10 @@ class ChartDataResponseResult(Schema):
fields.Dict(), description="A list with rejected filters"
)
from_dttm = fields.Integer(
desciption="Start timestamp of time range", required=False, allow_none=True
description="Start timestamp of time range", required=False, allow_none=True
)
to_dttm = fields.Integer(
desciption="End timestamp of time range", required=False, allow_none=True
description="End timestamp of time range", required=False, allow_none=True
)


Expand Down
13 changes: 11 additions & 2 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1112,14 +1112,19 @@ def save_or_overwrite_slice(
@event_logger.log_this
@expose("/tables/<int:db_id>/<schema>/<substr>/")
@expose("/tables/<int:db_id>/<schema>/<substr>/<force_refresh>/")
@expose("/tables/<int:db_id>/<schema>/<substr>/<force_refresh>/<exact_match>")
@expose("/tables/<int:db_id>/<schema>/<substr>/<force_refresh>/<exact_match>/")
@expose(
"/tables/<int:db_id>/<schema>/<substr>/<force_refresh>/<exact_match>/<int:page>/<int:page_size>"
)
def tables( # pylint: disable=too-many-locals,no-self-use,too-many-arguments
self,
db_id: int,
schema: str,
substr: str,
force_refresh: str = "false",
exact_match: str = "false",
page: Optional[int] = None,
page_size: Optional[int] = None,
) -> FlaskResponse:
"""Endpoint to fetch the list of tables for given database"""
# Guarantees database filtering by security access
Expand Down Expand Up @@ -1234,8 +1239,12 @@ def is_match(src: str, target: utils.DatasourceName) -> bool:
for vn in views[:max_views]
]
)

table_options.sort(key=lambda value: value["label"])
payload = {"tableLength": len(tables) + len(views), "options": table_options}
if page and page_size:
table_options = table_options[(page_size * (page - 1)) : (page_size * page)]

payload = {"tableLength": len(table_options), "options": table_options}
return json_success(json.dumps(payload))

@api
Expand Down
24 changes: 24 additions & 0 deletions tests/integration_tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,30 @@ def test_get_superset_tables_substr(self):
}
self.assertEqual(response, expected_response)

def test_get_superset_tables_with_pagination(self):
example_db = superset.utils.database.get_example_database()
schema_name = self.default_schema_backend_map[example_db.backend]
self.login(username="admin")
uri = f"superset/tables/{example_db.id}/{schema_name}/undefined/false/false/1/3"
rv = self.client.get(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 200)

expected_response = {
"options": [
{
"label": "ab_role",
"schema": schema_name,
"title": "ab_role",
"type": "table",
"value": "ab_role",
"extra": None,
}
],
"tableLength": 3,
}
self.assertEqual(response, expected_response)

def test_get_superset_tables_not_found(self):
self.login(username="admin")
uri = f"superset/tables/invalid/public/undefined/"
Expand Down