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

Add Avenger Backlog Notifications #932

Merged

Conversation

Santosh3007
Copy link
Contributor

@Santosh3007 Santosh3007 commented Mar 7, 2023

Source Academy Notifications

This is the first iteration of introducing notifications into Source Academy. Currently, this introduces two notification types: Avenger Backlog , Assessment Submission and only through the email medium.

In future iterations, more notifications can be included and other mediums of notifications can be explored.

Changes

  • Implement Notifications System Framework
  • Add Oban (Job Processing Library)
  • Add Bamboo (Email Dispatch)
  • Implementation for avenger backlog notifications
  • Implementation for assessment submission notifications
  • Tests for Bamboo, Oban & notification models

Notes

  • The original notifications table is left untouched for now
  • 2 triggers (and procedures) are added to the database to populate notification_configs when new courses are insert & when new assessments are created
  • Migration added to populate email column in users table using username (Only for NUS users where the provider is luminus)
  • Will improve robustness of Oban/Mailer testing in future PRs

Santosh3007 and others added 30 commits February 14, 2023 15:32
- Create Notifications module and NotificationType model
- Start migration for adding notifications
- No functionalities added yet only template code
Changes:
* Replaced is_enable to is_enabled for Notification Preferences
* Update default is_enabled to true for Notifcation Types
`import_config` must always appear at the bottom for environment
specific configurations to be applied correctly. All configurations
after this line will overwrite configurations that exists in the
environment specific ones.
- remove auto-generated controllers and views that are not used
If user preference has no time option, use the time_option from
notification_config instead. This is so that the behaviour of these
users with no preferences would always follow the default chosen by the
course admin
@coveralls
Copy link

coveralls commented Mar 7, 2023

Coverage Status

Coverage: 95.345% (-0.3%) from 95.608% when pulling 67a56f8 on Santosh3007:add-avenger-backlog-notification into 6837435 on source-academy:master.

@Junyi00 Junyi00 added the Enhancement New feature or request label Mar 8, 2023
Santosh3007 and others added 10 commits March 10, 2023 00:15
Current nus users will have their email attribute populated based on
their username. nus users are identified from the provider attribute.

Future nus users will have their email attribute populated on creation
ideally.
- move mailing logic to notification worker
- insert into sent_notifications when email is sent out successfully
- move guard clauses to prevent unnecessary querying

<p> There is a new submission by <%= @student_name %> for <a href="/"><%= @assessment_title %></a>. Please Review and grade the <a href="/">submission </a></p>

<a href="/">Unsubscribe</a> from this email topic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Rmb to update the links!


"""

# def list_sent_notifications do
Copy link
Contributor

Choose a reason for hiding this comment

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

would these commented out functions be used in the future?

@RichDom2185
Copy link
Member

Let us merge this first so that this can be tested on staging. The remaining issues seem to be minor, and I have created issues as above to be resolved.

@RichDom2185 RichDom2185 merged commit ed7f772 into source-academy:master May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants