[GitHub] [superset] john-bodley commented on a diff in pull request #24701: feat(Tags): Allow users to favorite Tags on CRUD Listview page

["john-bodley (via GitHub)" <gi...@apache.org>]

john-bodley commented on code in PR #24701:
URL: https://github.com/apache/superset/pull/24701#discussion_r1265649865


##########
superset/daos/tag.py:
##########
@@ -257,3 +266,90 @@ def get_tagged_objects_for_tags(
                 for obj in saved_queries
             )
         return results
+
+    @staticmethod
+    def user_favorite_tag(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 tag and user:
+            tag.users_favorited.append(user)
+            db.session.add(tag)
+            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.
+        """
+        # Find the tag
+        tag = TagDAO.find_by_id(tag_id)
+        user = g.user
+
+        # Remove the tag from the user's favorites
+        if tag and user:
+            tag.users_favorited.remove(user)

Review Comment:
   What's the TL;DR between tags and favoriting? It seems like we're doing double bookkeeping here which is likely suboptimal.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org