From df6ffc7d86b7334b2a3c309318cc8020cd8e781a Mon Sep 17 00:00:00 2001 From: zorun Date: Thu, 21 May 2020 21:13:33 +0200 Subject: Improve error handling when sending emails (#595) In one case, we were not catching a family of possible exceptions (socket.error), and in the two other cases there was no error handling at all. Sending emails can easily fail if no email server is configured, so it is really necessary to handle these errors instead of crashing with a HTTP 500 error. Refactor email sending code and add proper error handling. Show alert messages that tell the user if an email was sent or if there was an error. When sending a password reminder email or inviting people by email, we don't proceed to the next step in case of error, because sending emails is the whole point of these actions. --- ihatemoney/tests/tests.py | 129 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 113 insertions(+), 16 deletions(-) (limited to 'ihatemoney/tests/tests.py') diff --git a/ihatemoney/tests/tests.py b/ihatemoney/tests/tests.py index 4cdf316..dc6c455 100644 --- a/ihatemoney/tests/tests.py +++ b/ihatemoney/tests/tests.py @@ -5,6 +5,8 @@ import io import json import os import re +import smtplib +import socket from time import sleep import unittest from unittest.mock import MagicMock, patch @@ -51,10 +53,10 @@ class BaseTestCase(TestCase): follow_redirects=True, ) - def post_project(self, name): + def post_project(self, name, follow_redirects=True): """Create a fake project""" # create the project - self.client.post( + return self.client.post( "/create", data={ "name": name, @@ -63,6 +65,7 @@ class BaseTestCase(TestCase): "contact_email": f"{name}@notmyidea.org", "default_currency": "USD", }, + follow_redirects=follow_redirects, ) def create_project(self, name): @@ -141,10 +144,15 @@ class BudgetTestCase(IhatemoneyTestCase): self.login("raclette") self.post_project("raclette") - self.client.post( - "/raclette/invite", data={"emails": "zorglub@notmyidea.org"} + resp = self.client.post( + "/raclette/invite", + data={"emails": "zorglub@notmyidea.org"}, + follow_redirects=True, ) + # success notification + self.assertIn("Your invitations have been sent", resp.data.decode("utf-8")) + self.assertEqual(len(outbox), 2) self.assertEqual(outbox[0].recipients, ["raclette@notmyidea.org"]) self.assertEqual(outbox[1].recipients, ["zorglub@notmyidea.org"]) @@ -225,7 +233,15 @@ class BudgetTestCase(IhatemoneyTestCase): self.create_project("raclette") # Get password resetting link from mail with self.app.mail.record_messages() as outbox: - self.client.post("/password-reminder", data={"id": "raclette"}) + resp = self.client.post( + "/password-reminder", data={"id": "raclette"}, follow_redirects=True + ) + # Check that we are redirected to the right page + self.assertIn( + "A link to reset your password has been sent to you", + resp.data.decode("utf-8"), + ) + # Check that an email was sent self.assertEqual(len(outbox), 1) url_start = outbox[0].body.find("You can reset it here: ") + 23 url_end = outbox[0].body.find(".\n", url_start) @@ -250,17 +266,26 @@ class BudgetTestCase(IhatemoneyTestCase): def test_project_creation(self): with self.app.test_client() as c: - # add a valid project - c.post( - "/create", - data={ - "name": "The fabulous raclette party", - "id": "raclette", - "password": "party", - "contact_email": "raclette@notmyidea.org", - "default_currency": "USD", - }, - ) + with self.app.mail.record_messages() as outbox: + # add a valid project + resp = c.post( + "/create", + data={ + "name": "The fabulous raclette party", + "id": "raclette", + "password": "party", + "contact_email": "raclette@notmyidea.org", + "default_currency": "USD", + }, + follow_redirects=True, + ) + # an email is sent to the owner with a reminder of the password + self.assertEqual(len(outbox), 1) + self.assertEqual(outbox[0].recipients, ["raclette@notmyidea.org"]) + self.assertIn( + "A reminder email has just been sent to you", + resp.data.decode("utf-8"), + ) # session is updated self.assertTrue(session["raclette"]) @@ -2265,6 +2290,78 @@ class ModelsTestCase(IhatemoneyTestCase): self.assertEqual(bill.pay_each(), pay_each_expected) +class EmailFailureTestCase(IhatemoneyTestCase): + def test_creation_email_failure_smtp(self): + self.login("raclette") + with patch.object( + self.app.mail, "send", MagicMock(side_effect=smtplib.SMTPException) + ): + resp = self.post_project("raclette") + # Check that an error message is displayed + self.assertIn( + "We tried to send you an reminder email, but there was an error", + resp.data.decode("utf-8"), + ) + # Check that we were redirected to the home page anyway + self.assertIn( + 'You probably want to %s<\/em>' % string -- cgit v1.1