From 501e5431a01da6fba19b38e19f19a7bf96523441 Mon Sep 17 00:00:00 2001 From: Mel <97147377+MelissaAutumn@users.noreply.github.com> Date: Thu, 23 May 2024 08:40:16 -0700 Subject: [PATCH] Don't use db models for emails (#418) * Don't use db models for emails * Apply suggestions from code review Co-authored-by: Andreas --------- Co-authored-by: Andreas --- backend/src/appointment/controller/mailer.py | 40 ++++++++++--------- backend/src/appointment/routes/api.py | 3 +- backend/src/appointment/routes/schedule.py | 6 +-- backend/src/appointment/tasks/emails.py | 18 +++++---- .../templates/email/confirm.jinja2 | 2 +- .../templates/email/pending.jinja2 | 2 +- .../templates/email/rejected.jinja2 | 2 +- .../templates/email/support.jinja2 | 2 +- backend/test/unit/test_mailer.py | 4 +- 9 files changed, 42 insertions(+), 37 deletions(-) diff --git a/backend/src/appointment/controller/mailer.py b/backend/src/appointment/controller/mailer.py index a0b07419d..12798bfa3 100644 --- a/backend/src/appointment/controller/mailer.py +++ b/backend/src/appointment/controller/mailer.py @@ -168,10 +168,10 @@ def text(self): class ConfirmationMail(Mailer): - def __init__(self, confirm_url, deny_url, attendee, date, *args, **kwargs): + def __init__(self, confirm_url, deny_url, attendee_name, attendee_email, date, *args, **kwargs): """init Mailer with confirmation specific defaults""" - self.attendee = attendee - self.attendee.name = self.attendee.name.title() + self.attendee_name = attendee_name + self.attendee_email = attendee_email self.date = date self.confirmUrl = confirm_url self.denyUrl = deny_url @@ -182,8 +182,8 @@ def __init__(self, confirm_url, deny_url, attendee, date, *args, **kwargs): def text(self): return l10n('confirm-mail-plain', { - 'attendee_name': self.attendee.name, - 'attendee_email': self.attendee.email, + 'attendee_name': self.attendee_name, + 'attendee_email': self.attendee_email, 'date': self.date, 'confirm_url': self.confirmUrl, 'deny_url': self.denyUrl, @@ -191,7 +191,8 @@ def text(self): def html(self): return get_template("confirm.jinja2").render( - attendee=self.attendee, + attendee_name=self.attendee_name, + attendee_email=self.attendee_email, date=self.date, confirm=self.confirmUrl, deny=self.denyUrl, @@ -199,9 +200,9 @@ def html(self): class RejectionMail(Mailer): - def __init__(self, owner, date, *args, **kwargs): + def __init__(self, owner_name, date, *args, **kwargs): """init Mailer with rejection specific defaults""" - self.owner = owner + self.owner_name = owner_name self.date = date default_kwargs = { "subject": l10n('reject-mail-subject') @@ -210,18 +211,18 @@ def __init__(self, owner, date, *args, **kwargs): def text(self): return l10n('reject-mail-plain', { - 'owner_name': self.owner.name, + 'owner_name': self.owner_name, 'date': self.date }) def html(self): - return get_template("rejected.jinja2").render(owner=self.owner, date=self.date) + return get_template("rejected.jinja2").render(owner_name=self.owner_name, date=self.date) class PendingRequestMail(Mailer): - def __init__(self, owner, date, *args, **kwargs): + def __init__(self, owner_name, date, *args, **kwargs): """init Mailer with pending specific defaults""" - self.owner = owner + self.owner_name = owner_name self.date = date default_kwargs = { "subject": l10n('pending-mail-subject') @@ -230,18 +231,19 @@ def __init__(self, owner, date, *args, **kwargs): def text(self): return l10n('pending-mail-plain', { - 'owner_name': self.owner.name, + 'owner_name': self.owner_name, 'date': self.date }) def html(self): - return get_template("pending.jinja2").render(owner=self.owner, date=self.date) + return get_template("pending.jinja2").render(owner_name=self.owner_name, date=self.date) class SupportRequestMail(Mailer): - def __init__(self, requestee, topic, details, *args, **kwargs): + def __init__(self, requestee_name, requestee_email, topic, details, *args, **kwargs): """init Mailer with support specific defaults""" - self.requestee = requestee + self.requestee_name = requestee_name + self.requestee_email = requestee_email self.topic = topic self.details = details default_kwargs = { @@ -251,14 +253,14 @@ def __init__(self, requestee, topic, details, *args, **kwargs): def text(self): return l10n('support-mail-plain', { - 'requestee_name': self.requestee.name, - 'requestee_email': self.requestee.email, + 'requestee_name': self.requestee_name, + 'requestee_email': self.requestee_email, 'topic': self.topic, 'details': self.details, }) def html(self): - return get_template("support.jinja2").render(requestee=self.requestee, topic=self.topic, details=self.details) + return get_template("support.jinja2").render(requestee_name=self.requestee_name, requestee_email=self.requestee_email, topic=self.topic, details=self.details) class InviteAccountMail(Mailer): diff --git a/backend/src/appointment/routes/api.py b/backend/src/appointment/routes/api.py index 8db06f425..8637ba733 100644 --- a/backend/src/appointment/routes/api.py +++ b/backend/src/appointment/routes/api.py @@ -587,7 +587,8 @@ def send_feedback( background_tasks.add_task( send_support_email, - requestee=subscriber, + requestee_name=subscriber.name, + requestee_email=subscriber.email, topic=form_data.topic, details=form_data.details, ) diff --git a/backend/src/appointment/routes/schedule.py b/backend/src/appointment/routes/schedule.py index 8a2d5a814..05b89c741 100644 --- a/backend/src/appointment/routes/schedule.py +++ b/backend/src/appointment/routes/schedule.py @@ -254,10 +254,10 @@ def request_schedule_availability_slot( db.commit() # Sending confirmation email to owner - background_tasks.add_task(send_confirmation_email, url=url, attendee=attendee, date=date, to=subscriber.email) + background_tasks.add_task(send_confirmation_email, url=url, attendee_name=attendee.name, date=date, to=subscriber.email) # Sending pending email to attendee - background_tasks.add_task(send_pending_email, owner=subscriber, date=attendee_date, to=slot.attendee.email) + background_tasks.add_task(send_pending_email, owner_name=subscriber.name, date=attendee_date, to=slot.attendee.email) # Mini version of slot, so we can grab the newly created slot id for tests return schemas.SlotOut( @@ -321,7 +321,7 @@ def decide_on_schedule_availability_slot( date = slot.start.replace(tzinfo=timezone.utc).astimezone(ZoneInfo(subscriber.timezone)).strftime("%c") date = f"{date}, {slot.duration} minutes" # send rejection information to bookee - background_tasks.add_task(send_rejection_email, owner=subscriber, date=date, to=slot.attendee.email) + background_tasks.add_task(send_rejection_email, owner_name=subscriber.name, date=date, to=slot.attendee.email) if slot.appointment_id: # delete the appointment, this will also delete the slot. diff --git a/backend/src/appointment/tasks/emails.py b/backend/src/appointment/tasks/emails.py index 326119fc9..a313df3d0 100644 --- a/backend/src/appointment/tasks/emails.py +++ b/backend/src/appointment/tasks/emails.py @@ -7,30 +7,31 @@ def send_invite_email(to, attachment): mail.send() -def send_confirmation_email(url, attendee, date, to): +def send_confirmation_email(url, attendee_name, attendee_email, date, to): # send confirmation mail to owner mail = ConfirmationMail( f"{url}/1", f"{url}/0", - attendee, + attendee_name, + attendee_email, date, to=to ) mail.send() -def send_pending_email(owner, date, to): +def send_pending_email(owner_name, date, to): mail = PendingRequestMail( - owner=owner, + owner_name=owner_name, date=date, to=to ) mail.send() -def send_rejection_email(owner, date, to): +def send_rejection_email(owner_name, date, to): mail = RejectionMail( - owner=owner, + owner_name=owner_name, date=date, to=to ) @@ -42,9 +43,10 @@ def send_zoom_meeting_failed_email(to, appointment_title): mail.send() -def send_support_email(requestee, topic, details): +def send_support_email(requestee_name, requestee_email, topic, details): mail = SupportRequestMail( - requestee=requestee, + requestee_name=requestee_name, + requestee_email=requestee_email, topic=topic, details=details, ) diff --git a/backend/src/appointment/templates/email/confirm.jinja2 b/backend/src/appointment/templates/email/confirm.jinja2 index 0dc857772..d078cb7a3 100644 --- a/backend/src/appointment/templates/email/confirm.jinja2 +++ b/backend/src/appointment/templates/email/confirm.jinja2 @@ -1,7 +1,7 @@

- {{ l10n('confirm-mail-html-heading', {'attendee_name': attendee.name, 'attendee_email': attendee.email, 'date': date}) }} + {{ l10n('confirm-mail-html-heading', {'attendee_name': attendee_name, 'attendee_email': attendee_email, 'date': date}) }}

{{ l10n('confirm-mail-html-confirm-text') }}
diff --git a/backend/src/appointment/templates/email/pending.jinja2 b/backend/src/appointment/templates/email/pending.jinja2 index 3ccc8b86b..2a11db83f 100644 --- a/backend/src/appointment/templates/email/pending.jinja2 +++ b/backend/src/appointment/templates/email/pending.jinja2 @@ -1,7 +1,7 @@

- {{ l10n('pending-mail-html-heading', {'owner_name': owner.name, 'date': date}) }} + {{ l10n('pending-mail-html-heading', {'owner_name': owner_name, 'date': date}) }}

{% include 'includes/footer.jinja2' %} diff --git a/backend/src/appointment/templates/email/rejected.jinja2 b/backend/src/appointment/templates/email/rejected.jinja2 index 06e0a87ea..34101c73b 100644 --- a/backend/src/appointment/templates/email/rejected.jinja2 +++ b/backend/src/appointment/templates/email/rejected.jinja2 @@ -1,7 +1,7 @@

- {{ l10n('reject-mail-html-heading', {'owner_name': owner.name, 'date': date}) }} + {{ l10n('reject-mail-html-heading', {'owner_name': owner_name, 'date': date}) }}

{% include 'includes/footer.jinja2' %} diff --git a/backend/src/appointment/templates/email/support.jinja2 b/backend/src/appointment/templates/email/support.jinja2 index 50bd9263e..b0cb8697f 100644 --- a/backend/src/appointment/templates/email/support.jinja2 +++ b/backend/src/appointment/templates/email/support.jinja2 @@ -1,7 +1,7 @@

- {{ l10n('support-mail-html-heading', {'requestee_name': requestee.name, 'requestee_email': requestee.email}) }} + {{ l10n('support-mail-html-heading', {'requestee_name': requestee_name, 'requestee_email': requestee_email}) }}

{{ l10n('support-mail-html-topic', {'topic': topic}) }}
diff --git a/backend/test/unit/test_mailer.py b/backend/test/unit/test_mailer.py index edfb3a81c..2ad184c5e 100644 --- a/backend/test/unit/test_mailer.py +++ b/backend/test/unit/test_mailer.py @@ -19,7 +19,7 @@ def test_confirm(self, faker, with_l10n): now = datetime.datetime.now() attendee = schemas.AttendeeBase(email=faker.email(), name=faker.name(), timezone='Europe/Berlin') - mailer = ConfirmationMail(confirm_url, deny_url, attendee, now, to=fake_email) + mailer = ConfirmationMail(confirm_url, deny_url, attendee.name, attendee.email, now, to=fake_email) assert mailer.html() assert mailer.text() @@ -35,7 +35,7 @@ def test_reject(self, faker, with_l10n, make_pro_subscriber): now = datetime.datetime.now() fake_email = 'to@example.org' - mailer = RejectionMail(owner=subscriber, date=now, to=fake_email) + mailer = RejectionMail(owner_name=subscriber.name, date=now, to=fake_email) assert mailer.html() assert mailer.text()