PR#400: [frontend] refactor mailing code - copr/copr

copr / copr

#400 [frontend] refactor mailing code

Merged 5 years ago by clime. Opened 5 years ago by frostyx.

copr/ frostyx/copr mail-messages into master

[frontend] refactor mailing code

Jakub Kadlčík • 5 years ago

b96e8e7

frontend/coprs_frontend/coprs/mail.py

file added

+89

		`@@ -0,0 +1,89 @@`
		`+ import flask`
		`+ import platform`
		`+ import smtplib`
		`+ from email.mime.text import MIMEText`
		`+ from coprs import helpers`
		`+`
		`+`
		`+ class Message(object):`
		`+ subject = None`
		`+ text = None`
		`+`
		`+`
		`+ class PermissionRequestMessage(Message):`
		`+ def __init__(self, copr, applicant, permission_dict):`
		`+ """`
		`+ :param models.Copr copr:`
		`+ :param models.User applicant: object of a user that applies for new permissions (e.g. flask.g.user)`
		`+ :param models.CoprPermission permission: permission object`
		`+ :param dict permission_dict: {"old_builder": int, "old_admin": int, "new_builder": int, "new_admin": int}`
		`+ """`
		`+ self.subject = "[Copr] {0}: {1} is asking permissions".format(copr.name, applicant.name)`
		`+ self.text = ("{0} is asking for these permissions:\n\n"`
		`+ "Builder: {1} -> {2}\n"`
		`+ "Admin: {3} -> {4}\n\n"`
		`+ "Project: {5}\n"`
		`+ "Owner: {6}".format(`
		`+ applicant.name,`
		`+ helpers.PermissionEnum(permission_dict.get("old_builder", 0)),`
		`+ helpers.PermissionEnum(permission_dict.get("new_builder")),`
		`+ helpers.PermissionEnum(permission_dict.get("old_admin", 0)),`
		`+ helpers.PermissionEnum(permission_dict.get("new_admin")),`
		`+ copr.name,`
		`+ copr.owner_name))`
		`+`
		`+`
		`+ class PermissionChangeMessage(Message):`
		`+ def __init__(self, copr, permission_dict):`
		`+ """`
		`+ :param models.Copr copr:`
		`+ :param dict permission_dict: {"old_builder": int, "old_admin": int, "new_builder": int, "new_admin": int}`
		`+ """`
		`+ self.subject = "[Copr] {0}: Your permissions have changed".format(copr.name)`
		`+ self.text = (`
		`+ "Your permissions have changed:\n\n"`
		`+ "Builder: {0} -> {1}\n"`
		`+ "Admin: {2} -> {3}\n\n"`
		`+ "Project: {4}\n"`
		`+ "Owner: {5}".format(`
		`+ helpers.PermissionEnum(permission_dict["old_builder"]),`
		`+ helpers.PermissionEnum(permission_dict["new_builder"]),`
		`+ helpers.PermissionEnum(permission_dict["old_admin"]),`
		`+ helpers.PermissionEnum(permission_dict["new_admin"]),`
		`+ copr.name, copr.user.name))`
		`+`
		`+`
		`+ class LegalFlagMessage(Message):`
		`+ def __init__(self, copr, reporter, reason):`
		`+ """`
		`+ :param models.Copr copr:`
		`+ :param models.User reporter: A person who reported the legal issue (e.g. flask.g.user)`
		`+ :param str reason: What is the legal issue?`
		`+ """`
		`+ self.subject = "Legal flag raised on {0}".format(copr.name)`
		`+ self.text = ("{0}\n"`
		`+ "Navigate to {1}\n"`
		`+ "Contact on owner is: {2} <{3}>\n"`
		`+ "Reported by {4} <{5}>".format(`
		`+ reason,`
		`+ flask.url_for("admin_ns.legal_flag", _external=True),`
		`+ copr.user.username,`
		`+ copr.user.mail,`
		`+ reporter.name,`
		`+ reporter.mail))`
		`+`
		`+`
		`+ def send_mail(recipient, message, sender=None):`
		`+ """`
		`+ :param str/list recipient: One recipient email as a string or multiple emails in a list`
		`+ :param Message message:`
		`+ :param str sender: Email of a sender`
		`+ :return:`
		`+ """`
		`+ msg = MIMEText(message.text)`
		`+ msg["Subject"] = message.subject`
		`+ msg["From"] = sender or "root@{0}".format(platform.node())`
		`+ msg["To"] = ", ".join(recipient) if type(recipient) == list else recipient`
		`+ s = smtplib.SMTP("localhost")`
		`+ s.sendmail("root@{0}".format(platform.node()), recipient, msg.as_string())`
		`+ s.quit()`

frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py

file modified

+15 -54

		`@@ -36,6 +36,7 @@`
		`from coprs.logic.stat_logic import CounterStatLogic`
		`from coprs.logic.modules_logic import ModulesLogic, ModulemdGenerator, ModuleBuildFacade`
		`from coprs.rmodels import TimedStatEvents`
		`+ from coprs.mail import send_mail, LegalFlagMessage, PermissionRequestMessage, PermissionChangeMessage`

		`from coprs.logic.complex_logic import ComplexLogic`

		`@@ -564,22 +565,10 @@`
		`# sending emails`
		`if flask.current_app.config.get("SEND_EMAILS", False):`
		`for mail in admin_mails:`
		`- msg = MIMEText(`
		`- "{6} is asking for these permissions:\n\n"`
		`- "Builder: {0} -> {1}\nAdmin: {2} -> {3}\n\n"`
		`- "Project: {4}\nOwner: {5}".format(`
		`- helpers.PermissionEnum(old_builder),`
		`- helpers.PermissionEnum(new_builder),`
		`- helpers.PermissionEnum(old_admin),`
		`- helpers.PermissionEnum(new_admin),`
		`- copr.name, copr.user.name, flask.g.user.name))`
		`-`
		`- msg["Subject"] = "[Copr] {0}: {1} is asking permissions".format(copr.name, flask.g.user.name)`
		`- msg["From"] = "root@{0}".format(platform.node())`
		`- msg["To"] = mail`
		`- s = smtplib.SMTP("localhost")`
		`- s.sendmail("root@{0}".format(platform.node()), mail, msg.as_string())`
		`- s.quit()`
		`+ permission_dict = {"old_builder": old_builder, "old_admin": old_admin,`
		`+ "new_builder": new_builder, "new_admin": new_admin}`
		`+ msg = PermissionRequestMessage(copr, flask.g.user, permission_dict)`
		`+ send_mail(mail, msg, )`

		`return flask.redirect(flask.url_for("coprs_ns.copr_detail",`
		`username=copr.user.name,`
		`@@ -612,23 +601,10 @@`
		`flask.g.user, copr, perm, new_builder, new_admin)`
		`if flask.current_app.config.get("SEND_EMAILS", False) and \`
		`(old_builder is not new_builder or old_admin is not new_admin):`
		`-`
		`- msg = MIMEText(`
		`- "Your permissions have changed:\n\n"`
		`- "Builder: {0} -> {1}\nAdmin: {2} -> {3}\n\n"`
		`- "Project: {4}\nOwner: {5}".format(`
		`- helpers.PermissionEnum(old_builder),`
		`- helpers.PermissionEnum(new_builder),`
		`- helpers.PermissionEnum(old_admin),`
		`- helpers.PermissionEnum(new_admin),`
		`- copr.name, copr.user.name))`
		`-`
		`- msg["Subject"] = "[Copr] {0}: Your permissions have changed".format(copr.name)`
		`- msg["From"] = "root@{0}".format(platform.node())`
		`- msg["To"] = perm.user.mail`
		`- s = smtplib.SMTP("localhost")`
		`- s.sendmail("root@{0}".format(platform.node()), perm.user.mail, msg.as_string())`
		`- s.quit()`
		`+ permission_dict = {"old_builder": old_builder, "old_admin": old_admin,`
		`+ "new_builder": new_builder, "new_admin": new_admin}`
		`+ msg = PermissionChangeMessage(copr, permission_dict)`
		`+ send_mail(perm.user.mail, msg)`
		`# for now, we don't check for actions here, as permissions operation`
		`# don't collide with any actions`
		`except exceptions.InsufficientRightsException as e:`
		`@@ -695,11 +671,10 @@`
		`@login_required`
		`@req_with_copr`
		`def copr_legal_flag(copr):`
		`- contact_info = "{} <>".format(copr.user.username, copr.user.mail)`
		`- return process_legal_flag(contact_info, copr)`
		`+ return process_legal_flag(copr)`


		`- def process_legal_flag(contact_info, copr):`
		`+ def process_legal_flag(copr):`
		`form = forms.CoprLegalFlagForm()`
		`legal_flag = models.LegalFlag(raise_message=form.comment.data,`
		`raised_on=int(time.time()),`
		`@@ -707,25 +682,11 @@`
		`reporter=flask.g.user)`
		`db.session.add(legal_flag)`
		`db.session.commit()`
		`+`
		`send_to = app.config["SEND_LEGAL_TO"] or ["root@localhost"]`
		`- hostname = platform.node()`
		`- navigate_to = "\nNavigate to http://{0}{1}".format(`
		`- hostname, flask.url_for("admin_ns.legal_flag"))`
		`- contact = "\nContact on owner is: {}".format(contact_info)`
		`- reported_by = "\nReported by {0} <{1}>".format(flask.g.user.name,`
		`- flask.g.user.mail)`
		`- try:`
		`- msg = MIMEText(`
		`- form.comment.data + navigate_to + contact + reported_by, "plain")`
		`- except UnicodeEncodeError:`
		`- msg = MIMEText(form.comment.data.encode(`
		`- "utf-8") + navigate_to + contact + reported_by, "plain", "utf-8")`
		`- msg["Subject"] = "Legal flag raised on {0}".format(copr.name)`
		`- msg["From"] = "root@{0}".format(hostname)`
		`- msg["To"] = ", ".join(send_to)`
		`- s = smtplib.SMTP("localhost")`
		`- s.sendmail("root@{0}".format(hostname), send_to, msg.as_string())`
		`- s.quit()`
		`+ msg = LegalFlagMessage(copr, flask.g.user, form.comment.data)`
		`+ send_mail(send_to, msg)`
		`+`
		`flask.flash("Admin has been noticed about your report"`
		`" and will investigate the project shortly.")`
		`return flask.redirect(url_for_copr_details(copr))`

