Skip to content

Commit ed7f772

Browse files
Santosh3007Junyi00martin-henz
authored
Add Avenger Backlog Notifications (#932)
* Create NotificationType Model - Create Notifications module and NotificationType model - Start migration for adding notifications - No functionalities added yet only template code * Configure Oban and Bamboo for notifications system * Create NotificationConfig Model * Create TimeOption Model * Create NotificationPreference Model * Create SentNotification Model * Add Email Field to User Model * fix: Fix Oban configuration * chore: Add dependency on bamboo_phoenix for email template rendering * feat: Implement Oban job for avenger backlog emails * chore: Prevent browser from opening automatically when a local email is sent * Add migrations for notification triggers and avenger backlog notification entry * Implement notification configuration checks * Fix formatting errors * fix: Fix notifications schema typos Changes: * Replaced is_enable to is_enabled for Notification Preferences * Update default is_enabled to true for Notifcation Types * fix: Fix test configurations not being applied `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. * chore: remove unused controllers and views - remove auto-generated controllers and views that are not used * chore: Add tests for notifications * chore: Add test for avenger backlog email * chore: Resolve style violations * chore: Resolve style violations * chore: Resolve style violations * style: fix formatting * fix: Fix bad refactoring * fix: Fix testing environment with Oban and Bamboo * test: add tests for notification types * test: add tests for time options * chore: Update constraints and changesets for notification models * chore: Update default behaviour for no time_option in user preference 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 * style: fix formatting * test: add tests for sent notifications * Add tests for notifications module * fix: Fix testing with Oban Oban introduced changes to testing in v2.12, this commit changes the old test configurations to the new one recommended by official docs. * Add more tests for notifications module * test: add tests for NotificationWorker * style: fix formatting * style: fix formatting * style: fix formatting * fix: Fix tests under notification types name constraints * feat: Implement job for assessment submission mail * fix: Fix assessment submission worker * chore: Add test for assessment submission * chore: Add migration to populate existing nus users' emails 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. * feat: implement sent_notifications - move mailing logic to notification worker - insert into sent_notifications when email is sent out successfully * fix: fix tests * style: fix formatting * fix: fix guard clauses - move guard clauses to prevent unnecessary querying * fix: Fix db triggers not running for assessment submission notifications --------- Co-authored-by: Goh Jun Yi <54541329+Junyi00@users.noreply.github.com> Co-authored-by: Martin Henz <henz@comp.nus.edu.sg>
1 parent 6837435 commit ed7f772

File tree

53 files changed

+1892
-11
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+1892
-11
lines changed

config/config.exs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,6 @@ config :sentry,
7272
root_source_code_path: File.cwd!(),
7373
context_lines: 5
7474

75-
# Import environment specific config. This must remain at the bottom
76-
# of this file so it overrides the configuration defined above.
77-
import_config "#{Mix.env()}.exs"
78-
7975
# Configure Phoenix Swagger
8076
config :cadet, :phoenix_swagger,
8177
swagger_files: %{
@@ -93,3 +89,22 @@ config :guardian, Guardian.DB,
9389
token_types: ["refresh"],
9490
# default: 60 minute
9591
sweep_interval: 180
92+
93+
config :cadet, Oban,
94+
repo: Cadet.Repo,
95+
plugins: [
96+
# keep
97+
{Oban.Plugins.Pruner, max_age: 60},
98+
{Oban.Plugins.Cron,
99+
crontab: [
100+
{"@daily", Cadet.Workers.NotificationWorker,
101+
args: %{"notification_type" => "avenger_backlog"}}
102+
]}
103+
],
104+
queues: [default: 10, notifications: 1]
105+
106+
config :cadet, Cadet.Mailer, adapter: Bamboo.LocalAdapter
107+
108+
# Import environment specific config. This must remain at the bottom
109+
# of this file so it overrides the configuration defined above.
110+
import_config "#{Mix.env()}.exs"

config/prod.exs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,5 @@ config :logger, level: :info
4444
config :ex_aws,
4545
access_key_id: [:instance_role],
4646
secret_access_key: [:instance_role]
47+
48+
config :cadet, Cadet.Mailer, adapter: Bamboo.SesAdapter

config/test.exs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,9 @@ config :arc, storage: Arc.Storage.Local
8888

8989
if "test.secrets.exs" |> Path.expand(__DIR__) |> File.exists?(),
9090
do: import_config("test.secrets.exs")
91+
92+
config :cadet, Oban,
93+
repo: Cadet.Repo,
94+
testing: :manual
95+
96+
config :cadet, Cadet.Mailer, adapter: Bamboo.TestAdapter

lib/cadet/accounts/course_registrations.ex

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ defmodule Cadet.Accounts.CourseRegistrations do
6363
|> Repo.all()
6464
end
6565

66+
def get_staffs(course_id) do
67+
CourseRegistration
68+
|> where(course_id: ^course_id)
69+
|> where(role: :staff)
70+
|> Repo.all()
71+
end
72+
6673
def get_users(course_id, group_id) when is_ecto_id(group_id) and is_ecto_id(course_id) do
6774
CourseRegistration
6875
|> where([cr], cr.course_id == ^course_id)
@@ -200,4 +207,23 @@ defmodule Cadet.Accounts.CourseRegistrations do
200207
{:error, changeset} -> {:error, {:bad_request, full_error_messages(changeset)}}
201208
end
202209
end
210+
211+
def get_avenger_of(student_id) when is_ecto_id(student_id) do
212+
CourseRegistration
213+
|> Repo.get_by(id: student_id)
214+
|> Repo.preload(:group)
215+
|> Map.get(:group)
216+
|> case do
217+
nil ->
218+
nil
219+
220+
group ->
221+
avenger_id = Map.get(group, :leader_id)
222+
223+
CourseRegistration
224+
|> where([cr], cr.id == ^avenger_id)
225+
|> preload(:user)
226+
|> Repo.one()
227+
end
228+
end
203229
end

lib/cadet/accounts/user.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ defmodule Cadet.Accounts.User do
2020
field(:username, :string)
2121
field(:provider, :string)
2222
field(:super_admin, :boolean)
23+
field(:email, :string)
2324

2425
belongs_to(:latest_viewed_course, Course)
2526
has_many(:courses, CourseRegistration)

lib/cadet/application.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ defmodule Cadet.Application do
2020
# Start the GuardianDB sweeper
2121
worker(Guardian.DB.Token.SweeperServer, []),
2222
# Start the Quantum scheduler
23-
worker(Cadet.Jobs.Scheduler, [])
23+
worker(Cadet.Jobs.Scheduler, []),
24+
# Start the Oban instance
25+
{Oban, Application.fetch_env!(:cadet, Oban)}
2426
]
2527

2628
children =

lib/cadet/assessments/assessments.ex

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -721,11 +721,24 @@ defmodule Cadet.Assessments do
721721
|> Repo.one()
722722
end
723723

724+
def get_submission_by_id(submission_id) when is_ecto_id(submission_id) do
725+
Submission
726+
|> where(id: ^submission_id)
727+
|> join(:inner, [s], a in assoc(s, :assessment))
728+
|> preload([_, a], assessment: a)
729+
|> Repo.one()
730+
end
731+
724732
def finalise_submission(submission = %Submission{}) do
725733
with {:status, :attempted} <- {:status, submission.status},
726734
{:ok, updated_submission} <- update_submission_status_and_xp_bonus(submission) do
727735
# Couple with update_submission_status_and_xp_bonus to ensure notification is sent
728736
Notifications.write_notification_when_student_submits(submission)
737+
# Send email notification to avenger
738+
%{notification_type: "assessment_submission", submission_id: updated_submission.id}
739+
|> Cadet.Workers.NotificationWorker.new()
740+
|> Oban.insert()
741+
729742
# Begin autograding job
730743
GradingJob.force_grade_individual_submission(updated_submission)
731744

@@ -1151,7 +1164,8 @@ defmodule Cadet.Assessments do
11511164
{:ok, String.t()}
11521165
def all_submissions_by_grader_for_index(
11531166
grader = %CourseRegistration{course_id: course_id},
1154-
group_only \\ false
1167+
group_only \\ false,
1168+
ungraded_only \\ false
11551169
) do
11561170
show_all = not group_only
11571171

@@ -1161,6 +1175,11 @@ defmodule Cadet.Assessments do
11611175
else:
11621176
"where s.student_id in (select cr.id from course_registrations cr inner join groups g on cr.group_id = g.id where g.leader_id = $2) or s.student_id = $2"
11631177

1178+
ungraded_where =
1179+
if ungraded_only,
1180+
do: "where s.\"gradedCount\" < assts.\"questionCount\"",
1181+
else: ""
1182+
11641183
params = if show_all, do: [course_id], else: [course_id, grader.id]
11651184

11661185
# We bypass Ecto here and use a raw query to generate JSON directly from
@@ -1200,7 +1219,7 @@ defmodule Cadet.Assessments do
12001219
group by s.id) s
12011220
inner join
12021221
(select
1203-
a.id, to_json(a) as jsn
1222+
a.id, a."questionCount", to_json(a) as jsn
12041223
from
12051224
(select
12061225
a.id,
@@ -1240,6 +1259,7 @@ defmodule Cadet.Assessments do
12401259
from course_registrations cr
12411260
inner join
12421261
users u on u.id = cr.user_id) cr) unsubmitters on s.unsubmitted_by_id = unsubmitters.id
1262+
#{ungraded_where}
12431263
) q
12441264
""",
12451265
params

lib/cadet/courses/courses.ex

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ defmodule Cadet.Courses do
8282
end
8383
end
8484

85+
def get_all_course_ids do
86+
Course
87+
|> select([c], c.id)
88+
|> Repo.all()
89+
end
90+
8591
defp retrieve_course(course_id) when is_ecto_id(course_id) do
8692
Course
8793
|> where(id: ^course_id)
@@ -233,8 +239,8 @@ defmodule Cadet.Courses do
233239
)
234240
|> where(course_id: ^course_id)
235241
|> Repo.one()} do
236-
# It is ok to assume that user course registions already exist, as they would have been created
237-
# in the admin_user_controller before calling this function
242+
# It is ok to assume that user course registions already exist, as they would
243+
# have been created in the admin_user_controller before calling this function
238244
case role do
239245
# If student, update his course registration
240246
:student ->

lib/cadet/email.ex

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
defmodule Cadet.Email do
2+
@moduledoc """
3+
Contains methods for sending email notifications.
4+
"""
5+
use Bamboo.Phoenix, view: CadetWeb.EmailView
6+
import Bamboo.Email
7+
8+
def avenger_backlog_email(template_file_name, avenger, ungraded_submissions) do
9+
if is_nil(avenger.email) do
10+
nil
11+
else
12+
base_email()
13+
|> to(avenger.email)
14+
|> assign(:avenger_name, avenger.name)
15+
|> assign(:submissions, ungraded_submissions)
16+
|> subject("Backlog for #{avenger.name}")
17+
|> render("#{template_file_name}.html")
18+
end
19+
end
20+
21+
def assessment_submission_email(template_file_name, avenger, student, submission) do
22+
if is_nil(avenger.email) do
23+
nil
24+
else
25+
base_email()
26+
|> to(avenger.email)
27+
|> assign(:avenger_name, avenger.name)
28+
|> assign(:student_name, student.name)
29+
|> assign(:assessment_title, submission.assessment.title)
30+
|> subject("New submission for #{submission.assessment.title}")
31+
|> render("#{template_file_name}.html")
32+
end
33+
end
34+
35+
defp base_email do
36+
new_email()
37+
|> from("noreply@sourceacademy.org")
38+
|> put_html_layout({CadetWeb.LayoutView, "email.html"})
39+
end
40+
end

lib/cadet/jobs/scheduler.ex

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# credo:disable-for-this-file Credo.Check.Readability.ModuleDoc
22
# @moduledoc is actually generated by a macro inside Quantum
33
defmodule Cadet.Jobs.Scheduler do
4+
@moduledoc """
5+
Quantum is used for scheduling jobs with cron jobs.
6+
"""
47
use Quantum, otp_app: :cadet
58
end

lib/cadet/mailer.ex

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
defmodule Cadet.Mailer do
2+
@moduledoc """
3+
Mailer used to sent notification emails.
4+
"""
5+
use Bamboo.Mailer, otp_app: :cadet
6+
end

0 commit comments

Comments
 (0)