feat: use celery instead of cron jobs for notification digest#38185
Draft
AhtishamShahid wants to merge 1 commit intomasterfrom
Draft
feat: use celery instead of cron jobs for notification digest#38185AhtishamShahid wants to merge 1 commit intomasterfrom
AhtishamShahid wants to merge 1 commit intomasterfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
feat: use celery instead of cron jobs for notification digest
AhtishamShahid
force-pushed
the
ahtisham/use_celery_for_notification_digest
branch
from
f9fb0e1 to
08402fe
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors notification digest scheduling from cron/management-command driven execution to Celery-based, deduplicated per-user scheduling backed by a new DigestSchedule model and updated time-window handling.
Changes:
- Add configurable daily/weekly digest delivery schedule settings and a
DigestSchedulemodel to dedupe scheduled digest tasks. - Trigger per-user digest scheduling from
send_notifications, and introduce a new delayed Celery task flow for digest delivery. - Deprecate the legacy
send_email_digestmanagement command and adjust digest date-range calculation to use Django timezone utilities.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| openedx/envs/common.py | Adds instance-configurable UTC delivery schedule settings for daily/weekly digests. |
| openedx/core/djangoapps/notifications/tasks.py | Schedules digest delivery when notifications are created for daily/weekly cadence users. |
| openedx/core/djangoapps/notifications/models.py | Introduces DigestSchedule model used as the dedupe source of truth for scheduled digest tasks. |
| openedx/core/djangoapps/notifications/migrations/0012_digestschedule.py | Creates the DigestSchedule table and unique constraint. |
| openedx/core/djangoapps/notifications/management/commands/send_email_digest.py | Marks the cron-driven command as deprecated and changes runtime behavior. |
| openedx/core/djangoapps/notifications/email/utils.py | Switches digest window calculations to django.utils.timezone.now() and returns aware datetimes. |
| openedx/core/djangoapps/notifications/email/tasks.py | Implements new digest scheduling/deduping logic and a per-user delayed Celery task flow. |
| openedx/core/djangoapps/notifications/email/tests/test_tasks.py | Adds/updates tests for the new digest scheduling and delivery behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+350
to
+353
|
|
||
| user = User.objects.get(id=user_id) | ||
|
|
||
| if not user.has_usable_password(): |
Comment on lines
+43
to
+56
| warnings.warn( | ||
| "The send_email_digest management command is deprecated. " | ||
| "Digest emails are now scheduled automatically via delayed Celery tasks " | ||
| "when notifications are created. Remove any cron jobs calling this command.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| self.stderr.write( | ||
| self.style.WARNING( | ||
| "WARNING: This command is deprecated. Digest emails are now scheduled " | ||
| "automatically. Please remove cron jobs using this command." | ||
| ) | ||
| ) | ||
| return |
Comment on lines
17
to
33
| from openedx.core.djangoapps.notifications.email.tasks import ( | ||
| _cleanup_digest_schedule_for_current_window, | ||
| add_to_existing_buffer, | ||
| decide_email_action, | ||
| get_audience_for_cadence_email, | ||
| get_next_digest_delivery_time, | ||
| is_digest_already_scheduled, | ||
| is_digest_already_sent_in_window, | ||
| schedule_digest_buffer, | ||
| schedule_user_digest_email, | ||
| send_buffered_digest, | ||
| send_digest_email_to_all_users, | ||
| send_digest_email_to_user, | ||
| send_immediate_cadence_email, | ||
| send_immediate_email | ||
| send_immediate_email, | ||
| send_user_digest_email_task, | ||
| ) |
Comment on lines
+227
to
+239
| window_start = delivery_time - timedelta(days=7) | ||
| else: | ||
| return False | ||
|
|
||
| return Notification.objects.filter( | ||
| user_id=user_id, | ||
| email=True, | ||
| email_sent_on__gte=window_start, | ||
| email_sent_on__lte=delivery_time, | ||
| ).exists() | ||
|
|
||
|
|
||
| def schedule_user_digest_email(user_id, cadence_type): |
Comment on lines
+193
to
+194
| return f"digest:{user_id}:{cadence_type}:{window_key}" | ||
|
|
Comment on lines
+260
to
+262
| if cadence_type not in [EmailCadence.DAILY, EmailCadence.WEEKLY]: | ||
| logger.warning(f'<Digest Schedule> Invalid cadence_type {cadence_type} for user {user_id}') | ||
| return |
Comment on lines
336
to
+340
| """ | ||
| Send email digest to all eligible users | ||
| Delayed Celery task to send a digest email to a single user. | ||
|
|
||
| This task is scheduled with apply_async(eta=...) for the configured | ||
| delivery time. When it fires: |
Comment on lines
+226
to
+233
| if digest_schedule_users and is_email_notification_flag_enabled(): | ||
| logger.info( | ||
| f"Scheduling digest emails for {len(digest_schedule_users)} users " | ||
| f"for notification {notification_type}", | ||
| ) | ||
| for uid, cadence in digest_schedule_users.items(): | ||
| try: | ||
| schedule_user_digest_email(uid, cadence) |
Comment on lines
+423
to
+426
|
|
||
| def _cleanup_digest_schedule_for_current_window(user_id, cadence_type): | ||
| """ | ||
| Remove DigestSchedule records only for the current delivery window. |
Comment on lines
+320
to
+333
| 'user_id': user_id, | ||
| 'cadence_type': cadence_type, | ||
| }, | ||
| eta=delivery_time, | ||
| task_id=task_id, | ||
| ) | ||
|
|
||
| logger.info( | ||
| f'<Digest Schedule> Scheduled {cadence_type} digest for user {user_id} ' | ||
| f'at {delivery_time} (task_id={task_id})' | ||
| ) | ||
|
|
||
|
|
||
| @shared_task(bind=True, ignore_result=True, max_retries=3, default_retry_delay=300) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request implements a significant refactor of the email digest scheduling system for notifications. The main change is the shift from using a management command and cron jobs to an automated, deduplicated, and transactional Celery-based scheduling system for daily and weekly digest emails. This ensures that digest emails are reliably scheduled and sent without duplication, even when multiple notifications arrive concurrently. The update introduces a new
DigestSchedulemodel to track scheduled digest tasks, and deprecates the old management command. Additionally, timezone handling has been improved to use Django's timezone utilities.