Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add event for course wide notification request #285

Conversation

saadyousafarbi
Copy link
Contributor

@saadyousafarbi saadyousafarbi commented Oct 30, 2023

Description: This PR adds the COURSE_NOTIFICATION_REQUESTED event, that will be emitted whenever there is a request for a notification to be sent to all learners within a course.

The event will be emitted whenever there is a request for a notification to be sent out to a course-wide audience. The event has the course_key param identifying the course, and also provides the audience_filter param to specify subset of users within a course that the notification should be sent out to. Example of scenarios where this event could be used:

  • Send out course updates to all verified learners,
  • New Discussion post created in forums for course to be sent to all enrolled learner/ cohorted learners etc.

The event also contains fields like app_name, notification_type, content_context and content_url that contain information about the notification, and it's content.

The notification app in edx-platform will listen to this event, and process the notification request for course-wide notifications, delivering the notification to appropriate audience (PR to be created on edx-platform once this gets merged, and event becomes available).

Other

ISSUE: https://2u-internal.atlassian.net/browse/INF-1134

Reviewers:

  • tag reviewer
  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch from 4f79c91 to 79c78f9 Compare October 30, 2023 09:57
@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch from 79c78f9 to b75820a Compare October 30, 2023 10:15
@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch 3 times, most recently from c4b3a0c to f977944 Compare November 1, 2023 10:08
@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch 3 times, most recently from 8cee3f6 to db3d9aa Compare November 6, 2023 07:51
@saadyousafarbi saadyousafarbi marked this pull request as ready for review November 6, 2023 11:29
@saadyousafarbi saadyousafarbi requested a review from a team as a code owner November 6, 2023 11:29
@mariajgrimaldi
Copy link
Member

Can we add more information about the event? So we have more context about it. I noticed you attached a jira issue but it's private.

@mariajgrimaldi mariajgrimaldi self-assigned this Nov 6, 2023
@saadyousafarbi
Copy link
Contributor Author

@mariajgrimaldi sorry about the private jira link.
I have updated the description with information regarding the event. Please let me know if there are confusions/questions regarding it!
Thank you!

@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch 2 times, most recently from 849408e to 1e8a932 Compare November 13, 2023 08:37
Copy link
Member

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor request left! Thank you so much

@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch from 1e8a932 to 86734c3 Compare November 16, 2023 07:48
@saadyousafarbi saadyousafarbi force-pushed the saadyousafarbi/add-course-notification-event branch from 86734c3 to e1a2c30 Compare November 16, 2023 07:53
@saadyousafarbi
Copy link
Contributor Author

@mariajgrimaldi i have made the updates requested!

@mariajgrimaldi
Copy link
Member

Thank you! I'll be merging this now.

@mariajgrimaldi mariajgrimaldi merged commit 1a5f9ca into openedx:main Nov 16, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants