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

feat: only show active user for chart/dashboard/datasource owner drop… #20837

Merged
merged 11 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions superset-frontend/src/components/Datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,12 @@ function OwnersSelector({ datasource, onChange }) {
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,
})),
data: response.json.result
.filter(item => item.extra.active)
.map(item => ({
value: item.value,
label: item.text,
})),
totalCount: response.json.count,
}));
}, []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ const PropertiesModal = ({
return SupersetClient.get({
endpoint: `/api/v1/dashboard/related/${accessType}?q=${query}`,
}).then(response => ({
data: response.json.result.map(
(item: { value: number; text: string }) => ({
data: response.json.result
.filter((item: { extra: { active: boolean } }) => item.extra.active)
.map((item: { value: number; text: string }) => ({
value: item.value,
label: item.text,
}),
),
})),
totalCount: response.json.count,
}));
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ function PropertiesModal({
return SupersetClient.get({
endpoint: `/api/v1/chart/related/owners?q=${query}`,
}).then(response => ({
data: response.json.result.map(
(item: { value: number; text: string }) => ({
data: response.json.result
.filter((item: { extra: { active: boolean } }) => item.extra.active)
.map((item: { value: number; text: string }) => ({
value: item.value,
label: item.text,
}),
),
})),
totalCount: response.json.count,
}));
},
Expand Down
21 changes: 21 additions & 0 deletions superset/views/base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,15 @@ class BaseSupersetModelRestApi(ModelRestApi):
}
"""

extra_fields_rel_fields: Dict[str, List[str]] = {"owners": ["email", "active"]}
"""
Declare extra fields for the representation of the Model object::

extra_fields_rel_fields = {
"<RELATED_FIELD>": "[<RELATED_OBJECT_FIELD_1>, <RELATED_OBJECT_FIELD_2>]"
}
"""

allowed_distinct_fields: Set[str] = set()

add_columns: List[str]
Expand Down Expand Up @@ -317,13 +326,25 @@ def _get_text_for_model(self, model: Model, column_name: str) -> str:
return getattr(model, model_column_name)
return str(model)

def _get_extra_field_for_model(
self, model: Model, column_name: str
) -> Dict[str, str]:
ret = {}
if column_name in self.extra_fields_rel_fields:
model_column_names = self.extra_fields_rel_fields.get(column_name)
if model_column_names:
for key in model_column_names:
ret[key] = getattr(model, key)
return ret

def _get_result_from_rows(
self, datamodel: SQLAInterface, rows: List[Model], column_name: str
) -> List[Dict[str, Any]]:
return [
{
"value": datamodel.get_pk_value(row),
"text": self._get_text_for_model(row, column_name),
"extra": self._get_extra_field_for_model(row, column_name),
zephyring marked this conversation as resolved.
Show resolved Hide resolved
}
for row in rows
]
Expand Down
48 changes: 40 additions & 8 deletions tests/integration_tests/base_api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,26 @@ def test_get_filter_related_owners(self):
assert 4 == 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_no_csv user", "value": 6},
{"text": "gamma_sqllab user", "value": 4},
{
"extra": {"active": True, "email": "gamma@fab.org"},
"text": "gamma user",
"value": 2,
},
{
"extra": {"active": True, "email": "gamma2@fab.org"},
"text": "gamma2 user",
"value": 3,
},
{
"extra": {"active": True, "email": "gamma_no_csv@fab.org"},
"text": "gamma_no_csv user",
"value": 6,
},
{
"extra": {"active": True, "email": "gamma_sqllab@fab.org"},
"text": "gamma_sqllab user",
"value": 4,
},
]
# TODO Check me
assert expected_results == sorted_results
Expand All @@ -286,8 +302,16 @@ def test_get_ids_related_owners(self):
assert 2 == response["count"]
sorted_results = sorted(response["result"], key=lambda value: value["text"])
expected_results = [
{"text": "gamma user", "value": 2},
{"text": "gamma_sqllab user", "value": 4},
{
"extra": {"active": True, "email": "gamma@fab.org"},
"text": "gamma user",
"value": 2,
},
{
"extra": {"active": True, "email": "gamma_sqllab@fab.org"},
"text": "gamma_sqllab user",
"value": 4,
},
]
assert expected_results == sorted_results

Expand All @@ -305,8 +329,16 @@ def test_get_repeated_ids_related_owners(self):
assert 2 == response["count"]
sorted_results = sorted(response["result"], key=lambda value: value["text"])
expected_results = [
{"text": "gamma user", "value": 2},
{"text": "gamma_sqllab user", "value": 4},
{
"extra": {"active": True, "email": "gamma@fab.org"},
"text": "gamma user",
"value": 2,
},
{
"extra": {"active": True, "email": "gamma_sqllab@fab.org"},
"text": "gamma_sqllab user",
"value": 4,
},
]
assert expected_results == sorted_results

Expand Down
3 changes: 2 additions & 1 deletion tests/integration_tests/queries/saved_queries/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ def test_related_saved_query(self):
expected_result = {
"count": len(databases),
"result": [
{"text": str(database), "value": database.id} for database in databases
{"extra": {}, "text": str(database), "value": database.id}
for database in databases
],
}

Expand Down