Skip to content

Commit

Permalink
chore(command): Condense delete/bulk-delete operations
Browse files Browse the repository at this point in the history
  • Loading branch information
john-bodley committed Jul 7, 2023
1 parent 7409289 commit fd66f43
Show file tree
Hide file tree
Showing 35 changed files with 120 additions and 543 deletions.
10 changes: 3 additions & 7 deletions superset/annotation_layers/annotations/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand All @@ -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))
51 changes: 0 additions & 51 deletions superset/annotation_layers/annotations/commands/bulk_delete.py

This file was deleted.

14 changes: 7 additions & 7 deletions superset/annotation_layers/annotations/commands/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
8 changes: 2 additions & 6 deletions superset/annotation_layers/annotations/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ def __init__(self) -> None:
)


class AnnotationBulkDeleteFailedError(DeleteFailedError):
message = _("Annotations could not be deleted.")


class AnnotationNotFoundError(CommandException):
message = _("Annotation not found.")

Expand All @@ -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.")
13 changes: 4 additions & 9 deletions superset/annotation_layers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand All @@ -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))
54 changes: 0 additions & 54 deletions superset/annotation_layers/commands/bulk_delete.py

This file was deleted.

16 changes: 8 additions & 8 deletions superset/annotation_layers/commands/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
12 changes: 2 additions & 10 deletions superset/annotation_layers/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -456,7 +454,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()
Expand Down Expand Up @@ -516,7 +514,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(
Expand All @@ -527,7 +525,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",))
Expand Down
Loading

0 comments on commit fd66f43

Please sign in to comment.