From 4ac51c5e670b2d25c2fbf4b9c23072a7ec47e2d2 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 22 Jun 2023 17:10:12 +0200 Subject: [PATCH 1/4] Notification: expand management command to support more use cases - Follow the convention we have in other notifications regarind the context: use `production_uri` instead of `domain - Add all the `projects` for a particular user to the context, so we can show actions specific to those projects - Use the internal `get_context_data()` and `extra_context=` to follow the pattern stablished for the `Notification` class. Besides, make the new linter to pass. --- .../management/commands/contact_owners.py | 14 ++++++--- readthedocs/core/utils/contact.py | 31 +++++++++---------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/readthedocs/core/management/commands/contact_owners.py b/readthedocs/core/management/commands/contact_owners.py index 3dfc7388b06..b58fe5f9c9f 100644 --- a/readthedocs/core/management/commands/contact_owners.py +++ b/readthedocs/core/management/commands/contact_owners.py @@ -49,7 +49,11 @@ 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`` + * ``projects`` + * ``production_uri`` .. code:: markdown @@ -57,7 +61,7 @@ class Command(BaseCommand): Dear {{ user.firstname }}, - Greetings from [Read the Docs]({{ domain }}). + Greetings from [Read the Docs]({{ production_uri }}). .. note:: @@ -143,7 +147,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 @@ -178,14 +182,14 @@ def handle(self, *args, **options): notification_content = '' if options['notification']: file = Path(options['notification']) - with file.open() as f: + 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: + with file.open(encoding="utf8") as f: content = f.read().split('\n') email_subject = content[0].strip() email_content = '\n'.join(content[1:]).strip() diff --git a/readthedocs/core/utils/contact.py b/readthedocs/core/utils/contact.py index 46cd26d7a23..2cd7150c0f0 100644 --- a/readthedocs/core/utils/contact.py +++ b/readthedocs/core/utils/contact.py @@ -7,6 +7,7 @@ from django.template import Context, Engine from messages_extends import constants as message_constants +from readthedocs.core.permissions import AdminPermission from readthedocs.notifications import SiteNotification from readthedocs.notifications.backends import SiteBackend @@ -40,7 +41,8 @@ def contact_users( { 'user': , - 'domain': https://readthedocs.org, + 'production_uri': https://readthedocs.org, + 'projects': QuerySet(), } :returns: A dictionary with a list of sent/failed emails/notifications. @@ -67,20 +69,16 @@ 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}", + "projects": AdminPermission.projects(user), } context.update(context_function(user)) @@ -88,6 +86,7 @@ def render(self, *args, **kwargs): notification = TempNotification( user=user, success=True, + extra_context=context, ) try: if not dryrun: @@ -132,13 +131,13 @@ 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: From 5afc9ff574fb25024b49cf0439d9309cf247a87d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 22 Jun 2023 18:07:02 +0200 Subject: [PATCH 2/4] Notification: change `print` by `log` `structlog` give us a better experience here. --- .../management/commands/contact_owners.py | 106 +++++++++--------- readthedocs/core/utils/contact.py | 11 +- 2 files changed, 53 insertions(+), 64 deletions(-) diff --git a/readthedocs/core/management/commands/contact_owners.py b/readthedocs/core/management/commands/contact_owners.py index b58fe5f9c9f..3fa6a999810 100644 --- a/readthedocs/core/management/commands/contact_owners.py +++ b/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 @@ -81,30 +80,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 ' @@ -159,62 +158,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']) + 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']) + email_subject = "" + email_content = "" + if options["email"]: + file = Path(options["email"]) with file.open(encoding="utf8") as f: - content = f.read().split('\n') + 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"], + ) diff --git a/readthedocs/core/utils/contact.py b/readthedocs/core/utils/contact.py index 2cd7150c0f0..67ad42658a8 100644 --- a/readthedocs/core/utils/contact.py +++ b/readthedocs/core/utils/contact.py @@ -1,5 +1,4 @@ import structlog -from pprint import pprint import markdown from django.conf import settings @@ -34,7 +33,7 @@ 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:: @@ -91,16 +90,12 @@ def render(self, *args, **kwargs): try: if not dryrun: backend.send(notification) - else: - pprint(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, @@ -140,8 +135,6 @@ def render(self, *args, **kwargs): } if not dryrun: send_mail(**kwargs) - else: - pprint(kwargs) except Exception: log.exception('Email failed to send') failed_emails.update(emails) From f6357b84493a254f98bbc7a92748aec25e4e864f Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 22 Jun 2023 18:08:15 +0200 Subject: [PATCH 3/4] Template: add a small filter useful for sending emails I'll be using this filter to send emails to projects with a particular feature flag. --- readthedocs/projects/templatetags/projects_tags.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/readthedocs/projects/templatetags/projects_tags.py b/readthedocs/projects/templatetags/projects_tags.py index dd09febeaea..cf995ed023c 100644 --- a/readthedocs/projects/templatetags/projects_tags.py +++ b/readthedocs/projects/templatetags/projects_tags.py @@ -25,3 +25,9 @@ def sort_version_aware(versions): def is_project_user(user, project): """Checks if the user has access to the project.""" return user in AdminPermission.members(project) + + +@register.filter +def has_feature(project, feature): + """Check if the project has a particular feature flag.""" + return project.has_feature(feature) From a3cccdc1c700d0115a30d35dd2c17dc650299191 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 22 Jun 2023 18:10:40 +0200 Subject: [PATCH 4/4] Notification: email to send to users I'm commiting this to the repository so we we do review and we have some history about how we send emails. I had a hard time today figure it out how we used this in the past. --- .../commands/emails/unshallow-clone-email.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 readthedocs/core/management/commands/emails/unshallow-clone-email.md diff --git a/readthedocs/core/management/commands/emails/unshallow-clone-email.md b/readthedocs/core/management/commands/emails/unshallow-clone-email.md new file mode 100644 index 00000000000..77dd45c55a8 --- /dev/null +++ b/readthedocs/core/management/commands/emails/unshallow-clone-email.md @@ -0,0 +1,23 @@ +[Action required] Unshallow your clone via the configuration file + +{% load projects_tags %} +Some time ago we added an on-request "feature flag" to allow users to unshallow their Git repository +when clonning them as part of the build process. +With the introduction of the ability to [customize the build process](https://docs.readthedocs.io/en/latest/build-customization.html) via `build.jobs` and `build.commands`, +this feature flag is not required anymore and we are deprecating it. +Now, users have the ability to unshallow the Git repository clone without contacting support. + +We are sending you this email because you are a maintainer of the following projects that have +this "feature flag" enabled and you should unshallow your clone now by using the configuration file: + +{% spaceless %} +{% for project in projects %} +{% if project|has_feature:"dont_shallow_clone" %} +* [{{ production_uri }}{{ project.get_absolute_url }}]({{ project.slug }}) +{% endif %} +{% endfor %} +{% endspaceless %} + +Note this feature flag will be completely removed on **August 28th**. +Please, refer to [the example we have in the documentation](https://docs.readthedocs.io/en/latest/build-customization.html#unshallow-git-clone) +to unshallow your clone via the configuration file if your projects still require it.