feat: use celery instead of cron jobs for notification digest#38185
feat: use celery instead of cron jobs for notification digest#38185AhtishamShahid wants to merge 9 commits intomasterfrom
Conversation
f9fb0e1 to
08402fe
Compare
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.
openedx/core/djangoapps/notifications/management/commands/send_email_digest.py
Outdated
Show resolved
Hide resolved
2be6343 to
a33a061
Compare
8dee45f to
d952ef0
Compare
bmtcril
left a comment
There was a problem hiding this comment.
Overall this is looking good, one quick comment. I'm not seeing where this is wired up to Celery, though? Is something external calling send_notifications?
The send notification is called on different signals, and this is handled here |
bmtcril
left a comment
There was a problem hiding this comment.
This worked for me, but the mechanism is a little hard to understand. Can you update the document to explain the new process and remove the old cron language?
feat: use celery instead of cron jobs for notification digest fix: resolved reviews fix: resolved reviews
b252abb to
29d10c4
Compare
bmtcril
left a comment
There was a problem hiding this comment.
Thanks for all of the work on this, it worked for me locally 👍 👍
Added docs and removed references for cron |
|
Thanks, this should be good to go 👍 |
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.Issue
#38136