From 959fd763a8b22367e7de5d3438e457f3448c6ceb Mon Sep 17 00:00:00 2001
From: "Hugh A. Miles II" <hughmil3s@gmail.com>
Date: Tue, 12 Oct 2021 17:27:56 -0700
Subject: [PATCH] fix: Owners selection in dataset edit UX (#17063)

* boilerplate

* update owner select component

* this is working

* update onchange

* refactorig

* you need to useMemo or things break

* update test

* prettier

* move logic into bootstrap data endpoint

* address concerns

* oops

* oops

* fix test
---
 .../spec/fixtures/mockDatasource.js           |  1 +
 .../components/FiltersBadge_spec.tsx          |  1 -
 .../src/datasource/DatasourceEditor.jsx       | 60 ++++++++++++-------
 .../src/datasource/DatasourceModal.tsx        |  4 +-
 .../DatasourceControl.test.tsx                |  1 +
 .../views/CRUD/data/dataset/DatasetList.tsx   |  3 +-
 superset/connectors/base/models.py            | 12 ++++
 superset/views/core.py                        |  3 +
 8 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/superset-frontend/spec/fixtures/mockDatasource.js b/superset-frontend/spec/fixtures/mockDatasource.js
index 55d95ebde2..ca60a712b2 100644
--- a/superset-frontend/spec/fixtures/mockDatasource.js
+++ b/superset-frontend/spec/fixtures/mockDatasource.js
@@ -168,6 +168,7 @@ export default {
     id,
     granularity_sqla: [['ds', 'ds']],
     name: 'birth_names',
+    owners: [{ first_name: 'joe', last_name: 'man', id: 1 }],
     database: {
       allow_multi_schema_metadata_fetch: null,
       name: 'main',
diff --git a/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx b/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx
index f24b973199..539aa67a69 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx
@@ -139,7 +139,6 @@ describe('FiltersBadge', () => {
         wrapper.find('[data-test="incompatible-filter-count"]'),
       ).toHaveText('1');
       // to look at the shape of the wrapper use:
-      // console.log(wrapper.debug())
       expect(wrapper.find(Icons.AlertSolid)).toExist();
     });
   });
diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx
index e502a3e2b2..e725cdc238 100644
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import rison from 'rison';
-import React from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
 import { Row, Col } from 'src/common/components';
 import { Radio } from 'src/components/Radio';
@@ -26,6 +26,8 @@ import Alert from 'src/components/Alert';
 import Badge from 'src/components/Badge';
 import shortid from 'shortid';
 import { styled, SupersetClient, t, supersetTheme } from '@superset-ui/core';
+import { Select } from 'src/components';
+import { FormLabel } from 'src/components/Form';
 import Button from 'src/components/Button';
 import Tabs from 'src/components/Tabs';
 import CertifiedIcon from 'src/components/CertifiedIcon';
@@ -40,9 +42,7 @@ import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 
 import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
 import TextControl from 'src/explore/components/controls/TextControl';
-import { Select } from 'src/components';
 import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
-import SelectAsyncControl from 'src/explore/components/controls/SelectAsyncControl';
 import SpatialControl from 'src/explore/components/controls/SpatialControl';
 
 import CollectionTable from 'src/CRUD/CollectionTable';
@@ -374,12 +374,44 @@ const defaultProps = {
   onChange: () => {},
 };
 
