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...
apache · Apr 7, 2020 · 5e53506 · 5e53506
1 parent 4be8275
commit 5e53506
Show file tree

Hide file tree

Showing 12 changed files with 366 additions and 269 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
@@ -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",

diff --git a/superset-frontend/src/dashboard/components/PropertiesModal.jsx b/superset-frontend/src/dashboard/components/PropertiesModal.jsx
@@ -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,18 +56,18 @@ class PropertiesModal extends React.PureComponent {
         json_metadata: '',
       },
       isDashboardLoaded: false,
-      ownerOptions: null,
       isAdvancedOpen: false,
     };
     this.onChange = this.onChange.bind(this);
     this.onMetadataChange = this.onMetadataChange.bind(this);
     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>

diff --git a/superset-frontend/src/explore/components/PropertiesModal.tsx b/superset-frontend/src/explore/components/PropertiesModal.tsx
@@ -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,15 +71,14 @@ 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 || '');
   const [description, setDescription] = useState(slice.description || '');
   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>

diff --git a/superset/charts/api.py b/superset/charts/api.py
@@ -43,7 +43,8 @@
 )
 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"])