From 8eca9fb7ec680a9a42395841e74e907874b23c9b Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Thu, 21 May 2020 15:03:17 +0200 Subject: [PATCH] Simplify send_email(): don't flash messages within this function Since we have only three places that call send_email() and already several special cases, it's simpler to just check the return value and flash messages within the calling code. --- ihatemoney/utils.py | 14 ++++-------- ihatemoney/web.py | 53 +++++++++++++++++++++++++-------------------- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/ihatemoney/utils.py b/ihatemoney/utils.py index d206593b..adced8f7 100644 --- a/ihatemoney/utils.py +++ b/ihatemoney/utils.py @@ -12,7 +12,7 @@ import socket from babel import Locale from babel.numbers import get_currency_name, get_currency_symbol -from flask import current_app, flash, redirect, render_template +from flask import current_app, redirect, render_template from flask_babel import get_locale, lazy_gettext as _ import jinja2 from werkzeug.routing import HTTPException, RoutingException @@ -30,12 +30,10 @@ def slugify(value): return re.sub(r"[-\s]+", "-", value) -def send_email(mail_message, flash_success="", flash_error=""): - """Send an email using Flask-Mail, returning False if there was an error. +def send_email(mail_message): + """Send an email using Flask-Mail, with proper error handling. - Optionally display a "flash alert" message to the user. The flash - message is different depending on whether we could successfully send - the email or not. + Return True if everything went well, and False if there was an error. """ # Since Python 3.4, SMTPException and socket.error are actually # identical, but this was not the case before. Also, it is more clear @@ -43,12 +41,8 @@ def send_email(mail_message, flash_success="", flash_error=""): try: current_app.mail.send(mail_message) except (smtplib.SMTPException, socket.error): - if flash_error: - flash(flash_error, category="danger") return False # Email was sent successfully - if flash_success: - flash(flash_success, category="success") return True diff --git a/ihatemoney/web.py b/ihatemoney/web.py index ded06bd3..65757201 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -308,8 +308,12 @@ def create_project(): msg = Message( message_title, body=message_body, recipients=[project.contact_email] ) - success = send_email(msg, _("A reminder email has just been sent to you")) - if not success: + success = send_email(msg) + if success: + flash( + _("A reminder email has just been sent to you"), category="success" + ) + else: # Display the error as a simple "info" alert, because it's # not critical and doesn't prevent using the project. flash( @@ -339,19 +343,20 @@ def remind_password(): body=render_localized_template("password_reminder", project=project), recipients=[project.contact_email], ) - success = send_email( - remind_message, - "", - _( - "Sorry, there was an error while sending you an email " - "with password reset instructions. " - "Please check the email configuration of the server " - "or contact the administrator." - ), - ) + success = send_email(remind_message) if success: return redirect(url_for(".password_reminder_sent")) - + else: + flash( + _( + "Sorry, there was an error while sending you an email " + "with password reset instructions. " + "Please check the email configuration of the server " + "or contact the administrator." + ), + category="danger", + ) + # Fall-through: we stay on the same page and display the form again return render_template("password_reminder.html", form=form) @@ -598,18 +603,20 @@ def invite(): body=message_body, recipients=[email.strip() for email in form.emails.data.split(",")], ) - success = send_email( - msg, - _("Your invitations have been sent"), - _( - "Sorry, there was an error while trying to send the invitation emails. " - "Please check the email configuration of the server " - "or contact the administrator." - ), - ) + success = send_email(msg) if success: + flash(_("Your invitations have been sent"), category="success") return redirect(url_for(".list_bills")) - + else: + flash( + _( + "Sorry, there was an error while trying to send the invitation emails. " + "Please check the email configuration of the server " + "or contact the administrator." + ), + category="danger", + ) + # Fall-through: we stay on the same page and display the form again return render_template("send_invites.html", form=form)