Notification: expand management command to follow conventions (#10470)

* Notification: expand management command to follow conventions

- Follow the convention we have in other notifications regarind the context: use
  `production_uri` instead of `domain
- Use the internal `get_context_data()` and `extra_context=` to follow the
  pattern stablished for the `Notification` class.
- Make the new linter to pass.
- Use structlog instead of `print()` function

* Log (debug) rendered notification

Author: humitos

readthedocs/core/management/commands/contact_owners.py

@@ -1,6 +1,5 @@
 import sys
 from pathlib import Path
-from pprint import pprint
 
 import structlog
 from django.conf import settings
@@ -49,15 +48,18 @@ class Command(BaseCommand):
        to be included in the notification.
     * ``email.md`` is a Markdown file with the first line as the subject,
       and the rest is the content.
-      Note that ``user`` and ``domain`` are available in the context.
+
+      The context available is:
+        * ``user``
+        * ``production_uri``
 
     .. code:: markdown
 
        Read the Docs deprecated option, action required
 
        Dear {{ user.firstname }},
 
-       Greetings from [Read the Docs]({{ domain }}).
+       Greetings from [Read the Docs]({{ production_uri }}).
 
     .. note::
 
@@ -77,30 +79,30 @@ class Command(BaseCommand):
 
     def add_arguments(self, parser):
         parser.add_argument(
-            '--production',
-            action='store_true',
-            dest='production',
+            "--production",
+            action="store_true",
+            dest="production",
             default=False,
             help=(
-                'Send the email/notification for real, '
-                'otherwise we only print the notification in the console (dryrun).'
-            )
+                "Send the email/notification for real, "
+                "otherwise we only logs the notification in the console (dryrun)."
+            ),
         )
         parser.add_argument(
-            '--email',
+            "--email",
             help=(
-                'Path to a file with the email content in markdown. '
-                'The first line would be the subject.'
+                "Path to a file with the email content in markdown. "
+                "The first line would be the subject."
             ),
         )
         parser.add_argument(
-            '--notification',
-            help='Path to a file with the notification content in markdown.',
+            "--notification",
+            help="Path to a file with the notification content in markdown.",
         )
         parser.add_argument(
-            '--sticky',
-            action='store_true',
-            dest='sticky',
+            "--sticky",
+            action="store_true",
+            dest="sticky",
             default=False,
             help=(
                 'Make the notification sticky '
@@ -143,7 +145,7 @@ def handle(self, *args, **options):
             users = AdminPermission.owners(organization)
         elif usernames:
             file = Path(usernames)
-            with file.open() as f:
+            with file.open(encoding="utf8") as f:
                 usernames = f.readlines()
 
             # remove "\n" from lines
@@ -155,62 +157,59 @@ def handle(self, *args, **options):
                 organizationowner__organization__disabled=False
             ).distinct()
         else:
-            users = (
-                User.objects
-                .filter(projects__skip=False)
-                .distinct()
-            )
-
-        print(
-            "len(owners)={} production={} email={} notification={} sticky={}".format(
-                users.count(),
-                bool(options["production"]),
-                options["email"],
-                options["notification"],
-                options["sticky"],
-            )
+            users = User.objects.filter(projects__skip=False).distinct()
+
+        log.info(
+            "Command arguments.",
+            n_owners=users.count(),
+            production=bool(options["production"]),
+            email_filepath=options["email"],
+            notification_filepath=options["notification"],
+            sticky=options["sticky"],
         )
 
         if input("Continue? y/N: ") != "y":
             print("Aborting run.")
             return
 
-        notification_content = ''
-        if options['notification']:
-            file = Path(options['notification'])
-            with file.open() as f:
+        notification_content = ""
+        if options["notification"]:
+            file = Path(options["notification"])
+            with file.open(encoding="utf8") as f:
                 notification_content = f.read()
 
-        email_subject = ''
-        email_content = ''
-        if options['email']:
-            file = Path(options['email'])
-            with file.open() as f:
-                content = f.read().split('\n')
+        email_subject = ""
+        email_content = ""
+        if options["email"]:
+            file = Path(options["email"])
+            with file.open(encoding="utf8") as f:
+                content = f.read().split("\n")
             email_subject = content[0].strip()
-            email_content = '\n'.join(content[1:]).strip()
+            email_content = "\n".join(content[1:]).strip()
 
         resp = contact_users(
             users=users,
             email_subject=email_subject,
             email_content=email_content,
             notification_content=notification_content,
-            sticky_notification=options['sticky'],
-            dryrun=not options['production'],
+            sticky_notification=options["sticky"],
+            dryrun=not options["production"],
+        )
+
+        email = resp["email"]
+        log.info(
+            "Sending emails finished.",
+            total=len(email["sent"]),
+            total_failed=len(email["failed"]),
+            sent_emails=email["sent"],
+            failed_emails=email["failed"],
         )
 
-        email = resp['email']
-        total = len(email['sent'])
-        total_failed = len(email['failed'])
-        print(f'Emails sent ({total}):')
-        pprint(email['sent'])
-        print(f'Failed emails ({total_failed}):')
-        pprint(email['failed'])
-
-        notification = resp['notification']
-        total = len(notification['sent'])
-        total_failed = len(notification['failed'])
-        print(f'Notifications sent ({total})')
-        pprint(notification['sent'])
-        print(f'Failed notifications ({total_failed})')
-        pprint(notification['failed'])
+        notification = resp["notification"]
+        log.info(
+            "Sending notifications finished.",
+            total=len(notification["sent"]),
+            total_failed=len(notification["failed"]),
+            sent_notifications=notification["sent"],
+            failed_notifications=notification["failed"],
+        )

readthedocs/core/utils/contact.py

@@ -1,5 +1,4 @@
 import structlog
-from pprint import pprint
 
 import markdown
 from django.conf import settings
@@ -33,14 +32,14 @@ def contact_users(
     :param string notification_content: Content for the sticky notification (markdown)
     :param context_function: A callable that will receive an user
      and return a dict of additional context to be used in the email/notification content
-    :param bool dryrun: If `True` don't sent the email or notification, just print the content
+    :param bool dryrun: If `True` don't sent the email or notification, just logs the content
 
     The `email_content` and `notification_content` contents will be rendered using
     a template with the following context::
 
         {
             'user': <user object>,
-            'domain': https://readthedocs.org,
+            'production_uri': https://readthedocs.org,
         }
 
     :returns: A dictionary with a list of sent/failed emails/notifications.
@@ -67,41 +66,41 @@ class TempNotification(SiteNotification):
             success_level = message_constants.SUCCESS_PERSISTENT
 
         def render(self, *args, **kwargs):
-            context = {
-                'user': self.user,
-                'domain': f'https://{settings.PRODUCTION_DOMAIN}',
-            }
-            context.update(context_function(self.user))
             return markdown.markdown(
-                notification_template.render(Context(context))
+                notification_template.render(Context(self.get_context_data()))
             )
 
     total = users.count()
     for count, user in enumerate(users.iterator(), start=1):
         context = {
-            'user': user,
-            'domain': f'https://{settings.PRODUCTION_DOMAIN}',
+            "user": user,
+            "production_uri": f"https://{settings.PRODUCTION_DOMAIN}",
         }
         context.update(context_function(user))
 
         if notification_content:
             notification = TempNotification(
                 user=user,
                 success=True,
+                extra_context=context,
             )
             try:
                 if not dryrun:
                     backend.send(notification)
                 else:
-                    pprint(markdown.markdown(
-                        notification_template.render(Context(context))
-                    ))
+                    # Check we can render the notification with the context properly
+                    log.debug(
+                        "Rendered notification.",
+                        notification=markdown.markdown(
+                            notification_template.render(Context(context))
+                        ),
+                    )
             except Exception:
                 log.exception('Notification failed to send')
                 failed_notifications.add(user.username)
             else:
                 log.info(
-                    'Successfully set notification.',
+                    "Successfully sent notification.",
                     user_username=user.username,
                     count=count,
                     total=total,
@@ -132,17 +131,15 @@ def render(self, *args, **kwargs):
             )
 
             try:
-                kwargs = dict(
-                    subject=email_subject,
-                    message=email_txt_rendered,
-                    html_message=email_html_rendered,
-                    from_email=from_email,
-                    recipient_list=emails,
-                )
+                kwargs = {
+                    "subject": email_subject,
+                    "message": email_txt_rendered,
+                    "html_message": email_html_rendered,
+                    "from_email": from_email,
+                    "recipient_list": emails,
+                }
                 if not dryrun:
                     send_mail(**kwargs)
-                else:
-                    pprint(kwargs)
             except Exception:
                 log.exception('Email failed to send')
                 failed_emails.update(emails)