pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Filter owners select by text input (#9337) 

* filter owners select by text input

* use rison

* fix backend owners filter logic

* use fullname, not username on owners inputs

* fix some tests

* fixing tests

* deterministic tests

* appease linter

* add back search by username

* more comprehensive filter test

* add clarifying text

* formatting...
This commit is contained in:
David Aaron Suddjian 2020-04-07 09:09:02 -07:00 committed by GitHub
parent 4be827544e
commit 5e535062da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 366 additions and 269 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

290
superset-frontend/package-lock.json generated
View File

File diff suppressed because it is too large Load Diff

4
superset-frontend/package.json
View File

@ -92,6 +92,8 @@
"@superset-ui/translation": "^0.12.8",
"@types/classnames": "^2.2.9",
"@types/react-json-tree": "^0.6.11",
"@types/react-select": "^1.2.1",
"@types/rison": "0.0.6",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"aphrodite": "^2.3.1",
@ -156,6 +158,7 @@
"redux-thunk": "^2.1.0",
"redux-undo": "^1.0.0-beta9-9-7",
"regenerator-runtime": "^0.13.3",
"rison": "^0.1.1",
"shortid": "^2.2.6",
"urijs": "^1.18.10",
"use-query-params": "^0.4.5"
@ -177,7 +180,6 @@
"@types/react": "^16.9.23",
"@types/react-dom": "^16.9.5",
"@types/react-redux": "^7.1.7",
"@types/react-select": "^3.0.10",
"@types/react-table": "^7.0.2",
"@types/react-ultimate-pagination": "^1.2.0",
"@types/yargs": "12 - 15",

130
superset-frontend/src/dashboard/components/PropertiesModal.jsx
View File

@ -20,8 +20,9 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Row, Col, Button, Modal, FormControl } from 'react-bootstrap';
import Dialog from 'react-bootstrap-dialog';
import Select from 'react-select';
import { Async as SelectAsync } from 'react-select';
import AceEditor from 'react-ace';
import rison from 'rison';
import { t } from '@superset-ui/translation';
import { SupersetClient } from '@superset-ui/connection';
import '../stylesheets/buttons.less';
@ -55,7 +56,6 @@ class PropertiesModal extends React.PureComponent {
json_metadata: '',
},
isDashboardLoaded: false,
ownerOptions: null,
isAdvancedOpen: false,
};
this.onChange = this.onChange.bind(this);
@ -63,10 +63,11 @@ class PropertiesModal extends React.PureComponent {
this.onOwnersChange = this.onOwnersChange.bind(this);
this.save = this.save.bind(this);
this.toggleAdvanced = this.toggleAdvanced.bind(this);
this.loadOwnerOptions = this.loadOwnerOptions.bind(this);
this.handleErrorResponse = this.handleErrorResponse.bind(this);
}
componentDidMount() {
this.fetchOwnerOptions();
this.fetchDashboardDetails();
}
@ -90,41 +91,42 @@ class PropertiesModal extends React.PureComponent {
// datamodel, the dashboard could probably just be passed as a prop.
SupersetClient.get({
endpoint: `/api/v1/dashboard/${this.props.dashboardId}`,
})
.then(response => {
const dashboard = response.json.result;
this.setState(state => ({
isDashboardLoaded: true,
values: {
...state.values,
dashboard_title: dashboard.dashboard_title || '',
slug: dashboard.slug || '',
json_metadata: dashboard.json_metadata || '',
},
}));
const initialSelectedValues = dashboard.owners.map(owner => ({
value: owner.id,
label: owner.username,
}));
this.onOwnersChange(initialSelectedValues);
})
.catch(err => console.error(err));
}).then(response => {
const dashboard = response.json.result;
this.setState(state => ({
isDashboardLoaded: true,
values: {
...state.values,
dashboard_title: dashboard.dashboard_title || '',
slug: dashboard.slug || '',
json_metadata: dashboard.json_metadata || '',
},
}));
const initialSelectedOwners = dashboard.owners.map(owner => ({
value: owner.id,
label: `${owner.first_name} ${owner.last_name}`,
}));
this.onOwnersChange(initialSelectedOwners);
}, this.handleErrorResponse);
}
fetchOwnerOptions() {
SupersetClient.get({
endpoint: `/api/v1/dashboard/related/owners`,
})
.then(response => {
loadOwnerOptions(input = '') {
const query = rison.encode({ filter: input });
return SupersetClient.get({
endpoint: `/api/v1/dashboard/related/owners?q=${query}`,
}).then(
response => {
const options = response.json.result.map(item => ({
value: item.value,
label: item.text,
}));
this.setState({
ownerOptions: options,
});
})
.catch(err => console.error(err));
return { options };
},
badResponse => {
this.handleErrorResponse(badResponse);
return { options: [] };
},
);
}
updateFormState(name, value) {
@ -142,6 +144,17 @@ class PropertiesModal extends React.PureComponent {
}));
}
async handleErrorResponse(response) {
const { error, statusText } = await getClientErrorObject(response);
this.dialog.show({
title: 'Error',
bsSize: 'medium',
bsStyle: 'danger',
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: error || statusText || t('An error has occurred'),
});
}
save(e) {
e.preventDefault();
e.stopPropagation();
@ -157,38 +170,21 @@ class PropertiesModal extends React.PureComponent {
json_metadata: values.json_metadata || null,
owners,
}),
})
.then(({ json }) => {
this.props.addSuccessToast(t('The dashboard has been saved'));
this.props.onDashboardSave({
id: this.props.dashboardId,
title: json.result.dashboard_title,
slug: json.result.slug,
jsonMetadata: json.result.json_metadata,
ownerIds: json.result.owners,
});
this.props.onHide();
})
.catch(response =>
getClientErrorObject(response).then(({ error, statusText }) => {
this.dialog.show({
title: 'Error',
bsSize: 'medium',
bsStyle: 'danger',
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: error || statusText || t('An error has occurred'),
});
}),
);
}).then(({ json }) => {
this.props.addSuccessToast(t('The dashboard has been saved'));
this.props.onDashboardSave({
id: this.props.dashboardId,
title: json.result.dashboard_title,
slug: json.result.slug,
jsonMetadata: json.result.json_metadata,
ownerIds: json.result.owners,
});
this.props.onHide();
}, this.handleErrorResponse);
}
render() {
const {
ownerOptions,
values,
isDashboardLoaded,
isAdvancedOpen,
} = this.state;
const { values, isDashboardLoaded, isAdvancedOpen } = this.state;
return (
<Modal show={this.props.show} onHide={this.props.onHide} bsSize="lg">
<form onSubmit={this.save}>
@ -242,17 +238,19 @@ class PropertiesModal extends React.PureComponent {
<label className="control-label" htmlFor="owners">
{t('Owners')}
</label>
<Select
<SelectAsync
name="owners"
multi
isLoading={!ownerOptions}
value={values.owners}
options={ownerOptions || []}
loadOptions={this.loadOwnerOptions}
onChange={this.onOwnersChange}
disabled={!ownerOptions || !isDashboardLoaded}
disabled={!isDashboardLoaded}
filterOption={() => true} // options are filtered at the api
/>
<p className="help-block">
{t('Owners is a list of users who can alter the dashboard.')}
{t(
'Owners is a list of users who can alter the dashboard. Searchable by name or username.',
)}
</p>
</Col>
</Row>

63
superset-frontend/src/explore/components/PropertiesModal.tsx
View File

@ -28,7 +28,8 @@ import {
} from 'react-bootstrap';
// @ts-ignore
import Dialog from 'react-bootstrap-dialog';
import Select from 'react-select';
import { Async as SelectAsync, Option } from 'react-select';
import rison from 'rison';
import { t } from '@superset-ui/translation';
import { SupersetClient, Json } from '@superset-ui/connection';
import Chart from 'src/types/Chart';
@ -70,7 +71,6 @@ export default function PropertiesModalWrapper({
function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
const [submitting, setSubmitting] = useState(false);
const errorDialog = useRef<any>(null);
const [ownerOptions, setOwnerOptions] = useState(null);
// values of form inputs
const [name, setName] = useState(slice.slice_name || '');
@ -78,7 +78,7 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
const [cacheTimeout, setCacheTimeout] = useState(
slice.cache_timeout != null ? slice.cache_timeout : '',
);
const [owners, setOwners] = useState<any[] | null>(null);
const [owners, setOwners] = useState<Option[] | null>(null);
function showError({ error, statusText }: any) {
errorDialog.current.show({
@ -90,7 +90,7 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
});
}
async function fetchOwners() {
async function fetchChartData() {
try {
const response = await SupersetClient.get({
endpoint: `/api/v1/chart/${slice.slice_id}`,
@ -99,7 +99,7 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
setOwners(
chart.owners.map((owner: any) => ({
value: owner.id,
label: owner.username,
label: `${owner.first_name} ${owner.last_name}`,
})),
);
} catch (response) {
@ -110,34 +110,41 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
// get the owners of this slice
useEffect(() => {
fetchOwners();
fetchChartData();
}, []);
// get the list of users who can own a chart
useEffect(() => {
SupersetClient.get({
endpoint: `/api/v1/chart/related/owners`,
}).then(res => {
const { result } = res.json as Json;
setOwnerOptions(
result.map((item: any) => ({
const loadOptions = (input = '') => {
const query = rison.encode({ filter: input });
return SupersetClient.get({
endpoint: `/api/v1/chart/related/owners?q=${query}`,
}).then(
response => {
const { result } = response.json as Json;
const options = result.map((item: any) => ({
value: item.value,
label: item.text,
})),
);
});
}, []);
}));
return { options };
},
badResponse => {
getClientErrorObject(badResponse).then(showError);
return { options: [] };
},
);
};
const onSubmit = async (event: React.FormEvent) => {
event.stopPropagation();
event.preventDefault();
setSubmitting(true);
const payload = {
const payload: { [key: string]: any } = {
slice_name: name || null,
description: description || null,
cache_timeout: cacheTimeout || null,
owners: owners!.map(o => o.value),
};
if (owners) {
payload.owners = owners.map(o => o.value);
}
try {
const res = await SupersetClient.put({
endpoint: `/api/v1/chart/${slice.slice_id}`,
@ -229,17 +236,19 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
<label className="control-label" htmlFor="owners">
{t('Owners')}
</label>
<Select
name="owners"
<SelectAsync
multi
isLoading={!ownerOptions}
value={owners}
options={ownerOptions || []}
name="owners"
value={owners || []}
loadOptions={loadOptions}
onChange={setOwners}
disabled={!owners || !ownerOptions}
disabled={!owners}
filterOption={() => true} // options are filtered at the api
/>
<p className="help-block">
{t('A list of users who can alter the chart')}
{t(
'A list of users who can alter the chart. Searchable by name or username.',
)}
</p>
</FormGroup>
</Col>

9
superset/charts/api.py
View File

@ -43,7 +43,8 @@ from superset.charts.schemas import (
)
from superset.constants import RouteMethod
from superset.models.slice import Slice
from superset.views.base_api import BaseSupersetModelRestApi
from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter
from superset.views.filters import FilterRelatedOwners
logger = logging.getLogger(__name__)
@ -65,6 +66,8 @@ class ChartRestApi(BaseSupersetModelRestApi):
"description",
"owners.id",
"owners.username",
"owners.first_name",
"owners.last_name",
"dashboards.id",
"dashboards.dashboard_title",
"viz_type",
@ -116,7 +119,9 @@ class ChartRestApi(BaseSupersetModelRestApi):
"slices": ("slice_name", "asc"),
"owners": ("first_name", "asc"),
}
filter_rel_fields_field = {"owners": "first_name"}
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners)
}
allowed_rel_fields = {"owners"}
@expose("/", methods=["POST"])

9
superset/dashboards/api.py
View File

@ -45,7 +45,8 @@ from superset.dashboards.schemas import (
)
from superset.models.dashboard import Dashboard
from superset.views.base import generate_download_headers
from superset.views.base_api import BaseSupersetModelRestApi
from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter
from superset.views.filters import FilterRelatedOwners
logger = logging.getLogger(__name__)
@ -69,6 +70,8 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"json_metadata",
"owners.id",
"owners.username",
"owners.first_name",
"owners.last_name",
"changed_by_name",
"changed_by_url",
"changed_by.username",
@ -114,7 +117,9 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"slices": ("slice_name", "asc"),
"owners": ("first_name", "asc"),
}
filter_rel_fields_field = {"owners": "first_name"}
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners)
}
allowed_rel_fields = {"owners"}
@expose("/", methods=["POST"])

11
superset/datasets/api.py
View File

@ -43,8 +43,9 @@ from superset.datasets.schemas import (
get_export_ids_schema,
)
from superset.views.base import DatasourceFilter, generate_download_headers
from superset.views.base_api import BaseSupersetModelRestApi
from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter
from superset.views.database.filters import DatabaseFilter
from superset.views.filters import FilterRelatedOwners
logger = logging.getLogger(__name__)
@ -91,6 +92,8 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"template_params",
"owners.id",
"owners.username",
"owners.first_name",
"owners.last_name",
"columns",
"metrics",
]
@ -115,8 +118,10 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"metrics",
]
openapi_spec_tag = "Datasets"
filter_rel_fields_field = {"owners": "first_name", "database": "database_name"}
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners),
"database": "database_name",
}
filter_rel_fields = {"database": [["id", DatabaseFilter, lambda: []]]}
allowed_rel_fields = {"database", "owners"}

35
superset/views/base_api.py
View File

@ -16,12 +16,13 @@
# under the License.
import functools
import logging
from typing import Dict, Set, Tuple
from typing import cast, Dict, Set, Tuple, Type, Union
from flask import request
from flask_appbuilder import ModelRestApi
from flask_appbuilder.api import expose, protect, rison, safe
from flask_appbuilder.models.filters import BaseFilter, Filters
from flask_appbuilder.models.sqla.filters import FilterStartsWith
from sqlalchemy.exc import SQLAlchemyError
from superset.exceptions import SupersetSecurityException
@ -58,6 +59,14 @@ def check_ownership_and_item_exists(f):
return functools.update_wrapper(wraps, f)
class RelatedFieldFilter:
# data class to specify what filter to use on a /related endpoint
# pylint: disable=too-few-public-methods
def __init__(self, field_name: str, filter_class: Type[BaseFilter]):
self.field_name = field_name
self.filter_class = filter_class
class BaseSupersetModelRestApi(ModelRestApi):
"""
Extends FAB's ModelResApi to implement specific superset generic functionality
@ -86,12 +95,12 @@ class BaseSupersetModelRestApi(ModelRestApi):
...
}
""" # pylint: disable=pointless-string-statement
filter_rel_fields_field: Dict[str, str] = {}
related_field_filters: Dict[str, Union[RelatedFieldFilter, str]] = {}
"""
Declare the related field field for filtering::
Declare the filters for related fields::
filter_rel_fields_field = {
"<RELATED_FIELD>": "<RELATED_FIELD_FIELD>")
related_fields = {
"<RELATED_FIELD>": <RelatedFieldFilter>)
}
""" # pylint: disable=pointless-string-statement
filter_rel_fields: Dict[str, BaseFilter] = {}
@ -125,14 +134,18 @@ class BaseSupersetModelRestApi(ModelRestApi):
super()._init_properties()
def _get_related_filter(self, datamodel, column_name: str, value: str) -> Filters:
filter_field = self.filter_rel_fields_field.get(column_name)
filters = datamodel.get_filters([filter_field])
filter_field = self.related_field_filters.get(column_name)
if isinstance(filter_field, str):
filter_field = RelatedFieldFilter(cast(str, filter_field), FilterStartsWith)
filter_field = cast(RelatedFieldFilter, filter_field)
search_columns = [filter_field.field_name] if filter_field else None
filters = datamodel.get_filters(search_columns)
base_filters = self.filter_rel_fields.get(column_name)
if base_filters:
filters = filters.add_filter_list(base_filters)
if value:
filters.rest_add_filters(
[{"opr": "sw", "col": filter_field, "value": value}]
filters.add_filter_list(base_filters)
if value and filter_field:
filters.add_filter(
filter_field.field_name, filter_field.filter_class, value
)
return filters

48
superset/views/filters.py Normal file
View File

@ -0,0 +1,48 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you 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.
from flask_appbuilder.models.filters import BaseFilter
from flask_babel import lazy_gettext
from sqlalchemy import or_
from superset import security_manager
# pylint: disable=too-few-public-methods
class FilterRelatedOwners(BaseFilter):
"""
A filter to allow searching for related owners of a resource.
Use in the api by adding something like:
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners),
}
"""
name = lazy_gettext("Owner")
arg_name = "owners"
def apply(self, query, value):
user_model = security_manager.user_model
like_value = "%" + value + "%"
return query.filter(
or_(
# could be made to handle spaces between names more gracefully
(user_model.first_name + " " + user_model.last_name).ilike(like_value),
user_model.username.ilike(like_value),
)
)

18
tests/base_api_tests.py
View File

@ -158,20 +158,20 @@ class ApiOwnersTestCaseMixin:
API: Test get filter related owners
"""
self.login(username="admin")
argument = {"filter": "a"}
argument = {"filter": "gamma"}
uri = f"api/v1/{self.resource_name}/related/owners?q={prison.dumps(argument)}"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {
"count": 2,
"result": [
{"text": "admin user", "value": 1},
{"text": "alpha user", "value": 5},
],
}
self.assertEqual(response, expected_response)
self.assertEqual(3, response["count"])
sorted_results = sorted(response["result"], key=lambda value: value["text"])
expected_results = [
{"text": "gamma user", "value": 2},
{"text": "gamma2 user", "value": 3},
{"text": "gamma_sqllab user", "value": 4},
]
self.assertEqual(expected_results, sorted_results)
def test_get_related_fail(self):
"""

9
tests/chart_api_tests.py
View File

@ -486,7 +486,14 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
"cache_timeout": None,
"dashboards": [],
"description": None,
"owners": [{"id": 1, "username": "admin"}],
"owners": [
{
"id": 1,
"username": "admin",
"first_name": "admin",
"last_name": "user",
}
],
"params": None,
"slice_name": "title",
"viz_type": None,

9
tests/dashboards/api_tests.py
View File

@ -96,7 +96,14 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
"css": "",
"dashboard_title": "title",
"json_metadata": "",
"owners": [{"id": 1, "username": "admin"}],
"owners": [
{
"id": 1,
"username": "admin",
"first_name": "admin",
"last_name": "user",
}
],
"position_json": "",
"published": False,
"url": f"/superset/dashboard/slug1/",