chore(command): Condense delete/bulk-delete operations (#24607)

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
(cherry picked from commit a156816)

superset/annotation_layers/annotations/api.py

@@ -24,17 +24,13 @@
 from flask_babel import ngettext
 from marshmallow import ValidationError
 
-from superset.annotation_layers.annotations.commands.bulk_delete import (
-    BulkDeleteAnnotationCommand,
-)
 from superset.annotation_layers.annotations.commands.create import (
     CreateAnnotationCommand,
 )
 from superset.annotation_layers.annotations.commands.delete import (
     DeleteAnnotationCommand,
 )
 from superset.annotation_layers.annotations.commands.exceptions import (
-    AnnotationBulkDeleteFailedError,
     AnnotationCreateFailedError,
     AnnotationDeleteFailedError,
     AnnotationInvalidError,
@@ -438,7 +434,7 @@ def delete(  # pylint: disable=arguments-differ
               $ref: '#/components/responses/500'
         """
         try:
-            DeleteAnnotationCommand(annotation_id).run()
+            DeleteAnnotationCommand([annotation_id]).run()
             return self.response(200, message="OK")
         except AnnotationNotFoundError:
             return self.response_404()
@@ -495,7 +491,7 @@ def bulk_delete(self, **kwargs: Any) -> Response:
         """
         item_ids = kwargs["rison"]
         try:
-            BulkDeleteAnnotationCommand(item_ids).run()
+            DeleteAnnotationCommand(item_ids).run()
             return self.response(
                 200,
                 message=ngettext(
@@ -506,5 +502,5 @@ def bulk_delete(self, **kwargs: Any) -> Response:
             )
         except AnnotationNotFoundError:
             return self.response_404()
-        except AnnotationBulkDeleteFailedError as ex:
+        except AnnotationDeleteFailedError as ex:
             return self.response_422(message=str(ex))

superset/annotation_layers/annotations/commands/delete.py

@@ -30,22 +30,22 @@
 
 
 class DeleteAnnotationCommand(BaseCommand):
-    def __init__(self, model_id: int):
-        self._model_id = model_id
-        self._model: Optional[Annotation] = None
+    def __init__(self, model_ids: list[int]):
+        self._model_ids = model_ids
+        self._models: Optional[list[Annotation]] = None
 
     def run(self) -> None:
         self.validate()
-        assert self._model
+        assert self._models
 
         try:
-            AnnotationDAO.delete(self._model)
+            AnnotationDAO.delete(self._models)
         except DAODeleteFailedError as ex:
             logger.exception(ex.exception)
             raise AnnotationDeleteFailedError() from ex
 
     def validate(self) -> None:
         # Validate/populate model exists
-        self._model = AnnotationDAO.find_by_id(self._model_id)
-        if not self._model:
+        self._models = AnnotationDAO.find_by_ids(self._model_ids)
+        if not self._models or len(self._models) != len(self._model_ids):
             raise AnnotationNotFoundError()

superset/annotation_layers/annotations/commands/exceptions.py

@@ -48,10 +48,6 @@ def __init__(self) -> None:
         )
 
 
-class AnnotationBulkDeleteFailedError(DeleteFailedError):
-    message = _("Annotations could not be deleted.")
-
-
 class AnnotationNotFoundError(CommandException):
     message = _("Annotation not found.")
 
@@ -68,5 +64,5 @@ class AnnotationUpdateFailedError(CreateFailedError):
     message = _("Annotation could not be updated.")
 
 
-class AnnotationDeleteFailedError(CommandException):
-    message = _("Annotation delete failed.")
+class AnnotationDeleteFailedError(DeleteFailedError):
+    message = _("Annotations could not be deleted.")

superset/annotation_layers/api.py

@@ -23,14 +23,9 @@
 from flask_babel import ngettext
 from marshmallow import ValidationError
 
-from superset.annotation_layers.commands.bulk_delete import (
-    BulkDeleteAnnotationLayerCommand,
-)
 from superset.annotation_layers.commands.create import CreateAnnotationLayerCommand
 from superset.annotation_layers.commands.delete import DeleteAnnotationLayerCommand
 from superset.annotation_layers.commands.exceptions import (
-    AnnotationLayerBulkDeleteFailedError,
-    AnnotationLayerBulkDeleteIntegrityError,
     AnnotationLayerCreateFailedError,
     AnnotationLayerDeleteFailedError,
     AnnotationLayerDeleteIntegrityError,
@@ -151,7 +146,7 @@ def delete(self, pk: int) -> Response:
               $ref: '#/components/responses/500'
         """
         try:
-            DeleteAnnotationLayerCommand(pk).run()
+            DeleteAnnotationLayerCommand([pk]).run()
             return self.response(200, message="OK")
         except AnnotationLayerNotFoundError:
             return self.response_404()
@@ -346,7 +341,7 @@ def bulk_delete(self, **kwargs: Any) -> Response:
         """
         item_ids = kwargs["rison"]
         try:
-            BulkDeleteAnnotationLayerCommand(item_ids).run()
+            DeleteAnnotationLayerCommand(item_ids).run()
             return self.response(
                 200,
                 message=ngettext(
@@ -357,7 +352,7 @@ def bulk_delete(self, **kwargs: Any) -> Response:
             )
         except AnnotationLayerNotFoundError:
             return self.response_404()
-        except AnnotationLayerBulkDeleteIntegrityError as ex:
+        except AnnotationLayerDeleteIntegrityError as ex:
             return self.response_422(message=str(ex))
-        except AnnotationLayerBulkDeleteFailedError as ex:
+        except AnnotationLayerDeleteFailedError as ex:
             return self.response_422(message=str(ex))

superset/annotation_layers/commands/delete.py

@@ -31,24 +31,24 @@
 
 
 class DeleteAnnotationLayerCommand(BaseCommand):
-    def __init__(self, model_id: int):
-        self._model_id = model_id
-        self._model: Optional[AnnotationLayer] = None
+    def __init__(self, model_ids: list[int]):
+        self._model_ids = model_ids
+        self._models: Optional[list[AnnotationLayer]] = None
 
     def run(self) -> None:
         self.validate()
-        assert self._model
+        assert self._models
 
         try:
-            AnnotationLayerDAO.delete(self._model)
+            AnnotationLayerDAO.delete(self._models)
         except DAODeleteFailedError as ex:
             logger.exception(ex.exception)
             raise AnnotationLayerDeleteFailedError() from ex
 
     def validate(self) -> None:
         # Validate/populate model exists
-        self._model = AnnotationLayerDAO.find_by_id(self._model_id)
-        if not self._model:
+        self._models = AnnotationLayerDAO.find_by_ids(self._model_ids)
+        if not self._models or len(self._models) != len(self._model_ids):
             raise AnnotationLayerNotFoundError()
-        if AnnotationLayerDAO.has_annotations(self._model.id):
+        if AnnotationLayerDAO.has_annotations(self._model_ids):
             raise AnnotationLayerDeleteIntegrityError()

superset/annotation_layers/commands/exceptions.py

@@ -29,10 +29,6 @@ class AnnotationLayerInvalidError(CommandInvalidError):
     message = _("Annotation layer parameters are invalid.")
 
 
-class AnnotationLayerBulkDeleteFailedError(DeleteFailedError):
-    message = _("Annotation layer could not be deleted.")
-
-
 class AnnotationLayerCreateFailedError(CreateFailedError):
     message = _("Annotation layer could not be created.")
 
@@ -45,18 +41,14 @@ class AnnotationLayerNotFoundError(CommandException):
     message = _("Annotation layer not found.")
 
 
-class AnnotationLayerDeleteFailedError(CommandException):
-    message = _("Annotation layer delete failed.")
+class AnnotationLayerDeleteFailedError(DeleteFailedError):
+    message = _("Annotation layers could not be deleted.")
 
 
 class AnnotationLayerDeleteIntegrityError(CommandException):
     message = _("Annotation layer has associated annotations.")
 
 
-class AnnotationLayerBulkDeleteIntegrityError(CommandException):
-    message = _("Annotation layer has associated annotations.")
-
-
 class AnnotationLayerNameUniquenessValidationError(ValidationError):
     """
     Marshmallow validation error for annotation layer name already exists

superset/charts/api.py

@@ -32,11 +32,9 @@
 from werkzeug.wsgi import FileWrapper
 
 from superset import app, is_feature_enabled, thumbnail_cache
-from superset.charts.commands.bulk_delete import BulkDeleteChartCommand
 from superset.charts.commands.create import CreateChartCommand
 from superset.charts.commands.delete import DeleteChartCommand
 from superset.charts.commands.exceptions import (
-    ChartBulkDeleteFailedError,
     ChartCreateFailedError,
     ChartDeleteFailedError,
     ChartForbiddenError,
@@ -461,7 +459,7 @@ def delete(self, pk: int) -> Response:
               $ref: '#/components/responses/500'
         """
         try:
-            DeleteChartCommand(pk).run()
+            DeleteChartCommand([pk]).run()
             return self.response(200, message="OK")
         except ChartNotFoundError:
             return self.response_404()
@@ -521,7 +519,7 @@ def bulk_delete(self, **kwargs: Any) -> Response:
         """
         item_ids = kwargs["rison"]
         try:
-            BulkDeleteChartCommand(item_ids).run()
+            DeleteChartCommand(item_ids).run()
             return self.response(
                 200,
                 message=ngettext(
@@ -532,7 +530,7 @@ def bulk_delete(self, **kwargs: Any) -> Response:
             return self.response_404()
         except ChartForbiddenError:
             return self.response_403()
-        except ChartBulkDeleteFailedError as ex:
+        except ChartDeleteFailedError as ex:
             return self.response_422(message=str(ex))
 
     @expose("/<pk>/cache_screenshot/", methods=("GET",))