betodealmeida commented on a change in pull request #17063:
URL: https://github.com/apache/superset/pull/17063#discussion_r727290170
##########
File path: superset/connectors/base/models.py
##########
@@ -265,7 +265,11 @@ def data(self) -> Dict[str, Any]:
"metrics": [o.data for o in self.metrics],
# TODO deprecate, move logic to JS
"order_by_choices": order_by_choices,
- "owners": [owner.id for owner in self.owners],
+ # Passing the format needed to power owner selection in edit screen
+ "owners": [
+ {"label": f"{owner.first_name} {owner.last_name}", "value":
owner.id}
+ for owner in self.owners
+ ],
Review comment:
`label` and `value` are concerns tied to the component you're using.
Let's discuss this a bit more.
##########
File path: superset/connectors/base/models.py
##########
@@ -265,7 +265,11 @@ def data(self) -> Dict[str, Any]:
"metrics": [o.data for o in self.metrics],
# TODO deprecate, move logic to JS
"order_by_choices": order_by_choices,
- "owners": [owner.id for owner in self.owners],
+ # Passing the format needed to power owner selection in edit screen
+ "owners": [
+ {"label": f"{owner.first_name} {owner.last_name}", "value":
owner.id}
+ for owner in self.owners
+ ],
Review comment:
`label` and `value` are concerns tied to the component you're using in
the frontend. Let's discuss this a bit more.
##########
File path: superset/connectors/base/models.py
##########
@@ -265,7 +265,11 @@ def data(self) -> Dict[str, Any]:
"metrics": [o.data for o in self.metrics],
# TODO deprecate, move logic to JS
"order_by_choices": order_by_choices,
- "owners": [owner.id for owner in self.owners],
+ # Passing the format needed to power owner selection in edit screen
+ "owners": [
+ {"label": f"{owner.first_name} {owner.last_name}", "value":
owner.id}
+ for owner in self.owners
+ ],
Review comment:
I assume we have endpoints where we can get full user information,
including first name and last name. The component should fetch from there and
build `label` and `value` however it needs.
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -374,6 +374,40 @@ const defaultProps = {
onChange: () => {},
};
+function OwnersSelector({ datasource, onChange }) {
+ const selectedOwners = datasource.owners.map(owner => ({
+ value: owner.id,
+ label: `${owner.first_name} ${owner.last_name}`,
+ }));
+ const loadOptions = useMemo(() => (search = '', page, pageSize) => {
+ const query = rison.encode({ filter: search, page, page_size: pageSize });
+ return SupersetClient.get({
+ endpoint: `/api/v1/chart/related/owners?q=${query}`,
+ }).then(response => ({
+ data: response.json.result.map(item => ({
+ value: item.value,
+ label: item.text,
+ })),
+ totalCount: response.json.count,
+ }));
+ });
Review comment:
I'm going to push back here... let's try to understand why `useMemo` is
needed? My understanding is the same as Elizabeth's, without the dependency
array it's a no-op.
We've had a lot of quality issues lately, let's not commit code that we
don't understand what's doing.
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -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,
+ }));
+ }, []);
Review comment:
Curious to what the error is when you just use the function directly?
Using `useCallback` is basically the same as `useMemo`, though now you have
an empty array of dependencies so it will memoize forever, which is what we
want.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
