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

chore(tags): Allow for lookup via ids vs. name in the API #25996

Merged
merged 7 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions superset-frontend/src/features/tags/TagModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Input } from 'antd';
import { Divider } from 'src/components';
import Button from 'src/components/Button';
import { Tag } from 'src/views/CRUD/types';
import { fetchObjects } from 'src/features/tags/tags';
import { fetchObjectsByTagIds } from 'src/features/tags/tags';

const StyledModalBody = styled.div`
.ant-select-dropdown {
Expand Down Expand Up @@ -107,8 +107,8 @@ const TagModal: React.FC<TagModalProps> = ({
};
clearResources();
if (isEditMode) {
fetchObjects(
{ tags: editTag.name, types: null },
fetchObjectsByTagIds(
{ tagIds: [editTag.id], types: null },
(data: Tag[]) => {
data.forEach(updateResourceOptions);
setDashboardsToTag(resourceMap[TaggableResources.Dashboard]);
Expand Down
17 changes: 17 additions & 0 deletions superset-frontend/src/features/tags/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,20 @@ export function fetchObjects(
.then(({ json }) => callback(json.result))
.catch(response => error(response));
}

export function fetchObjectsByTagIds(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is fetchObjects above this just dead code now? I don't see it being used elsewhere.

{
tagIds = [],
types,
}: { tagIds: number[] | undefined; types: string | null },
callback: (json: JsonObject) => void,
error: (response: Response) => void,
) {
let url = `/api/v1/tag/get_objects/?tagIds=${tagIds}`;
if (types) {
url += `&types=${types}`;
}
SupersetClient.get({ endpoint: url })
.then(({ json }) => callback(json.result))
.catch(response => error(response));
}
10 changes: 7 additions & 3 deletions superset-frontend/src/pages/AllEntities/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions';
import { Tag } from 'src/views/CRUD/types';
import TagModal from 'src/features/tags/TagModal';
import withToasts, { useToasts } from 'src/components/MessageToasts/withToasts';
import { fetchObjects, fetchSingleTag } from 'src/features/tags/tags';
import { fetchObjectsByTagIds, fetchSingleTag } from 'src/features/tags/tags';
import Loading from 'src/components/Loading';

interface TaggedObject {
Expand Down Expand Up @@ -146,8 +146,12 @@ function AllEntities() {

const fetchTaggedObjects = () => {
setLoading(true);
fetchObjects(
{ tags: tag?.name || '', types: null },
if (!tag) {
addDangerToast('Error tag object is not referenced!');
return;
}
fetchObjectsByTagIds(
{ tagIds: [tag?.id] || '', types: null },
(data: TaggedObject[]) => {
const objects = { dashboard: [], chart: [], query: [] };
data.forEach(function (object) {
Expand Down
8 changes: 8 additions & 0 deletions superset/daos/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ def find_tagged_object(
.first()
)

@staticmethod
def get_tagged_objects_by_tag_id(
tag_ids: Optional[list[int]], obj_types: Optional[list[str]] = None
) -> list[dict[str, Any]]:
tags = db.session.query(Tag).filter(Tag.id.in_(tag_ids)).all()
tag_names = [tag.name for tag in tags]
return TagDAO.get_tagged_objects_for_tags(tag_names, obj_types)

@staticmethod
def get_tagged_objects_for_tags(
tags: Optional[list[str]] = None, obj_types: Optional[list[str]] = None
Expand Down
15 changes: 12 additions & 3 deletions superset/tags/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,12 +584,21 @@ def get_objects(self) -> Response:
500:
$ref: '#/components/responses/500'
"""
tag_ids = [
tag_id for tag_id in request.args.get("tagIds", "").split(",") if tag_id
]
tags = [tag for tag in request.args.get("tags", "").split(",") if tag]
# filter types
types = [type_ for type_ in request.args.get("types", "").split(",") if type_]

try:
tagged_objects = TagDAO.get_tagged_objects_for_tags(tags, types)
if tag_ids:
# priotize using ids for lookups vs. names mainly using this
# for backward compatibility
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(tag_ids, types)
else:
tagged_objects = TagDAO.get_tagged_objects_for_tags(tags, types)

result = [
self.object_entity_response_schema.dump(tagged_object)
for tagged_object in tagged_objects
Expand All @@ -609,11 +618,11 @@ def get_objects(self) -> Response:
log_to_statsd=False,
)
def favorite_status(self, **kwargs: Any) -> Response:
"""Favorite Stars for Dashboards
"""Favorite Stars for Tags
---
get:
description: >-
Check favorited dashboards for current user
Get favorited tags for current user
parameters:
- in: query
name: q
Expand Down
33 changes: 33 additions & 0 deletions tests/integration_tests/tags/dao_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,39 @@ def test_get_objects_from_tag(self):
tagged_objects = TagDAO.get_tagged_objects_for_tags(obj_types=["chart"])
assert len(tagged_objects) == num_charts

@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
@pytest.mark.usefixtures("with_tagging_system_feature")
@pytest.mark.usefixtures("create_tags")
# test get objects from tag
def test_get_objects_from_tag_with_id(self):
# create tagged objects
dashboard = (
db.session.query(Dashboard)
.filter(Dashboard.dashboard_title == "World Bank's Data")
.first()
)
dashboard_id = dashboard.id
tag_1 = db.session.query(Tag).filter_by(name="example_tag_1").one()
tag_2 = db.session.query(Tag).filter_by(name="example_tag_2").one()
tag_ids = [tag_1.id, tag_2.id]
self.insert_tagged_object(
object_id=dashboard_id, object_type=ObjectType.dashboard, tag_id=tag_1.id
)
# get objects
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(tag_ids)
assert len(tagged_objects) == 1

# test get objects from tag with type
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(
tag_ids, obj_types=["dashboard", "chart"]
)
assert len(tagged_objects) == 1

tagged_objects = TagDAO.get_tagged_objects_by_tag_id(
tag_ids, obj_types=["chart"]
)
assert len(tagged_objects) == 0

@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
@pytest.mark.usefixtures("with_tagging_system_feature")
@pytest.mark.usefixtures("create_tagged_objects")
Expand Down
Loading