feat(Tags): Allow users to favorite Tags on CRUD Listview page (#24701)

This commit is contained in:
Hugh A. Miles II
2023-07-27 13:17:26 -04:00
committed by GitHub
parent d26ea980ac
commit 3b46511439
9 changed files with 593 additions and 4 deletions

View File

@@ -24,7 +24,7 @@ import {
createErrorHandler,
Actions,
} from 'src/views/CRUD/utils';
import { useListViewResource } from 'src/views/CRUD/hooks';
import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks';
import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
import ListView, {
@@ -42,6 +42,7 @@ import { deleteTags } from 'src/features/tags/tags';
import { Tag as AntdTag } from 'antd';
import { Tag } from 'src/views/CRUD/types';
import TagCard from 'src/features/tags/TagCard';
import FaveStar from 'src/components/FaveStar';
const emptyState = {
title: t('No Tags created'),
@@ -90,6 +91,13 @@ function TagList(props: TagListProps) {
refreshData,
} = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
const tagIds = useMemo(() => tags.map(c => c.id), [tags]);
const [saveFavoriteStatus, favoriteStatus] = useFavoriteStatus(
'tag',
tagIds,
addDangerToast,
);
// TODO: Fix usage of localStorage keying on the user id
const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
@@ -109,6 +117,25 @@ function TagList(props: TagListProps) {
const columns = useMemo(
() => [
{
Cell: ({
row: {
original: { id },
},
}: any) =>
userId && (
<FaveStar
itemId={id}
saveFaveStar={saveFavoriteStatus}
isStarred={favoriteStatus[id]}
/>
),
Header: '',
id: 'id',
disableSortBy: true,
size: 'xs',
hidden: !userId,
},
{
Cell: ({
row: {

View File

@@ -564,10 +564,15 @@ const favoriteApis = {
method: 'GET',
endpoint: '/api/v1/dashboard/favorite_status/',
}),
tag: makeApi<Array<string | number>, FavoriteStatusResponse>({
requestType: 'rison',
method: 'GET',
endpoint: '/api/v1/tag/favorite_status/',
}),
};
export function useFavoriteStatus(
type: 'chart' | 'dashboard',
type: 'chart' | 'dashboard' | 'tag',
ids: Array<string | number>,
handleErrorMsg: (message: string) => void,
) {

View File

@@ -18,15 +18,26 @@ import logging
from operator import and_
from typing import Any, Optional
from flask import g
from sqlalchemy.exc import SQLAlchemyError
from superset.daos.base import BaseDAO
from superset.daos.exceptions import DAOCreateFailedError, DAODeleteFailedError
from superset.exceptions import MissingUserContextException
from superset.extensions import db
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery
from superset.tags.models import get_tag, ObjectTypes, Tag, TaggedObject, TagTypes
from superset.tags.commands.exceptions import TagNotFoundError
from superset.tags.models import (
get_tag,
ObjectTypes,
Tag,
TaggedObject,
TagTypes,
user_favorite_tag_table,
)
from superset.utils.core import get_user_id
logger = logging.getLogger(__name__)
@@ -257,3 +268,98 @@ class TagDAO(BaseDAO[Tag]):
for obj in saved_queries
)
return results
@staticmethod
def favorite_tag_by_id_for_current_user( # pylint: disable=invalid-name
tag_id: int,
) -> None:
"""
Marks a specific tag as a favorite for the current user.
This function will find the tag by the provided id,
create a new UserFavoriteTag object that represents
the user's preference, add that object to the database
session, and commit the session. It uses the currently
authenticated user from the global 'g' object.
Args:
tag_id: The id of the tag that is to be marked as
favorite.
Raises:
Any exceptions raised by the find_by_id function,
the UserFavoriteTag constructor, or the database session's
add and commit methods will propagate up to the caller.
Returns:
None.
"""
tag = TagDAO.find_by_id(tag_id)
user = g.user
if not user:
raise MissingUserContextException(message="User doesn't exist")
if not tag:
raise TagNotFoundError()
tag.users_favorited.append(user)
db.session.commit()
@staticmethod
def remove_user_favorite_tag(tag_id: int) -> None:
"""
Removes a tag from the current user's favorite tags.
This function will find the tag by the provided id and remove the tag
from the user's list of favorite tags. It uses the currently authenticated
user from the global 'g' object.
Args:
tag_id: The id of the tag that is to be removed from the favorite tags.
Raises:
Any exceptions raised by the find_by_id function, the database session's
commit method will propagate up to the caller.
Returns:
None.
"""
tag = TagDAO.find_by_id(tag_id)
user = g.user
if not user:
raise MissingUserContextException(message="User doesn't exist")
if not tag:
raise TagNotFoundError()
tag.users_favorited.remove(user)
# Commit to save the changes
db.session.commit()
@staticmethod
def favorited_ids(tags: list[Tag]) -> list[int]:
"""
Returns the IDs of tags that the current user has favorited.
This function takes in a list of Tag objects, extracts their IDs, and checks
which of these IDs exist in the user_favorite_tag_table for the current user.
The function returns a list of these favorited tag IDs.
Args:
tags (list[Tag]): A list of Tag objects.
Returns:
list[Any]: A list of IDs corresponding to the tags that are favorited by
the current user.
Example:
favorited_ids([tag1, tag2, tag3])
Output: [tag_id1, tag_id3] # if the current user has favorited tag1 and tag3
"""
ids = [tag.id for tag in tags]
return [
star.tag_id
for star in db.session.query(user_favorite_tag_table.c.tag_id)
.filter(
user_favorite_tag_table.c.tag_id.in_(ids),
user_favorite_tag_table.c.user_id == get_user_id(),
)
.all()
]

View File

@@ -200,6 +200,10 @@ class DatabaseNotFound(SupersetException):
status = 400
class MissingUserContextException(SupersetException):
status = 422
class QueryObjectValidationError(SupersetException):
status = 400

View File

@@ -0,0 +1,53 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""create_user_favorite_table
Revision ID: e0f6f91c2055
Revises: bf646a0c1501
Create Date: 2023-07-12 20:34:57.553981
"""
# revision identifiers, used by Alembic.
revision = "e0f6f91c2055"
down_revision = "bf646a0c1501"
import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql
def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"user_favorite_tag",
sa.Column("user_id", sa.Integer(), nullable=False),
sa.Column("tag_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(
["tag_id"],
["tag.id"],
),
sa.ForeignKeyConstraint(
["user_id"],
["ab_user.id"],
),
)
# ### end Alembic commands ###
def downgrade():
op.drop_table("user_favorite_tag")

View File

@@ -23,6 +23,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.daos.tag import TagDAO
from superset.exceptions import MissingUserContextException
from superset.extensions import event_logger
from superset.tags.commands.create import CreateCustomTagCommand
from superset.tags.commands.delete import DeleteTaggedObjectCommand, DeleteTagsCommand
@@ -61,6 +62,9 @@ class TagRestApi(BaseSupersetModelRestApi):
"get_all_objects",
"add_objects",
"delete_object",
"add_favorite",
"remove_favorite",
"favorite_status",
}
resource_name = "tag"
@@ -384,3 +388,150 @@ class TagRestApi(BaseSupersetModelRestApi):
exc_info=True,
)
return self.response_422(message=str(ex))
@expose("/favorite_status/", methods=("GET",))
@protect()
@safe
@statsd_metrics
@rison({"type": "array", "items": {"type": "integer"}})
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
f".favorite_status",
log_to_statsd=False,
)
def favorite_status(self, **kwargs: Any) -> Response:
"""Favorite Stars for Dashboards
---
get:
description: >-
Check favorited dashboards for current user
parameters:
- in: query
name: q
content:
application/json:
schema:
$ref: '#/components/schemas/get_fav_star_ids_schema'
responses:
200:
description:
content:
application/json:
schema:
$ref: "#/components/schemas/GetFavStarIdsSchema"
400:
$ref: '#/components/responses/400'
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
500:
$ref: '#/components/responses/500'
"""
try:
requested_ids = kwargs["rison"]
tags = TagDAO.find_by_ids(requested_ids)
users_favorited_tags = TagDAO.favorited_ids(tags)
res = [
{"id": request_id, "value": request_id in users_favorited_tags}
for request_id in requested_ids
]
return self.response(200, result=res)
except TagNotFoundError:
return self.response_404()
except MissingUserContextException as ex:
return self.response_422(message=str(ex))
@expose("/<pk>/favorites/", methods=("POST",))
@protect()
@safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
f".add_favorite",
log_to_statsd=False,
)
def add_favorite(self, pk: int) -> Response:
"""Marks the tag as favorite
---
post:
description: >-
Marks the tag as favorite for the current user
parameters:
- in: path
schema:
type: integer
name: pk
responses:
200:
description: Tag added to favorites
content:
application/json:
schema:
type: object
properties:
result:
type: object
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
try:
TagDAO.favorite_tag_by_id_for_current_user(pk)
return self.response(200, result="OK")
except TagNotFoundError:
return self.response_404()
except MissingUserContextException as ex:
return self.response_422(message=str(ex))
@expose("/<pk>/favorites/", methods=("DELETE",))
@protect()
@safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
f".remove_favorite",
log_to_statsd=False,
)
def remove_favorite(self, pk: int) -> Response:
"""Remove the tag from the user favorite list
---
delete:
description: >-
Remove the tag from the user favorite list
parameters:
- in: path
schema:
type: integer
name: pk
responses:
200:
description: Tag removed from favorites
content:
application/json:
schema:
type: object
properties:
result:
type: object
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
try:
TagDAO.remove_user_favorite_tag(pk)
return self.response(200, result="OK")
except TagNotFoundError:
return self.response_404()
except MissingUserContextException as ex:
return self.response_422(message=str(ex))

View File

@@ -20,11 +20,12 @@ import enum
from typing import TYPE_CHECKING
from flask_appbuilder import Model
from sqlalchemy import Column, Enum, ForeignKey, Integer, String, Text
from sqlalchemy import Column, Enum, ForeignKey, Integer, String, Table, Text
from sqlalchemy.engine.base import Connection
from sqlalchemy.orm import relationship, Session, sessionmaker
from sqlalchemy.orm.mapper import Mapper
from superset import security_manager
from superset.models.helpers import AuditMixinNullable
if TYPE_CHECKING:
@@ -36,6 +37,13 @@ if TYPE_CHECKING:
Session = sessionmaker(autoflush=False)
user_favorite_tag_table = Table(
"user_favorite_tag",
Model.metadata, # pylint: disable=no-member
Column("user_id", Integer, ForeignKey("ab_user.id")),
Column("tag_id", Integer, ForeignKey("tag.id")),
)
class TagTypes(enum.Enum):
@@ -82,6 +90,10 @@ class Tag(Model, AuditMixinNullable):
"TaggedObject", back_populates="tag", overlaps="objects,tags"
)
users_favorited = relationship(
security_manager.user_model, secondary=user_favorite_tag_table
)
class TaggedObject(Model, AuditMixinNullable):

View File

@@ -18,12 +18,17 @@
"""Unit tests for Superset"""
import json
from flask import g
import pytest
import prison
from sqlalchemy.sql import func
from sqlalchemy import and_
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery
from superset.tags.models import user_favorite_tag_table
from unittest.mock import patch
import tests.integration_tests.test_app
from superset import db, security_manager
@@ -372,3 +377,83 @@ class TestTagApi(SupersetTestCase):
# check that tags are all gone
tags = db.session.query(Tag).filter(Tag.name.in_(example_tag_names))
self.assertEqual(tags.count(), 0)
@pytest.mark.usefixtures("create_tags")
def test_delete_favorite_tag(self):
self.login(username="admin")
user_id = self.get_user(username="admin").get_id()
tag = db.session.query(Tag).first()
uri = f"api/v1/tag/{tag.id}/favorites/"
tag = db.session.query(Tag).first()
rv = self.client.post(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 200)
from sqlalchemy import and_
from superset.tags.models import user_favorite_tag_table
from flask import g
association_row = (
db.session.query(user_favorite_tag_table)
.filter(
and_(
user_favorite_tag_table.c.tag_id == tag.id,
user_favorite_tag_table.c.user_id == user_id,
)
)
.one_or_none()
)
assert association_row is not None
uri = f"api/v1/tag/{tag.id}/favorites/"
rv = self.client.delete(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 200)
association_row = (
db.session.query(user_favorite_tag_table)
.filter(
and_(
user_favorite_tag_table.c.tag_id == tag.id,
user_favorite_tag_table.c.user_id == user_id,
)
)
.one_or_none()
)
assert association_row is None
@pytest.mark.usefixtures("create_tags")
def test_add_tag_not_found(self):
self.login(username="admin")
uri = f"api/v1/tag/123/favorites/"
rv = self.client.post(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_tags")
def test_delete_favorite_tag_not_found(self):
self.login(username="admin")
uri = f"api/v1/tag/123/favorites/"
rv = self.client.delete(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_tags")
@patch("superset.daos.tag.g")
def test_add_tag_user_not_found(self, flask_g):
self.login(username="admin")
flask_g.user = None
uri = f"api/v1/tag/123/favorites/"
rv = self.client.post(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 422)
@pytest.mark.usefixtures("create_tags")
@patch("superset.daos.tag.g")
def test_delete_favorite_tag_user_not_found(self, flask_g):
self.login(username="admin")
flask_g.user = None
uri = f"api/v1/tag/123/favorites/"
rv = self.client.delete(uri, follow_redirects=True)
self.assertEqual(rv.status_code, 422)

View File

@@ -0,0 +1,146 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from collections.abc import Iterator
import pytest
from sqlalchemy.orm.session import Session
def test_user_favorite_tag(mocker):
from superset.daos.tag import TagDAO
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch(
"superset.daos.tag.TagDAO"
) # Replace with the actual path to TagDAO
mock_TagDAO.find_by_id.return_value = mocker.MagicMock(users_favorited=[])
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
mock_g.user = mocker.MagicMock()
# Call the function with a test tag_id
TagDAO.favorite_tag_by_id_for_current_user(123)
# Check that find_by_id was called with the right argument
mock_TagDAO.find_by_id.assert_called_once_with(123)
# Check that users_favorited was updated correctly
assert mock_TagDAO.find_by_id().users_favorited == [mock_g.user]
mock_session.commit.assert_called_once()
def test_remove_user_favorite_tag(mocker):
from superset.daos.tag import TagDAO
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch("superset.daos.tag.TagDAO")
mock_tag = mocker.MagicMock(users_favorited=[])
mock_TagDAO.find_by_id.return_value = mock_tag
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
mock_user = mocker.MagicMock()
mock_g.user = mock_user
# Append the mock user to the tag's list of favorited users
mock_tag.users_favorited.append(mock_user)
# Call the function with a test tag_id
TagDAO.remove_user_favorite_tag(123)
# Check that find_by_id was called with the right argument
mock_TagDAO.find_by_id.assert_called_once_with(123)
# Check that users_favorited no longer contains the user
assert mock_user not in mock_tag.users_favorited
# Check that the session was committed
mock_session.commit.assert_called_once()
def test_remove_user_favorite_tag_no_user(mocker):
from superset.daos.tag import TagDAO
from superset.exceptions import MissingUserContextException
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch("superset.daos.tag.TagDAO")
mock_tag = mocker.MagicMock(users_favorited=[])
mock_TagDAO.find_by_id.return_value = mock_tag
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
# Test with no user
mock_g.user = None
with pytest.raises(MissingUserContextException):
TagDAO.remove_user_favorite_tag(1)
def test_remove_user_favorite_tag_exc_raise(mocker):
from superset.daos.tag import TagDAO
from superset.exceptions import MissingUserContextException
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch("superset.daos.tag.TagDAO")
mock_tag = mocker.MagicMock(users_favorited=[])
mock_TagDAO.find_by_id.return_value = mock_tag
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
# Test that exception is raised when commit fails
mock_session.commit.side_effect = Exception("DB Error")
with pytest.raises(Exception):
TagDAO.remove_user_favorite_tag(1)
def test_user_favorite_tag_no_user(mocker):
from superset.daos.tag import TagDAO
from superset.exceptions import MissingUserContextException
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch("superset.daos.tag.TagDAO")
mock_tag = mocker.MagicMock(users_favorited=[])
mock_TagDAO.find_by_id.return_value = mock_tag
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
# Test with no user
mock_g.user = None
with pytest.raises(MissingUserContextException):
TagDAO.favorite_tag_by_id_for_current_user(1)
def test_user_favorite_tag_exc_raise(mocker):
from superset.daos.tag import TagDAO
from superset.exceptions import MissingUserContextException
# Mock the behavior of TagDAO and g
mock_session = mocker.patch("superset.daos.tag.db.session")
mock_TagDAO = mocker.patch("superset.daos.tag.TagDAO")
mock_tag = mocker.MagicMock(users_favorited=[])
mock_TagDAO.find_by_id.return_value = mock_tag
mock_g = mocker.patch("superset.daos.tag.g") # Replace with the actual path to g
# Test that exception is raised when commit fails
mock_session.commit.side_effect = Exception("DB Error")
with pytest.raises(Exception):
TagDAO.remove_user_favorite_tag(1)