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

chore: Shows the dataset description in the gallery dropdown #16200

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
22 changes: 7 additions & 15 deletions superset-frontend/src/addSlice/AddSliceContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ import VizTypeGallery from 'src/explore/components/controls/VizTypeControl/VizTy
import { styledMount as mount } from 'spec/helpers/theming';
import { act } from 'spec/helpers/testing-library';

const defaultProps = {
datasources: [
{ label: 'my first table', value: '1__table' },
{ label: 'another great table', value: '2__table' },
],
const datasource = {
value: '1',
label: 'table',
};

describe('AddSliceContainer', () => {
Expand All @@ -43,7 +41,7 @@ describe('AddSliceContainer', () => {
>;

beforeEach(async () => {
wrapper = mount(<AddSliceContainer {...defaultProps} />) as ReactWrapper<
wrapper = mount(<AddSliceContainer />) as ReactWrapper<
AddSliceContainerProps,
AddSliceContainerState,
AddSliceContainer
Expand All @@ -68,11 +66,8 @@ describe('AddSliceContainer', () => {
});

it('renders an enabled button if datasource and viz type is selected', () => {
const datasourceValue = defaultProps.datasources[0].value;
wrapper.setState({
datasourceValue,
datasourceId: datasourceValue.split('__')[0],
datasourceType: datasourceValue.split('__')[1],
datasource,
visType: 'table',
});
expect(
Expand All @@ -81,15 +76,12 @@ describe('AddSliceContainer', () => {
});

it('formats explore url', () => {
const datasourceValue = defaultProps.datasources[0].value;
wrapper.setState({
datasourceValue,
datasourceId: datasourceValue.split('__')[0],
datasourceType: datasourceValue.split('__')[1],
datasource,
visType: 'table',
});
const formattedUrl =
'/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%2C%22datasource%22%3A%221__table%22%7D';
'/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%2C%22datasource%22%3A%221%22%7D';
expect(wrapper.instance().exploreUrl()).toBe(formattedUrl);
});
});
143 changes: 123 additions & 20 deletions superset-frontend/src/addSlice/AddSliceContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,34 @@
* under the License.
*/
import React from 'react';
import rison from 'rison';
import Button from 'src/components/Button';
import { Select } from 'src/components';
import { css, styled, t } from '@superset-ui/core';
import {
css,
styled,
t,
SupersetClient,
JsonResponse,
} from '@superset-ui/core';
import { FormLabel } from 'src/components/Form';
import { Tooltip } from 'src/components/Tooltip';

import VizTypeGallery, {
MAX_ADVISABLE_VIZ_GALLERY_WIDTH,
} from 'src/explore/components/controls/VizTypeControl/VizTypeGallery';

interface Datasource {
label: string;
value: string;
}

export type AddSliceContainerProps = {
datasources: Datasource[];
type Dataset = {
id: number;
table_name: string;
description: string;
datasource_type: string;
};

export type AddSliceContainerProps = {};

export type AddSliceContainerState = {
datasourceId?: string;
datasourceType?: string;
datasourceValue?: string;
datasource?: { label: string; value: string };
visType: string | null;
};

Expand Down Expand Up @@ -81,6 +87,42 @@ const StyledContainer = styled.div`
margin-top: ${theme.gridUnit * 6}px;
}
}

& .ant-tooltip-open {
display: inline;
}

&&&& .ant-select-selector {
padding: 0;
}

&&&& .ant-select-selection-placeholder {
padding-left: ${theme.gridUnit * 3}px;
}
`}
`;

const TooltipContent = styled.div<{ hasDescription: boolean }>`
${({ theme, hasDescription }) => `
.tooltip-header {
font-size: ${
hasDescription ? theme.typography.sizes.l : theme.typography.sizes.s
}px;
font-weight: ${
hasDescription
? theme.typography.weights.bold
: theme.typography.weights.normal
};
}

.tooltip-description {
margin-top: ${theme.gridUnit * 2}px;
display: -webkit-box;
-webkit-line-clamp: 20;
Copy link
Member

Choose a reason for hiding this comment

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

Ooh, you don't get to use that every day.

-webkit-box-orient: vertical;
overflow: hidden;
text-overflow: ellipsis;
}
`}
`;

Expand All @@ -97,6 +139,16 @@ const StyledVizTypeGallery = styled(VizTypeGallery)`
`}
`;

const StyledLabel = styled.span`
${({ theme }) => `
position: absolute;
left: ${theme.gridUnit * 3}px;
right: ${theme.gridUnit * 3}px;
overflow: hidden;
text-overflow: ellipsis;
`}
`;

export default class AddSliceContainer extends React.PureComponent<
AddSliceContainerProps,
AddSliceContainerState
Expand All @@ -110,13 +162,16 @@ export default class AddSliceContainer extends React.PureComponent<
this.changeDatasource = this.changeDatasource.bind(this);
this.changeVisType = this.changeVisType.bind(this);
this.gotoSlice = this.gotoSlice.bind(this);
this.newLabel = this.newLabel.bind(this);
this.loadDatasources = this.loadDatasources.bind(this);
this.handleFilterOption = this.handleFilterOption.bind(this);
}

exploreUrl() {
const formData = encodeURIComponent(
JSON.stringify({
viz_type: this.state.visType,
datasource: this.state.datasourceValue,
datasource: this.state.datasource?.value,
}),
);
return `/superset/explore/?form_data=${formData}`;
Expand All @@ -126,19 +181,66 @@ export default class AddSliceContainer extends React.PureComponent<
window.location.href = this.exploreUrl();
}

changeDatasource(value: string) {
this.setState({
datasourceValue: value,
datasourceId: value.split('__')[0],
});
changeDatasource(datasource: { label: string; value: string }) {
this.setState({ datasource });
}

changeVisType(visType: string | null) {
this.setState({ visType });
}

isBtnDisabled() {
return !(this.state.datasourceId && this.state.visType);
return !(this.state.datasource?.value && this.state.visType);
}

newLabel(item: Dataset) {
return (
<Tooltip
mouseEnterDelay={1}
placement="right"
title={
<TooltipContent hasDescription={!!item.description}>
<div className="tooltip-header">{item.table_name}</div>
{item.description && (
<div className="tooltip-description">{item.description}</div>
)}
</TooltipContent>
}
>
<StyledLabel>{item.table_name}</StyledLabel>
</Tooltip>
);
}

loadDatasources(search: string, page: number, pageSize: number) {
const query = rison.encode({
columns: ['id', 'table_name', 'description', 'datasource_type'],
filter: search,
page,
page_size: pageSize,
});
return SupersetClient.get({
endpoint: `/api/v1/dataset?q=${query}`,
}).then((response: JsonResponse) => {
const list = response.json.result.map((item: Dataset) => ({
value: `${item.id}__${item.datasource_type}`,
label: this.newLabel(item),
labelText: item.table_name,
}));
return {
data: list,
totalCount: response.json.count,
};
});
}

handleFilterOption(
search: string,
option: { label: string; value: number; labelText: string },
) {
const searchValue = search.trim().toLowerCase();
const { labelText } = option;
return labelText.toLowerCase().includes(searchValue);
}

render() {
Expand All @@ -151,11 +253,12 @@ export default class AddSliceContainer extends React.PureComponent<
ariaLabel={t('Dataset')}
name="select-datasource"
header={<FormLabel required>{t('Choose a dataset')}</FormLabel>}
filterOption={this.handleFilterOption}
onChange={this.changeDatasource}
options={this.props.datasources}
options={this.loadDatasources}
placeholder={t('Choose a dataset')}
showSearch
value={this.state.datasourceValue}
value={this.state.datasource}
/>
<span>
{t(
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/addSlice/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ initFeatureFlags(bootstrapData.common.feature_flags);
const App = () => (
<ThemeProvider theme={theme}>
<DynamicPluginProvider>
<AddSliceContainer datasources={bootstrapData.datasources} />
<AddSliceContainer />
</DynamicPluginProvider>
</ThemeProvider>
);
Expand Down
2 changes: 2 additions & 0 deletions superset/datasets/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"changed_on_utc",
"changed_on_delta_humanized",
"default_endpoint",
"description",
"datasource_type",
"explore_url",
"extra",
"kind",
Expand Down
10 changes: 1 addition & 9 deletions superset/views/chart/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import lazy_gettext as _

from superset import is_feature_enabled, security_manager
from superset import is_feature_enabled
from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.models.slice import Slice
from superset.typing import FlaskResponse
Expand Down Expand Up @@ -62,15 +62,7 @@ def pre_delete(self, item: "SliceModelView") -> None:
@expose("/add", methods=["GET", "POST"])
@has_access
def add(self) -> FlaskResponse:
datasources = [
{"value": str(d.id) + "__" + d.type, "label": repr(d)}
for d in security_manager.get_user_datasources()
]
payload = {
"datasources": sorted(
datasources,
key=lambda d: d["label"].lower() if isinstance(d["label"], str) else "",
),
"common": common_bootstrap_payload(),
"user": bootstrap_user_data(g.user),
}
Expand Down
2 changes: 2 additions & 0 deletions tests/integration_tests/datasets/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ def test_get_dataset_list(self):
"changed_on_delta_humanized",
"changed_on_utc",
"database",
"datasource_type",
"default_endpoint",
"description",
"explore_url",
"extra",
"id",
Expand Down