feat: scope immediate notification email batching per user#38866
feat: scope immediate notification email batching per user#38866AhtishamShahid wants to merge 1 commit into
Conversation
Immediate-email batching was scoped by user AND course_id, so a learner enrolled in multiple active courses received a separate buffered digest per course. Notification preferences are now account-level, so batching should feel account-level from the learner's perspective too. Scope the buffer by user only in decide_email_action, schedule_digest_buffer and send_buffered_digest: the first eligible notification is sent immediately, and additional immediate notifications within NOTIFICATION_IMMEDIATE_EMAIL_BUFFER_MINUTES are grouped into a single digest regardless of which course generated them. The digest already resolves per-notification course names, so multi-course digests render unchanged. course_key is retained in send_buffered_digest's signature so digest tasks enqueued before the change still deserialize. Closes openedx/tutor-contrib-platform-notifications#66
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @AhtishamShahid! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Pull request overview
This PR changes immediate-email notification batching to be per user (instead of per user + course), so that multiple immediate notifications across different courses within the buffer window are combined into a single buffered digest email.
Changes:
- Removed
course_id=course_keyscoping from the immediate-email buffer decision, last-sent lookup, and buffered-digest collection queries. - Updated/improved docstrings/comments to reflect the new per-user batching semantics while retaining
course_keyin task signatures for rollout compatibility. - Updated and added tests to assert cross-course buffering and digest pooling behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openedx/core/djangoapps/notifications/email/tasks.py | Drops course scoping from immediate-email buffering/digest queries to batch across all courses per user. |
| openedx/core/djangoapps/notifications/email/tests/test_tasks.py | Updates expectations for shared per-user buffering and adds coverage for cross-course digest collection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| buffer_minutes = get_buffer_minutes() | ||
| buffer_threshold = datetime.now() - timedelta(minutes=buffer_minutes) | ||
|
|
||
| # Use select_for_update to prevent race conditions | ||
| # Use select_for_update to prevent race conditions. | ||
| # Scoped by user only so all of the user's courses share one buffer window. | ||
| recent_notifications = Notification.objects.select_for_update().filter( | ||
| user=user, | ||
| course_id=course_key, | ||
| created__gte=buffer_threshold | ||
| ) |
Description
Immediate-email notification batching was scoped by user and
course_id. A learner enrolled inseveral active courses (e.g. an instructor) therefore received a separate buffered digest per course:
an immediate notification in Course A, followed by one in Course B within the buffer window, was treated
as a brand-new "first" notification for Course B and sent immediately, producing multiple emails.
Notification preferences are now account-level (configured by type/app in Account Settings, not per
course), so batching should feel account-level too. This change scopes the immediate-email buffer by
user only:
NOTIFICATION_IMMEDIATE_EMAIL_BUFFER_MINUTESaregrouped into one buffered digest, regardless of which enrolled course generated them.
What changed
openedx/core/djangoapps/notifications/email/tasks.py— dropped thecourse_id=course_keyfilter fromthe three buffer queries:
decide_email_action— the "recent email / existing buffer" check now spans all of the user's courses.schedule_digest_buffer— thelast_sentlookup is no longer course-scoped, so the digest windowstarts at the user's most recent send.
send_buffered_digest— pools all of the user's scheduled notifications; per-notification course namesare already resolved in
create_email_digest_context, so multi-course digests render unchanged (notemplate changes needed).
course_keyis intentionally retained insend_buffered_digest's signature so digest tasks enqueuedbefore this change still deserialize during rollout.
Supporting information
Resolves openedx/tutor-contrib-platform-notifications#66
Testing instructions
Automated (
openedx/core/djangoapps/notifications/email/tests/test_tasks.py):(
test_recent_email_in_different_course_schedules_buffer,test_multiple_courses_share_buffer) toassert the shared per-user buffer.
test_digest_collects_notifications_across_courses, asserting a single digest pools schedulednotifications from multiple courses.
pytest openedx/core/djangoapps/notifications/email/tests/test_tasks.py→ 93 passed.Manual: with
NOTIFICATION_IMMEDIATE_EMAIL_BUFFER_MINUTESset, an immediate notification in one coursefollowed by immediate notifications in a different course within the window produces a single buffered
digest email covering both courses (verified end-to-end on a Verawood dev stack).
This PR was created using @claude