From b80e56a98865787027a40e8dfcd60093cf2d078f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 30 Jul 2024 18:00:24 +0200 Subject: [PATCH] Move set default mailbox to settings (#2173) --- app/dashboard/views/mailbox.py | 6 +- app/mailbox_utils.py | 34 ++++------- app/user_settings.py | 25 +++++++- tests/test_mailbox_utils.py | 38 ------------ .../user_settings/test_set_default_mailbox.py | 61 +++++++++++++++++++ 5 files changed, 100 insertions(+), 64 deletions(-) create mode 100644 tests/user_settings/test_set_default_mailbox.py diff --git a/app/dashboard/views/mailbox.py b/app/dashboard/views/mailbox.py index 7e7fa26f..bf0ca832 100644 --- a/app/dashboard/views/mailbox.py +++ b/app/dashboard/views/mailbox.py @@ -9,7 +9,7 @@ from itsdangerous import TimestampSigner from wtforms import validators, IntegerField from wtforms.fields.html5 import EmailField -from app import parallel_limiter, mailbox_utils +from app import parallel_limiter, mailbox_utils, user_settings from app.config import MAILBOX_SECRET from app.dashboard.base import dashboard_bp from app.db import Session @@ -72,8 +72,8 @@ def mailbox_route(): return redirect(request.url) try: mailbox_id = request.form.get("mailbox_id") - mailbox = mailbox_utils.set_default_mailbox(current_user, mailbox_id) - except mailbox_utils.MailboxError as e: + mailbox = user_settings.set_default_mailbox(current_user, mailbox_id) + except user_settings.CannotSetMailbox as e: flash(e.msg, "warning") return redirect(url_for("dashboard.mailbox_route")) diff --git a/app/mailbox_utils.py b/app/mailbox_utils.py index 94f9695c..c4a00ebe 100644 --- a/app/mailbox_utils.py +++ b/app/mailbox_utils.py @@ -32,16 +32,24 @@ def create_mailbox( send_verification_link: bool = True, ) -> Mailbox: if not user.is_premium(): - LOG.i(f"User {user} has created mailbox with {email} but is not premium") + LOG.i( + f"User {user} has tried to create mailbox with {email} but is not premium" + ) raise MailboxError("Only premium plan can add additional mailbox") if not is_valid_email(email): - LOG.i(f"User {user} has created mailbox with {email} but is not valid email") + LOG.i( + f"User {user} has tried to create mailbox with {email} but is not valid email" + ) raise MailboxError("Invalid email") elif mailbox_already_used(email, user): - LOG.i(f"User {user} has created mailbox with {email} but email is already used") + LOG.i( + f"User {user} has tried to create mailbox with {email} but email is already used" + ) raise MailboxError("Email already used") elif not email_can_be_used_as_mailbox(email): - LOG.i(f"User {user} has created mailbox with {email} but email is invalid") + LOG.i( + f"User {user} has tried to create mailbox with {email} but email is invalid" + ) raise MailboxError("Invalid email") new_mailbox = Mailbox.create(email=email, user_id=user.id, commit=True) @@ -109,24 +117,6 @@ def delete_mailbox( return mailbox -def set_default_mailbox(user: User, mailbox_id: int) -> Mailbox: - mailbox = Mailbox.get(mailbox_id) - - if not mailbox or mailbox.user_id != user.id: - raise MailboxError("Invalid mailbox") - - if not mailbox.verified: - raise MailboxError("This is mailbox is not verified") - - if mailbox.id == user.default_mailbox_id: - return mailbox - LOG.i(f"User {user} has set mailbox {mailbox} as his default one") - - user.default_mailbox_id = mailbox.id - Session.commit() - return mailbox - - def clear_activation_codes_for_mailbox(mailbox: Mailbox): Session.query(MailboxActivation).filter( MailboxActivation.mailbox_id == mailbox.id diff --git a/app/user_settings.py b/app/user_settings.py index f9bbb967..cef224dd 100644 --- a/app/user_settings.py +++ b/app/user_settings.py @@ -2,7 +2,7 @@ from typing import Optional from app.db import Session from app.log import LOG -from app.models import User, SLDomain, CustomDomain +from app.models import User, SLDomain, CustomDomain, Mailbox class CannotSetAlias(Exception): @@ -10,6 +10,11 @@ class CannotSetAlias(Exception): self.msg = msg +class CannotSetMailbox(Exception): + def __init__(self, msg: str): + self.msg = msg + + def set_default_alias_domain(user: User, domain_name: Optional[str]): if domain_name is None: LOG.i(f"User {user} has set no domain as default domain") @@ -45,3 +50,21 @@ def set_default_alias_domain(user: User, domain_name: Optional[str]): user.default_alias_public_domain_id = None user.default_alias_custom_domain_id = custom_domain.id Session.flush() + + +def set_default_mailbox(user: User, mailbox_id: int) -> Mailbox: + mailbox = Mailbox.get(mailbox_id) + + if not mailbox or mailbox.user_id != user.id: + raise CannotSetMailbox("Invalid mailbox") + + if not mailbox.verified: + raise CannotSetMailbox("This is mailbox is not verified") + + if mailbox.id == user.default_mailbox_id: + return mailbox + LOG.i(f"User {user} has set mailbox {mailbox} as his default one") + + user.default_mailbox_id = mailbox.id + Session.commit() + return mailbox diff --git a/tests/test_mailbox_utils.py b/tests/test_mailbox_utils.py index 1596981a..03560edb 100644 --- a/tests/test_mailbox_utils.py +++ b/tests/test_mailbox_utils.py @@ -176,44 +176,6 @@ def test_delete_with_transfer(): assert job.payload["transfer_mailbox_id"] is None -def test_set_default_mailbox(): - other = create_new_user() - mailbox = mailbox_utils.create_mailbox( - other, - random_email(), - use_digit_codes=True, - send_verification_link=False, - ) - mailbox.verified = True - Session.commit() - mailbox_utils.set_default_mailbox(other, mailbox.id) - other = User.get(other.id) - assert other.default_mailbox_id == mailbox.id - - -def test_cannot_set_unverified(): - mailbox = mailbox_utils.create_mailbox( - user, - random_email(), - use_digit_codes=True, - send_verification_link=False, - ) - with pytest.raises(mailbox_utils.MailboxError): - mailbox_utils.set_default_mailbox(user, mailbox.id) - - -def test_cannot_default_other_user_mailbox(): - other = create_new_user() - mailbox = mailbox_utils.create_mailbox( - other, - random_email(), - use_digit_codes=True, - send_verification_link=False, - ) - with pytest.raises(mailbox_utils.MailboxError): - mailbox_utils.set_default_mailbox(user, mailbox.id) - - def test_verify_non_existing_mailbox(): with pytest.raises(mailbox_utils.MailboxError): mailbox_utils.verify_mailbox_code(user, 999999999, "9999999") diff --git a/tests/user_settings/test_set_default_mailbox.py b/tests/user_settings/test_set_default_mailbox.py new file mode 100644 index 00000000..9eaf8610 --- /dev/null +++ b/tests/user_settings/test_set_default_mailbox.py @@ -0,0 +1,61 @@ +from typing import Optional +import pytest + +from app import mailbox_utils, user_settings, config +from app.db import Session +from app.models import User +from utils import create_new_user, random_email + + +user: Optional[User] = None + + +def setup_module(): + global user + config.SKIP_MX_LOOKUP_ON_CHECK = True + user = create_new_user() + user.trial_end = None + user.lifetime = True + Session.commit() + + +def teardown_module(): + config.SKIP_MX_LOOKUP_ON_CHECK = False # noqa: F821 + + +def test_set_default_mailbox(): + other = create_new_user() + mailbox = mailbox_utils.create_mailbox( + other, + random_email(), + use_digit_codes=True, + send_verification_link=False, + ) + mailbox.verified = True + Session.commit() + user_settings.set_default_mailbox(other, mailbox.id) + other = User.get(other.id) + assert other.default_mailbox_id == mailbox.id + + +def test_cannot_set_unverified(): + mailbox = mailbox_utils.create_mailbox( + user, + random_email(), + use_digit_codes=True, + send_verification_link=False, + ) + with pytest.raises(user_settings.CannotSetMailbox): + user_settings.set_default_mailbox(user, mailbox.id) + + +def test_cannot_default_other_user_mailbox(): + other = create_new_user() + mailbox = mailbox_utils.create_mailbox( + other, + random_email(), + use_digit_codes=True, + send_verification_link=False, + ) + with pytest.raises(user_settings.CannotSetMailbox): + user_settings.set_default_mailbox(user, mailbox.id)