[GitHub] [superset] zephyring commented on a diff in pull request #24964: feat: Tags ListView Page

["zephyring (via GitHub)" <gi...@apache.org>]

zephyring commented on code in PR #24964:
URL: https://github.com/apache/superset/pull/24964#discussion_r1320180256


##########
superset/tags/api.py:
##########
@@ -192,14 +193,77 @@ def post(self) -> Response:
             return self.response(201)
         except TagInvalidError as ex:
             return self.response_422(message=ex.normalized_messages())
-        except TagCreateFailedError as ex:
-            logger.error(
-                "Error creating model %s: %s",
-                self.__class__.__name__,
-                str(ex),
-                exc_info=True,
-            )
-            return self.response_500(message=str(ex))
+
+    @expose("/bulk_create", methods=("POST",))
+    @protect()
+    @safe
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.bulk_create",
+        log_to_statsd=False,
+    )
+    def bulk_create(self) -> Response:
+        """Bulk create tags and tagged objects
+        ---
+        post:
+          summary: Get all objects associated with a tag

Review Comment:
   the openapi spec here needs to be updated



##########
tests/integration_tests/tags/api_tests.py:
##########
@@ -501,3 +503,54 @@ def test_put_tag(self):
             .one_or_none()
         )
         assert tag is not None
+
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    @pytest.mark.usefixtures("create_tags")
+    def test_failed_put_tag(self):
+        self.login(username="admin")
+
+        tag_to_update = db.session.query(Tag).first()
+        uri = f"api/v1/tag/{tag_to_update.id}"
+        rv = self.client.put(uri, json={"foo": "bar"})
+
+        self.assertEqual(rv.status_code, 400)
+
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    def test_post_bulk_tag(self):
+        self.login(username="admin")
+        uri = "api/v1/tag/bulk_create"
+        dashboard = (
+            db.session.query(Dashboard)
+            .filter(Dashboard.dashboard_title == "World Bank's Data")
+            .first()
+        )
+        chart = db.session.query(Slice).first()
+        tags = ["tag1", "tag2", "tag3"]
+        rv = self.client.post(
+            uri,
+            json={
+                "tags": ["tag1", "tag2", "tag3"],
+                "objects_to_tag": [["dashboard", dashboard.id], ["chart", chart.id]],

Review Comment:
   I personally prefer a flatten structure where tag to resource is 1:1 in the request. So in the above example instead of 3 tags and 2 resources which end up backend applying all tags to all resources the frontend will be responsible for composing 6 resource to tag. Something like
   ```
   [
     {
       "resource_type": "dashboard",
       "resource_id": 1,
       "tag": "tag1",
     },
     {
       "resource_type": "chart",
       "resource_id": 1,
       "tag": "tag1",
     },
     {
       "resource_type": "dashboard",
       "resource_id": 1,
       "tag": "tag2",
     },
     {
       "resource_type": "chart",
       "resource_id": 1,
       "tag": "tag2",
     },
     {
       "resource_type": "dashboard",
       "resource_id": 1,
       "tag": "tag3",
     },
     {
       "resource_type": "chart",
       "resource_id": 1,
       "tag": "tag3",
     },
   ].
   ```
   This way the request is much more flexible so if you want to tag chart1 with tag1, tag2 and dashboard1 with tag2, tag3. You don't need to compose multiple requests to backend and doing the math.



-- 
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