From 105c46f6651de1d820bc9911bf8726fdb99076e1 Mon Sep 17 00:00:00 2001 From: Alastair Porter Date: Fri, 1 Apr 2022 19:04:39 +0200 Subject: [PATCH] Allow mail to be sent to multiple recipients Now the recipients argument must be a list, but everywhere that we use this it already was. Add some additional error checking to ensure that configuration is good before sending the message --- brainzutils/mail.py | 18 +++++++++++----- brainzutils/test/test_mail.py | 39 ++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/brainzutils/mail.py b/brainzutils/mail.py index e1d90d2..91b466f 100644 --- a/brainzutils/mail.py +++ b/brainzutils/mail.py @@ -2,12 +2,14 @@ from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText -from flask import current_app - +from typing import List import smtplib import socket -def send_mail(subject, text, recipients, attachments=None, +from flask import current_app + + +def send_mail(subject: str, text: str, recipients: List[str], attachments=None, from_name="MetaBrainz Notifications", from_addr=None, boundary=None): """This function can be used as a foundation for sending email. @@ -21,6 +23,12 @@ def send_mail(subject, text, recipients, attachments=None, from_name: Name of the sender. from_addr: Email address of the sender. """ + if not isinstance(recipients, list): + raise ValueError("recipients must be a list of email addresses") + + if 'SMTP_SERVER' not in current_app.config or 'SMTP_PORT' not in current_app.config: + raise ValueError("Flask current_app requires config items SMTP_SERVER and SMTP_PORT to be set") + if attachments is None: attachments = [] if from_addr is None: @@ -32,12 +40,12 @@ def send_mail(subject, text, recipients, attachments=None, if not recipients: return - message =MIMEMultipart() + message = MIMEMultipart() if boundary is not None: message = MIMEMultipart(boundary=boundary) - message['To']="<%s>" %(recipients) + message['To'] = ", ".join(recipients) message['Subject'] = subject message['From'] = "%s <%s>" % (from_name, from_addr) message.attach(MIMEText(text, _charset='utf-8')) diff --git a/brainzutils/test/test_mail.py b/brainzutils/test/test_mail.py index 3f762d2..8ade4e6 100644 --- a/brainzutils/test/test_mail.py +++ b/brainzutils/test/test_mail.py @@ -2,7 +2,6 @@ import smtplib from unittest import mock -from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from brainzutils import flask @@ -10,21 +9,51 @@ class MailTestCase(unittest.TestCase): + def test_send_email_missing_config(self): + app = flask.CustomFlask(__name__) + with app.app_context(): + with self.assertRaises(ValueError) as err: + mail.send_mail( + subject='ListenBrainz Spotify Importer Error', + text='It is a test mail', + recipients=[], + attachments=None, + from_name='ListenBrainz', + from_addr='noreply@metabrainz.org', + boundary='b' + ) + assert "Flask current_app requires config items" in str(err.exception) + + def test_send_email_string_recipients(self): + app = flask.CustomFlask(__name__) + with app.app_context(): + with self.assertRaises(ValueError) as err: + mail.send_mail( + subject='ListenBrainz Spotify Importer Error', + text='It is a test mail', + recipients='wrongemail@metabrainz.org', + attachments=None, + from_name='ListenBrainz', + from_addr='noreply@metabrainz.org', + boundary='b' + ) + assert str(err.exception) == "recipients must be a list of email addresses" + @mock.patch('smtplib.SMTP') def test_send_email(self, mock_smtp): app = flask.CustomFlask(__name__) app.config['SMTP_SERVER'] = 'localhost' - app.config['SMTP_PORT'] = 8080 + app.config['SMTP_PORT'] = 25 with app.app_context(): from_address = 'noreply@metabrainz.org' - recipients = 'sarthak2907@gmail.com' + recipients = ['musicbrainz@metabrainz.org', 'listenbrainz@metabrainz.org'] text = 'It is a test mail' from_name = 'ListenBrainz' subject = 'ListenBrainz Spotify Importer Error' boundary = '===============2220963697271485568==' message = MIMEMultipart(boundary=boundary) - message['To'] = '<%s>' % (recipients) + message['To'] = "musicbrainz@metabrainz.org, listenbrainz@metabrainz.org" message['Subject'] = subject message['From'] = '%s <%s>' % (from_name, from_address) message.attach(MIMEText(text, _charset='utf-8')) @@ -32,7 +61,7 @@ def test_send_email(self, mock_smtp): mail.send_mail( subject='ListenBrainz Spotify Importer Error', text='It is a test mail', - recipients='sarthak2907@gmail.com', + recipients=recipients, attachments=None, from_name='ListenBrainz', from_addr='noreply@metabrainz.org',