From 36be9fbdb107414f21c4793a97d206d9a2c7cdd6 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Mon, 4 Aug 2025 16:23:30 +0530 Subject: [PATCH 1/5] feat: add ability to override middlewares for recurring nudges --- .../schedules/management/commands/__init__.py | 11 +++++++++-- openedx/core/djangoapps/schedules/tasks.py | 18 +++++++++++++----- openedx/core/lib/celery/task_utils.py | 2 +- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index bd0082f5331e..00c6b22c2026 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -35,6 +35,11 @@ def add_arguments(self, parser): type=int, help='Number of weekly emails to be sent', ) + parser.add_argument( + '--override-middlewares', + nargs='*', + help='Use these middelwares when emulating http request' + ) def handle(self, *args, **options): self.log_debug('Args = %r', options) @@ -54,14 +59,16 @@ def handle(self, *args, **options): self.log_debug('Running for site %s', site.domain) override_recipient_email = options.get('override_recipient_email') - self.send_emails(site, current_date, override_recipient_email) + override_middlewares = options.get('override_middlewares') + self.send_emails(site, current_date, override_recipient_email, override_middlewares) - def enqueue(self, day_offset, site, current_date, override_recipient_email=None): + def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares = None): self.async_send_task.enqueue( site, current_date, day_offset, override_recipient_email, + override_middlewares, ) def send_emails(self, *args, **kwargs): diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 55288d63f1a8..dec6c459ac19 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -20,6 +20,7 @@ set_custom_attribute ) from eventtracking import tracker +from importlib import import_module from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -103,7 +104,7 @@ class BinnedScheduleMessageBaseTask(ScheduleMessageBaseTask): task_instance = None @classmethod - def enqueue(cls, site, current_date, day_offset, override_recipient_email=None): # lint-amnesty, pylint: disable=missing-function-docstring + def enqueue(cls, site, current_date, day_offset, override_recipient_email=None, override_middlewares=None): # lint-amnesty, pylint: disable=missing-function-docstring set_code_owner_attribute_from_module(__name__) current_date = resolvers._get_datetime_beginning_of_day(current_date) # lint-amnesty, pylint: disable=protected-access @@ -120,6 +121,7 @@ def enqueue(cls, site, current_date, day_offset, override_recipient_email=None): day_offset, bin, override_recipient_email, + override_middlewares, ) cls.log_info('Launching task with args = %r', task_args) cls.task_instance.apply_async( @@ -128,16 +130,17 @@ def enqueue(cls, site, current_date, day_offset, override_recipient_email=None): ) def run( # lint-amnesty, pylint: disable=arguments-differ - self, site_id, target_day_str, day_offset, bin_num, override_recipient_email=None, + self, site_id, target_day_str, day_offset, bin_num, override_recipient_email=None, override_middlewares=None, ): set_code_owner_attribute_from_module(__name__) site = Site.objects.select_related('configuration').get(id=site_id) - with emulate_http_request(site=site): + middleware_classes = [self.class_from_classpath(cls) for cls in override_middlewares] if override_middlewares else None + with emulate_http_request(site=site, middleware_classes=middleware_classes) as request: msg_type = self.make_message_type(day_offset) - _annotate_for_monitoring(msg_type, site, bin_num, target_day_str, day_offset) + _annotate_for_monitoring(msg_type, request.site, bin_num, target_day_str, day_offset) return self.resolver( # lint-amnesty, pylint: disable=not-callable self.async_send_task, - site, + request.site, deserialize(target_day_str), day_offset, bin_num, @@ -146,6 +149,11 @@ def run( # lint-amnesty, pylint: disable=arguments-differ def make_message_type(self, day_offset): raise NotImplementedError + + def class_from_classpath(self, class_path): + module_name, klass = class_path.rsplit('.', 1) + module = import_module(module_name) + return getattr(module, klass) @shared_task(base=LoggedTask, ignore_result=True) diff --git a/openedx/core/lib/celery/task_utils.py b/openedx/core/lib/celery/task_utils.py index 738f074be68c..d5c323dfefd0 100644 --- a/openedx/core/lib/celery/task_utils.py +++ b/openedx/core/lib/celery/task_utils.py @@ -45,7 +45,7 @@ def emulate_http_request(site=None, user=None, middleware_classes=None): _run_method_if_implemented(middleware, 'process_request', request) try: - yield + yield request except Exception as exc: for middleware in reversed(middleware_instances): _run_method_if_implemented(middleware, 'process_exception', request, exc) From 98c8a05fc69cb861922848340def8688dc0db1d3 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Wed, 13 Aug 2025 15:57:34 +0530 Subject: [PATCH 2/5] chore: use action append for override-middlewares argument --- .../core/djangoapps/schedules/management/commands/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index 00c6b22c2026..63a8292f46a5 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -37,8 +37,8 @@ def add_arguments(self, parser): ) parser.add_argument( '--override-middlewares', - nargs='*', - help='Use these middelwares when emulating http request' + action='append', + help='Use these middlewares when emulating http requests. Provide the option multiple times to use multiple middlewares.' ) def handle(self, *args, **options): From a412bcd7103152a5a5140f60294b7f690425b011 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Wed, 13 Aug 2025 15:58:27 +0530 Subject: [PATCH 3/5] feat: add ability to run command for all sites --- .../schedules/management/commands/__init__.py | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index 63a8292f46a5..cb3ea32908f3 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -8,9 +8,11 @@ import pytz from django.contrib.sites.models import Site from django.core.management.base import BaseCommand +from django.db.models import Q from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin +ALL_SITES = "all-sites" class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): # lint-amnesty, pylint: disable=missing-class-docstring async_send_task = None # define in subclass @@ -29,7 +31,11 @@ def add_arguments(self, parser): '--override-recipient-email', help='Send all emails to this address instead of the actual recipient' ) - parser.add_argument('site_domain_name') + parser.add_argument( + 'site_domain_name', + nargs='?', + default=ALL_SITES + ) parser.add_argument( '--weeks', type=int, @@ -54,13 +60,17 @@ def handle(self, *args, **options): tzinfo=pytz.UTC ) self.log_debug('Current date = %s', current_date.isoformat()) - - site = Site.objects.get(domain__iexact=options['site_domain_name']) - self.log_debug('Running for site %s', site.domain) - override_recipient_email = options.get('override_recipient_email') override_middlewares = options.get('override_middlewares') - self.send_emails(site, current_date, override_recipient_email, override_middlewares) + + site_domain_name = options['site_domain_name'] + site_filter = Q() + if site_domain_name != ALL_SITES: + site_filter |= Q(domain__iexact=site_domain_name) + + for site in Site.objects.filter(site_filter): + self.log_debug('Running for site %s', site.domain) + self.send_emails(site, current_date, override_recipient_email, override_middlewares) def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares = None): self.async_send_task.enqueue( From 7e29c8c180de0101ddc5e4afda5246a698439115 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 14 Aug 2025 11:05:32 +0530 Subject: [PATCH 4/5] test: add test case to verify Site query filter works correctly --- .../schedules/management/commands/__init__.py | 5 +++-- .../tests/test_send_email_base_command.py | 17 ++++++++++++++++- openedx/core/djangoapps/schedules/tasks.py | 6 +++--- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index cb3ea32908f3..fb3ee8f58c5a 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -14,6 +14,7 @@ ALL_SITES = "all-sites" + class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): # lint-amnesty, pylint: disable=missing-class-docstring async_send_task = None # define in subclass @@ -44,7 +45,7 @@ def add_arguments(self, parser): parser.add_argument( '--override-middlewares', action='append', - help='Use these middlewares when emulating http requests. Provide the option multiple times to use multiple middlewares.' + help='Use these middlewares when emulating http requests.' ) def handle(self, *args, **options): @@ -72,7 +73,7 @@ def handle(self, *args, **options): self.log_debug('Running for site %s', site.domain) self.send_emails(site, current_date, override_recipient_email, override_middlewares) - def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares = None): + def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares=None): self.async_send_task.enqueue( site, current_date, diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py index 47ba67cc0999..63c7f4f0df23 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py @@ -10,8 +10,9 @@ import ddt import pytz from django.conf import settings +from django.contrib.sites.models import Site -from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, ALL_SITES from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms @@ -33,9 +34,23 @@ def test_handle(self): send_emails.assert_called_once_with( self.site, datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC), + None, None ) + def test_handle_all_sites(self): + with patch.object(self.command, 'send_emails') as send_emails: + self.command.handle(site_domain_name=ALL_SITES, date='2017-09-29') + expected_sites = Site.objects.all() + for expected_site in expected_sites: + send_emails.assert_any_call( + expected_site, + datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC), + None, + None + ) + assert send_emails.call_count == len(expected_sites) + def test_weeks_option(self): with patch.object(self.command, 'enqueue') as enqueue: self.command.handle(site_domain_name=self.site.domain, date='2017-09-29', weeks=12) diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index dec6c459ac19..7efce5d93c9e 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -134,8 +134,8 @@ def run( # lint-amnesty, pylint: disable=arguments-differ ): set_code_owner_attribute_from_module(__name__) site = Site.objects.select_related('configuration').get(id=site_id) - middleware_classes = [self.class_from_classpath(cls) for cls in override_middlewares] if override_middlewares else None - with emulate_http_request(site=site, middleware_classes=middleware_classes) as request: + middlewares = [self.class_from_classpath(cls) for cls in override_middlewares] if override_middlewares else None + with emulate_http_request(site=site, middleware_classes=middlewares) as request: msg_type = self.make_message_type(day_offset) _annotate_for_monitoring(msg_type, request.site, bin_num, target_day_str, day_offset) return self.resolver( # lint-amnesty, pylint: disable=not-callable @@ -149,7 +149,7 @@ def run( # lint-amnesty, pylint: disable=arguments-differ def make_message_type(self, day_offset): raise NotImplementedError - + def class_from_classpath(self, class_path): module_name, klass = class_path.rsplit('.', 1) module = import_module(module_name) From a58135a4c7b66673d804fdb023f150394243608f Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Fri, 15 Aug 2025 14:05:04 +0530 Subject: [PATCH 5/5] chore: address review comments --- .../schedules/management/commands/__init__.py | 29 +++++++++++-------- .../tests/test_send_email_base_command.py | 4 +-- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index fb3ee8f58c5a..0b7255976563 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -8,12 +8,9 @@ import pytz from django.contrib.sites.models import Site from django.core.management.base import BaseCommand -from django.db.models import Q from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin -ALL_SITES = "all-sites" - class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): # lint-amnesty, pylint: disable=missing-class-docstring async_send_task = None # define in subclass @@ -35,7 +32,11 @@ def add_arguments(self, parser): parser.add_argument( 'site_domain_name', nargs='?', - default=ALL_SITES + default=None, + help=( + 'Domain name for the site to use. ' + 'Do not provide a domain if you wish to run this for all sites' + ) ) parser.add_argument( '--weeks', @@ -45,7 +46,10 @@ def add_arguments(self, parser): parser.add_argument( '--override-middlewares', action='append', - help='Use these middlewares when emulating http requests.' + help=( + 'Use this middleware when emulating http requests. ' + 'To use multiple middlewares, provide this argument multiple times' + ) ) def handle(self, *args, **options): @@ -65,13 +69,14 @@ def handle(self, *args, **options): override_middlewares = options.get('override_middlewares') site_domain_name = options['site_domain_name'] - site_filter = Q() - if site_domain_name != ALL_SITES: - site_filter |= Q(domain__iexact=site_domain_name) - - for site in Site.objects.filter(site_filter): - self.log_debug('Running for site %s', site.domain) - self.send_emails(site, current_date, override_recipient_email, override_middlewares) + sites = Site.objects.filter(domain__iexact=site_domain_name) if site_domain_name else Site.objects.all() + + if sites: + for site in sites: + self.log_debug('Running for site %s', site.domain) + self.send_emails(site, current_date, override_recipient_email, override_middlewares) + else: + self.log_info("No matching site found") def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares=None): self.async_send_task.enqueue( diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py index 63c7f4f0df23..11b33d585195 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_email_base_command.py @@ -12,7 +12,7 @@ from django.conf import settings from django.contrib.sites.models import Site -from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, ALL_SITES +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms @@ -40,7 +40,7 @@ def test_handle(self): def test_handle_all_sites(self): with patch.object(self.command, 'send_emails') as send_emails: - self.command.handle(site_domain_name=ALL_SITES, date='2017-09-29') + self.command.handle(site_domain_name=None, date='2017-09-29') expected_sites = Site.objects.all() for expected_site in expected_sites: send_emails.assert_any_call(