From f05f01bf77d0ce71667467132a505fe8b5a3adf1 Mon Sep 17 00:00:00 2001 From: Carlos Quintana <74399022+cquintana92@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:39:18 +0200 Subject: [PATCH] chore: QOL improvements on alias delete due to cascade FKs (#2144) --- app/alias_utils.py | 8 +++- app/api/views/alias.py | 4 +- app/dashboard/views/index.py | 5 ++- app/models.py | 38 ++++++++++++++++++- .../versions/2024_070516_d608b8e48082_.py | 31 +++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 migrations/versions/2024_070516_d608b8e48082_.py diff --git a/app/alias_utils.py b/app/alias_utils.py index 64bae459..e0aeb6ae 100644 --- a/app/alias_utils.py +++ b/app/alias_utils.py @@ -34,6 +34,7 @@ from app.events.generated.event_pb2 import ( from app.log import LOG from app.models import ( Alias, + AliasDeleteReason, CustomDomain, Directory, User, @@ -309,7 +310,9 @@ def try_auto_create_via_domain(address: str) -> Optional[Alias]: return None -def delete_alias(alias: Alias, user: User): +def delete_alias( + alias: Alias, user: User, reason: AliasDeleteReason = AliasDeleteReason.Unspecified +): """ Delete an alias and add it to either global or domain trash Should be used instead of Alias.delete, DomainDeletedAlias.create, DeletedAlias.create @@ -324,6 +327,7 @@ def delete_alias(alias: Alias, user: User): user_id=user.id, email=alias.email, domain_id=alias.custom_domain_id, + reason=reason, ) Session.add(domain_deleted_alias) Session.commit() @@ -332,7 +336,7 @@ def delete_alias(alias: Alias, user: User): ) else: if not DeletedAlias.get_by(email=alias.email): - deleted_alias = DeletedAlias(email=alias.email) + deleted_alias = DeletedAlias(email=alias.email, reason=reason) Session.add(deleted_alias) Session.commit() LOG.i(f"Moving {alias} to global trash {deleted_alias}") diff --git a/app/api/views/alias.py b/app/api/views/alias.py index 50f455dc..c2db060c 100644 --- a/app/api/views/alias.py +++ b/app/api/views/alias.py @@ -26,7 +26,7 @@ from app.errors import ( ) from app.extensions import limiter from app.log import LOG -from app.models import Alias, Contact, Mailbox, AliasMailbox +from app.models import Alias, Contact, Mailbox, AliasMailbox, AliasDeleteReason @deprecated @@ -161,7 +161,7 @@ def delete_alias(alias_id): if not alias or alias.user_id != user.id: return jsonify(error="Forbidden"), 403 - alias_utils.delete_alias(alias, user) + alias_utils.delete_alias(alias, user, AliasDeleteReason.ManualAction) return jsonify(deleted=True), 200 diff --git a/app/dashboard/views/index.py b/app/dashboard/views/index.py index a5eaed7f..a9a4f263 100644 --- a/app/dashboard/views/index.py +++ b/app/dashboard/views/index.py @@ -12,6 +12,7 @@ from app.extensions import limiter from app.log import LOG from app.models import ( Alias, + AliasDeleteReason, AliasGeneratorEnum, User, EmailLog, @@ -143,7 +144,9 @@ def index(): if request.form.get("form-name") == "delete-alias": LOG.i(f"User {current_user} requested deletion of alias {alias}") email = alias.email - alias_utils.delete_alias(alias, current_user) + alias_utils.delete_alias( + alias, current_user, AliasDeleteReason.ManualAction + ) flash(f"Alias {email} has been deleted", "success") elif request.form.get("form-name") == "disable-alias": alias_utils.change_alias_status(alias, enabled=False) diff --git a/app/models.py b/app/models.py index 3f9952bd..bfbc8375 100644 --- a/app/models.py +++ b/app/models.py @@ -263,6 +263,15 @@ class UnsubscribeBehaviourEnum(EnumE): PreserveOriginal = 2 +class AliasDeleteReason(EnumE): + Unspecified = 0 + UserHasBeenDeleted = 1 + ManualAction = 2 + DirectoryDeleted = 3 + MailboxDeleted = 4 + CustomDomainDeleted = 5 + + class IntEnumType(sa.types.TypeDecorator): impl = sa.Integer @@ -667,6 +676,12 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle): user: User = cls.get(obj_id) EventDispatcher.send_event(user, EventContent(user_deleted=UserDeleted())) + # Manually delete all aliases for the user that is about to be deleted + from app.alias_utils import delete_alias + + for alias in Alias.filter_by(user_id=user.id): + delete_alias(alias, user, AliasDeleteReason.UserHasBeenDeleted) + res = super(User, cls).delete(obj_id) if commit: Session.commit() @@ -2261,6 +2276,12 @@ class DeletedAlias(Base, ModelMixin): __tablename__ = "deleted_alias" email = sa.Column(sa.String(256), unique=True, nullable=False) + reason = sa.Column( + IntEnumType(AliasDeleteReason), + nullable=False, + default=AliasDeleteReason.Unspecified, + server_default=str(AliasDeleteReason.Unspecified.value), + ) @classmethod def create(cls, **kw): @@ -2448,6 +2469,13 @@ class CustomDomain(Base, ModelMixin): if obj.is_sl_subdomain: DeletedSubdomain.create(domain=obj.domain) + from app import alias_utils + + for alias in Alias.filter_by(custom_domain_id=obj_id): + alias_utils.delete_alias( + alias, obj.user, AliasDeleteReason.CustomDomainDeleted + ) + return super(CustomDomain, cls).delete(obj_id) @property @@ -2520,6 +2548,12 @@ class DomainDeletedAlias(Base, ModelMixin): domain = orm.relationship(CustomDomain) user = orm.relationship(User, foreign_keys=[user_id]) + reason = sa.Column( + IntEnumType(AliasDeleteReason), + nullable=False, + default=AliasDeleteReason.Unspecified, + server_default=str(AliasDeleteReason.Unspecified.value), + ) @classmethod def create(cls, **kw): @@ -2611,7 +2645,7 @@ class Directory(Base, ModelMixin): for alias in Alias.filter_by(directory_id=obj_id): from app import alias_utils - alias_utils.delete_alias(alias, user) + alias_utils.delete_alias(alias, user, AliasDeleteReason.DirectoryDeleted) DeletedDirectory.create(name=obj.name) cls.filter(cls.id == obj_id).delete() @@ -2739,7 +2773,7 @@ class Mailbox(Base, ModelMixin): from app import alias_utils # only put aliases that have mailbox as a single mailbox into trash - alias_utils.delete_alias(alias, user) + alias_utils.delete_alias(alias, user, AliasDeleteReason.MailboxDeleted) Session.commit() cls.filter(cls.id == obj_id).delete() diff --git a/migrations/versions/2024_070516_d608b8e48082_.py b/migrations/versions/2024_070516_d608b8e48082_.py new file mode 100644 index 00000000..0e46c9fd --- /dev/null +++ b/migrations/versions/2024_070516_d608b8e48082_.py @@ -0,0 +1,31 @@ +"""empty message + +Revision ID: d608b8e48082 +Revises: 06a9a7133445 +Create Date: 2024-07-05 16:56:04.220173 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'd608b8e48082' +down_revision = '06a9a7133445' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('deleted_alias', sa.Column('reason', sa.Integer(), default=0, server_default='0', nullable=False)) + op.add_column('domain_deleted_alias', sa.Column('reason', sa.Integer(), default=0, server_default='0', nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('domain_deleted_alias', 'reason') + op.drop_column('deleted_alias', 'reason') + # ### end Alembic commands ###