bito-code-review[bot] commented on code in PR #41461:
URL: https://github.com/apache/superset/pull/41461#discussion_r3523801241
##########
superset/tags/filters.py:
##########
@@ -48,6 +49,31 @@ def apply(self, query: Query, value: bool) -> Query:
return query
+class TagFavoriteFilter(BaseFilter): # pylint: disable=too-few-public-methods
+ """
+ Custom filter for the GET list that filters tags the current user has
+ favorited or not.
+
+ Tag favorites are stored in the dedicated ``user_favorite_tag_table`` M2M
+ table rather than in ``FavStar``, so this filter cannot reuse
+ ``BaseFavoriteFilter`` (which queries ``FavStar``).
+ """
+
+ name = _("Is favorite")
+ arg_name = "tag_is_favorite"
+
+ def apply(self, query: Query, value: Any) -> Query:
+ # If anonymous user filter nothing
+ if security_manager.current_user is None:
+ return query
+ users_favorite_query = db.session.query(
+ user_favorite_tag_table.c.tag_id
+ ).filter(user_favorite_tag_table.c.user_id == get_user_id())
+ if value:
+ return query.filter(Tag.id.in_(users_favorite_query))
+ return query.filter(~Tag.id.in_(users_favorite_query))
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to add unit tests for the new filter class is a
standard practice for ensuring the correctness of new logic. While it is noted
that existing parallel implementations lack dedicated unit tests, adding tests
for this new class improves the test coverage and reliability of the codebase.
Applying this suggestion is recommended to ensure the filtering logic behaves
as expected for both authenticated and anonymous users.
--
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]