+function OwnersSelector({ datasource, onChange }) {
+  const loadOptions = useCallback((search = '', page, pageSize) => {
+    const query = rison.encode({ filter: search, page, page_size: pageSize });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dataset/related/owners?q=${query}`,
+    }).then(response => ({
+      data: response.json.result.map(item => ({
+        value: item.value,
+        label: item.text,
+      })),
+      totalCount: response.json.count,
+    }));
+  }, []);
+
+  return (
+    <Select
+      ariaLabel={t('Select owners')}
+      mode="multiple"
+      name="owners"
+      value={datasource.owners}
+      options={loadOptions}
+      onChange={onChange}
+      header={<FormLabel>{t('Owners')}</FormLabel>}
+      allowClear
+    />
+  );
+}
+
 class DatasourceEditor extends React.PureComponent {
   constructor(props) {
     super(props);
     this.state = {
       datasource: {
         ...props.datasource,
+        owners: props.datasource.owners.map(owner => ({
+          value: owner.id,
+          label: `${owner.first_name} ${owner.last_name}`,
+        })),
         metrics: props.datasource.metrics?.map(metric => {
           const {
             certified_by: certifiedByMetric,
@@ -717,23 +749,11 @@ class DatasourceEditor extends React.PureComponent {
             }
           />
         )}
-        <Field
-          fieldKey="owners"
-          label={t('Owners')}
-          description={t('Owners of the dataset')}
-          control={
-            <SelectAsyncControl
-              dataEndpoint="api/v1/dataset/related/owners"
-              multi
-              mutator={data =>
-                data.result.map(pk => ({
-                  value: pk.value,
-                  label: `${pk.text}`,
-                }))
-              }
-            />
-          }
-          controlProps={{}}
+        <OwnersSelector
+          datasource={datasource}
+          onChange={newOwners => {
+            this.onDatasourceChange({ ...datasource, owners: newOwners });
+          }}
         />
       </Fieldset>
     );
diff --git a/superset-frontend/src/datasource/DatasourceModal.tsx b/superset-frontend/src/datasource/DatasourceModal.tsx
index 75d1708c15..5cd6f3aa3f 100644
--- a/superset-frontend/src/datasource/DatasourceModal.tsx
+++ b/superset-frontend/src/datasource/DatasourceModal.tsx
@@ -99,7 +99,6 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
       currentDatasource.schema;
 
     setIsSaving(true);
-
     SupersetClient.post({
       endpoint: '/datasource/save/',
       postPayload: {
@@ -119,6 +118,9 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
             }),
           ),
           type: currentDatasource.type || currentDatasource.datasource_type,
+          owners: currentDatasource.owners.map(
+            (o: { label: string; value: number }) => o.value,
+          ),
         },
       },
     })
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
index 744b929a62..43ac29b2c7 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -41,6 +41,7 @@ const createProps = () => ({
     name: 'channels',
     type: 'table',
     columns: [],
+    owners: [{ first_name: 'john', last_name: 'doe', id: 1, username: 'jd' }],
   },
   validationErrors: [],
   name: 'datasource',
diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
index e8f05830bb..bdc9f4084d 100644
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
@@ -165,7 +165,6 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
         endpoint: `/api/v1/dataset/${id}`,
       })
         .then(({ json = {} }) => {
-          const owners = json.result.owners.map((owner: any) => owner.id);
           const addCertificationFields = json.result.columns.map(
             (column: ColumnObject) => {
               const {
@@ -181,7 +180,7 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
           );
           // eslint-disable-next-line no-param-reassign
           json.result.columns = [...addCertificationFields];
-          setDatasetCurrentlyEditing({ ...json.result, owners });
+          setDatasetCurrentlyEditing(json.result);
         })
         .catch(() => {
           addDangerToast(
diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index 844dc48bb8..711f1b72b9 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -118,6 +118,18 @@ class BaseDatasource(
     def kind(self) -> DatasourceKind:
         return DatasourceKind.VIRTUAL if self.sql else DatasourceKind.PHYSICAL
 
+    @property
+    def owners_data(self) -> List[Dict[str, Any]]:
+        return [
+            {
+                "first_name": o.first_name,
+                "last_name": o.last_name,
+                "username": o.username,
+                "id": o.id,
+            }
+            for o in self.owners
+        ]
+
     @property
     def is_virtual(self) -> bool:
         return self.kind == DatasourceKind.VIRTUAL
diff --git a/superset/views/core.py b/superset/views/core.py
index 8d6345b223..6dfa63050c 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -856,6 +856,9 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
         except (SupersetException, SQLAlchemyError):
             datasource_data = dummy_datasource_data
 
+        if datasource:
+            datasource_data["owners"] = datasource.owners_data
+
         bootstrap_data = {
             "can_add": slice_add_perm,
             "can_download": slice_download_perm,