frontend/coprs_frontend/tests/test_mail.py

file added

+33

		`@@ -0,0 +1,33 @@`
		`+ from coprs.mail import PermissionRequestMessage, PermissionChangeMessage, LegalFlagMessage`
		`+ from tests.coprs_test_case import CoprsTestCase`
		`+ from coprs import app`
		`+`
		`+`
		`+ class TestMail(CoprsTestCase):`
		`+ def test_permissions_request_message(self, f_users, f_coprs, f_copr_permissions, f_db):`
		`+ msg = PermissionRequestMessage(self.c1, self.u2, {"new_builder": 1, "new_admin": 0})`
		`+ assert msg.subject == "[Copr] foocopr: user2 is asking permissions"`
		`+ assert msg.text == ("user2 is asking for these permissions:\n\n"`
		`+ "Builder: nothing -> request\n"`
		`+ "Admin: nothing -> nothing\n\n"`
		`+ "Project: foocopr\n"`
		`+ "Owner: user1")`
		`+`
		`+ def test_permissions_change_message(self, f_users, f_coprs, f_copr_permissions, f_db):`
		`+ msg = PermissionChangeMessage(self.c1, {"old_builder": 0, "old_admin": 2, "new_builder": 2, "new_admin": 0})`
		`+ assert msg.subject == "[Copr] foocopr: Your permissions have changed"`
		`+ assert msg.text == ("Your permissions have changed:\n\n"`
		`+ "Builder: nothing -> approved\n"`
		`+ "Admin: approved -> nothing\n\n"`
		`+ "Project: foocopr\n"`
		`+ "Owner: user1")`
		`+`
		`+ def test_legal_flag_message(self, f_users, f_coprs, f_db):`
		`+ app.config["SERVER_NAME"] = "localhost"`
		`+ with app.app_context():`
		`+ msg = LegalFlagMessage(self.c1, self.u2, "There are forbidden packages in the project")`
		`+ assert msg.subject == "Legal flag raised on foocopr"`
		`+ assert msg.text == ("There are forbidden packages in the project\n"`
		`+ "Navigate to http://localhost/admin/legal-flag/\n"`
		`+ "Contact on owner is: user1 <user1@foo.bar>\n"`
		`+ "Reported by user2 <user2@spam.foo>")`

frostyx commented 5 years ago

I was about to implement a feature for notifying people, that they have repos for outdated chroots and whether they want to preserve them ... so I examined the current code, that we use for sending emails, in order to do it the same way.

IMHO the code is awfully hard to read, it is not testable and implementing another message basically means copy-pasting a big chunk of code. Therefore I decided to refactor it first.