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

Fix some router security and code style issues #973

Closed
wants to merge 88 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
88 commits
Select commit Hold shift + click to select a range
fcc4cf5
Create NotificationType Model
Santosh3007 Feb 14, 2023
b429644
Configure Oban and Bamboo for notifications system
Junyi00 Feb 14, 2023
d024a75
Create NotificationConfig Model
Santosh3007 Feb 14, 2023
cd20b4a
Create TimeOption Model
Santosh3007 Feb 14, 2023
a580719
Create NotificationPreference Model
Santosh3007 Feb 14, 2023
877220a
Create SentNotification Model
Santosh3007 Feb 14, 2023
c8ecd5d
Add Email Field to User Model
Junyi00 Feb 15, 2023
6a6cec7
fix: Fix Oban configuration
Junyi00 Feb 15, 2023
1b5ca22
chore: Add dependency on bamboo_phoenix for email template rendering
Junyi00 Feb 15, 2023
39367e4
feat: Implement Oban job for avenger backlog emails
Junyi00 Feb 15, 2023
e6cd06c
chore: Prevent browser from opening automatically when a local email …
Junyi00 Feb 15, 2023
4cc7b7f
Add migrations for notification triggers and avenger backlog
Junyi00 Feb 15, 2023
1802e49
Implement notification configuration checks
Junyi00 Feb 15, 2023
0f7b7e4
Fix formatting errors
Santosh3007 Feb 16, 2023
dd951e8
fix: Fix notifications schema typos
Junyi00 Feb 21, 2023
554054f
fix: Fix test configurations not being applied
Junyi00 Feb 21, 2023
960d75b
chore: remove unused controllers and views
Santosh3007 Feb 27, 2023
851a309
chore: Add tests for notifications
Junyi00 Feb 27, 2023
fa4ea44
chore: Add test for avenger backlog email
Junyi00 Feb 27, 2023
6602170
chore: Resolve style violations
Junyi00 Feb 27, 2023
669ac75
chore: Resolve style violations
Junyi00 Feb 27, 2023
e8bff4c
chore: Resolve style violations
Junyi00 Feb 27, 2023
45bbb34
style: fix formatting
Santosh3007 Feb 27, 2023
f6857b9
fix: Fix bad refactoring
Junyi00 Feb 27, 2023
f709625
fix: Fix testing environment with Oban and Bamboo
Junyi00 Feb 27, 2023
5b43002
test: add tests for notification types
Santosh3007 Feb 28, 2023
b37f4a5
test: add tests for time options
Santosh3007 Feb 28, 2023
b0000c6
chore: Update constraints and changesets for notification models
Junyi00 Feb 28, 2023
7003af5
chore: Update default behaviour for no time_option in user preference
Junyi00 Feb 28, 2023
1f25fc0
style: fix formatting
Santosh3007 Feb 28, 2023
d6d6869
test: add tests for sent notifications
Santosh3007 Mar 1, 2023
fdfd195
Add tests for notifications module
Junyi00 Mar 1, 2023
31c7b5c
Merge branch 'add-avenger-backlog-notification' of https://github.com…
Junyi00 Mar 1, 2023
498ca1b
fix: Fix testing with Oban
Junyi00 Mar 2, 2023
70823a6
Add more tests for notifications module
Junyi00 Mar 2, 2023
cf160f8
test: add tests for NotificationWorker
Santosh3007 Mar 5, 2023
83d4943
style: fix formatting
Santosh3007 Mar 5, 2023
bf362c7
style: fix formatting
Santosh3007 Mar 5, 2023
05aac5c
style: fix formatting
Santosh3007 Mar 6, 2023
3b571ff
Merge branch 'add-avenger-backlog-notification' of https://github.com…
Junyi00 Mar 6, 2023
8f0e982
fix: Fix tests under notification types name constraints
Junyi00 Mar 6, 2023
ba29201
feat: Implement job for assessment submission mail
Santosh3007 Feb 26, 2023
c07f88f
Merge branch 'add-avenger-backlog-notification' of https://github.com…
Junyi00 Mar 9, 2023
4f120ee
fix: Fix assessment submission worker
Junyi00 Mar 10, 2023
0b245a9
chore: Add test for assessment submission
Junyi00 Mar 10, 2023
9600d40
chore: Add migration to populate existing nus users' emails
Junyi00 Mar 11, 2023
22c1391
feat: implement sent_notifications
Santosh3007 Mar 12, 2023
8ef9a59
fix: fix tests
Santosh3007 Mar 12, 2023
a516463
style: fix formatting
Santosh3007 Mar 12, 2023
8994947
fix: fix guard clauses
Santosh3007 Mar 13, 2023
b93cdbd
fix: Fix db triggers not running for assessment submission notifications
Junyi00 Mar 13, 2023
deca215
feat: Add endpoints for notifications
Junyi00 Mar 18, 2023
5c321ca
fix: Duplicate records returned with multiple time options
Junyi00 Mar 23, 2023
1422153
chore: Update notifications endpoints
Junyi00 Mar 23, 2023
de920be
Add more endpoints for notifications UI
Junyi00 Apr 2, 2023
a0b8eef
update notifications query to return a single preference
Junyi00 Apr 4, 2023
059d969
Update endpoint to batch update notification configs instead
Junyi00 Apr 5, 2023
2cc2f35
Fix Notifications Endpoints
Santosh3007 Apr 5, 2023
2880a3a
Add upsert endpoint for notification preferences
Santosh3007 May 30, 2023
dc02296
Merge branch 'master' into add-notifications-endpoints
Santosh3007 Jun 14, 2023
e8a28d6
Fix linting
Santosh3007 Jun 14, 2023
5cc0439
Fix linting
Santosh3007 Jun 14, 2023
6206538
Fix linting
Santosh3007 Jun 14, 2023
45f3e1c
Fix Notification Type tests
Santosh3007 Jun 14, 2023
5b50c47
Fix spec issues for notifications
Junyi00 Jun 14, 2023
b620cd3
FIx linting
Santosh3007 Jun 16, 2023
1ff2fb1
Fix avenger backlog tests
Santosh3007 Jun 16, 2023
bb46771
Fix Type Checking
Santosh3007 Jun 16, 2023
95206d4
Add tests for notifications controller
Junyi00 Jun 16, 2023
eedaf8c
Fix credo errors
Junyi00 Jun 16, 2023
5949366
Fix formatting issues
Junyi00 Jun 16, 2023
9cc08b4
Fix module name definition
Junyi00 Jun 16, 2023
6c242a0
Add more tests and remove unused controller actions
Junyi00 Jun 16, 2023
df05af4
Add user level configuration for avenger backlog
Santosh3007 Jun 18, 2023
65a2a8a
Merge branch 'master' into add-notifications-endpoints
RichDom2185 Jun 21, 2023
c21e42f
Merge branch 'master' into add-notifications-endpoints
martin-henz Jun 22, 2023
eeae507
update gettext
Catherine9898 Jul 3, 2023
05291a7
modify get router
Catherine9898 Jul 3, 2023
45fae7a
5routerdone
Catherine9898 Jul 6, 2023
f2d9d36
allrouters
Catherine9898 Jul 7, 2023
d3ff8dd
modifycodestyle
Catherine9898 Jul 10, 2023
0cd5f3b
modify admin page
Catherine9898 Jul 17, 2023
eeb0bbe
delete useless comments
Catherine9898 Jul 18, 2023
88fa350
delete
Catherine9898 Jul 18, 2023
283a518
delete useless comments
Catherine9898 Jul 18, 2023
0981277
add slash
Catherine9898 Jul 19, 2023
7f4ea7f
Merge branch 'master' into cat
YaleChen299 Jul 25, 2023
b9190fb
Merge branch 'master' into cat
RichDom2185 Sep 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 181 additions & 10 deletions lib/cadet/notifications.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""

import Ecto.Query, warn: false
require Logger
alias Cadet.Repo

alias Cadet.Notifications.{
Expand Down Expand Up @@ -49,20 +50,94 @@

def get_notification_config!(notification_type_id, course_id, assconfig_id) do
query =
from(n in Cadet.Notifications.NotificationConfig,
join: ntype in Cadet.Notifications.NotificationType,
on: n.notification_type_id == ntype.id,
where: n.notification_type_id == ^notification_type_id and n.course_id == ^course_id
Cadet.Notifications.NotificationConfig
|> join(:inner, [n], ntype in Cadet.Notifications.NotificationType,
on: n.notification_type_id == ntype.id
)
|> where([n], n.notification_type_id == ^notification_type_id and n.course_id == ^course_id)
|> filter_assconfig_id(assconfig_id)
|> Repo.one()

case query do
nil ->
Logger.error(
"No NotificationConfig found for Course #{course_id} and NotificationType #{notification_type_id}"
)

nil

config ->
config
end
end

defp filter_assconfig_id(query, nil) do
query |> where([c], is_nil(c.assessment_config_id))
end

defp filter_assconfig_id(query, assconfig_id) do
query |> where([c], c.assessment_config_id == ^assconfig_id)
end

def get_notification_config!(id), do: Repo.get!(NotificationConfig, id)

@doc """
Gets all notification configs that belong to a course
"""
def get_notification_configs(course_id) do
query =
if is_nil(assconfig_id) do
where(query, [c], is_nil(c.assessment_config_id))
else
where(query, [c], c.assessment_config_id == ^assconfig_id)
end
Cadet.Notifications.NotificationConfig
|> where([n], n.course_id == ^course_id)
|> Repo.all()

Repo.one(query)
query
|> Repo.preload([:notification_type, :course, :assessment_config, :time_options])
end

@doc """
Gets all notification configs with preferences that
1. belongs to the course of the course reg,
2. only notifications that it can configure based on course reg's role
"""
def get_configurable_notification_configs(cr_id) do
cr = Repo.get(Cadet.Accounts.CourseRegistration, cr_id)

case cr do
nil ->
nil

_ ->
is_staff = cr.role == :staff

query =
Cadet.Notifications.NotificationConfig
|> join(:inner, [n], ntype in Cadet.Notifications.NotificationType,
on: n.notification_type_id == ntype.id
)
|> join(:inner, [n], c in Cadet.Courses.Course, on: n.course_id == c.id)
|> join(:left, [n], ac in Cadet.Courses.AssessmentConfig,
on: n.assessment_config_id == ac.id
)
|> join(:left, [n], p in Cadet.Notifications.NotificationPreference,
on: p.notification_config_id == n.id
)
|> where(
[n, ntype, c, ac, p],
ntype.for_staff == ^is_staff and
n.course_id == ^cr.course_id and
(p.course_reg_id == ^cr.id or is_nil(p.course_reg_id))
)
|> Repo.all()

query
|> Repo.preload([
:notification_type,
:course,
:assessment_config,
:time_options,
:notification_preferences
])
end
end

@doc """
Expand All @@ -83,6 +158,17 @@
|> Repo.update()
end

def update_many_noti_configs(noti_configs) when is_list(noti_configs) do
Repo.transaction(fn ->
for noti_config <- noti_configs do
case Repo.update(noti_config) do
{:ok, res} -> res
{:error, error} -> Repo.rollback(error)
end
end
end)
end

@doc """
Returns an `%Ecto.Changeset{}` for tracking notification_config changes.

Expand Down Expand Up @@ -112,6 +198,24 @@
"""
def get_time_option!(id), do: Repo.get!(TimeOption, id)

@doc """
Gets all time options for a notification config
"""
def get_time_options_for_config(notification_config_id) do
query =
Cadet.Notifications.TimeOption
|> join(:inner, [to], nc in Cadet.Notifications.NotificationConfig,
on: to.notification_config_id == nc.id
)
|> where([to, nc], nc.id == ^notification_config_id)
|> Repo.all()

query
end

@doc """
Gets all time options for an assessment config and notification type
"""
def get_time_options_for_assessment(assessment_config_id, notification_type_id) do
query =
from(ac in Cadet.Courses.AssessmentConfig,
Expand All @@ -126,6 +230,9 @@
Repo.all(query)
end

@doc """
Gets the default time options for an assessment config and notification type
"""
def get_default_time_option_for_assessment!(assessment_config_id, notification_type_id) do
query =
from(ac in Cadet.Courses.AssessmentConfig,
Expand Down Expand Up @@ -160,6 +267,34 @@
|> Repo.insert()
end

def upsert_many_time_options(time_options) when is_list(time_options) do
Repo.transaction(fn ->
for to <- time_options do
case Repo.insert(to,
on_conflict: {:replace, [:is_default]},
conflict_target: [:minutes, :notification_config_id]
) do
{:ok, time_option} -> time_option
{:error, error} -> Repo.rollback(error)
end
end
end)
end

def upsert_many_noti_preferences(noti_prefs) when is_list(noti_prefs) do
Repo.transaction(fn ->
for np <- noti_prefs do
case Repo.insert(np,
on_conflict: {:replace, [:is_enabled, :time_option_id]},
conflict_target: [:course_reg_id, :notification_config_id]
) do
{:ok, noti_pref} -> noti_pref
{:error, error} -> Repo.rollback(error)
end
end
end)
end

@doc """
Deletes a time_option.

Expand All @@ -176,6 +311,42 @@
Repo.delete(time_option)
end

def delete_many_time_options(to_ids) when is_list(to_ids) do
Repo.transaction(fn ->
for to_id <- to_ids do
time_option = Repo.get(TimeOption, to_id)

case time_option do
nil ->
Repo.rollback("Time option does not exist")

_ ->
case Repo.delete(time_option) do
{:ok, deleted_time_option} -> deleted_time_option
{:delete_error, error} -> Repo.rollback(error)
end
end
end
end)
end

@doc """
Gets the notification preference based from its id
"""
def get_notification_preference!(notification_preference_id) do
query =
NotificationPreference
|> join(:left, [np], to in TimeOption, on: to.id == np.time_option_id)
|> where([np, to], np.id == ^notification_preference_id)
|> preload(:time_option)
|> Repo.one!()

query
end

@doc """
Gets the notification preference based from notification type and course reg
"""
def get_notification_preference(notification_type_id, course_reg_id) do
query =
from(np in NotificationPreference,
Expand Down Expand Up @@ -276,7 +447,7 @@
|> Repo.insert()
end

@doc """

Check warning on line 450 in lib/cadet/notifications.ex

View workflow job for this annotation

GitHub Actions / Run CI

module attribute @doc was set but no definition follows it
Returns the list of sent_notifications.

## Examples
Expand Down
5 changes: 4 additions & 1 deletion lib/cadet/notifications/notification_config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defmodule Cadet.Notifications.NotificationConfig do
use Ecto.Schema
import Ecto.Changeset
alias Cadet.Courses.{Course, AssessmentConfig}
alias Cadet.Notifications.NotificationType
alias Cadet.Notifications.{NotificationType, TimeOption, NotificationPreference}

schema "notification_configs" do
field(:is_enabled, :boolean, default: false)
Expand All @@ -14,6 +14,9 @@ defmodule Cadet.Notifications.NotificationConfig do
belongs_to(:course, Course)
belongs_to(:assessment_config, AssessmentConfig)

has_many(:time_options, TimeOption)
has_many(:notification_preferences, NotificationPreference)

timestamps()
end

Expand Down
3 changes: 2 additions & 1 deletion lib/cadet/notifications/notification_preference.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ defmodule Cadet.Notifications.NotificationPreference do
@doc false
def changeset(notification_preference, attrs) do
notification_preference
|> cast(attrs, [:is_enabled, :notification_config_id, :course_reg_id])
|> cast(attrs, [:is_enabled, :notification_config_id, :course_reg_id, :time_option_id])
|> validate_required([:notification_config_id, :course_reg_id])
|> unique_constraint(:unique_course_reg_and_config, name: :single_preference_per_config)
|> prevent_nil_is_enabled()
end

Expand Down
5 changes: 3 additions & 2 deletions lib/cadet/notifications/notification_type.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ defmodule Cadet.Notifications.NotificationType do
field(:is_enabled, :boolean, default: false)
field(:name, :string)
field(:template_file_name, :string)
field(:for_staff, :boolean)

timestamps()
end

@doc false
def changeset(notification_type, attrs) do
notification_type
|> cast(attrs, [:name, :template_file_name, :is_enabled, :is_autopopulated])
|> validate_required([:name, :template_file_name, :is_autopopulated])
|> cast(attrs, [:name, :template_file_name, :is_enabled, :is_autopopulated, :for_staff])
|> validate_required([:name, :template_file_name, :is_autopopulated, :for_staff])
|> unique_constraint(:name)
|> prevent_nil_is_enabled()
end
Expand Down
55 changes: 30 additions & 25 deletions lib/cadet/workers/NotificationWorker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ defmodule Cadet.Workers.NotificationWorker do
Contain oban workers for sending notifications
"""
use Oban.Worker, queue: :notifications, max_attempts: 1
require Logger
alias Cadet.{Email, Notifications, Mailer}
alias Cadet.Repo

Expand Down Expand Up @@ -73,39 +74,43 @@ defmodule Cadet.Workers.NotificationWorker do
for avenger_cr <- avengers_crs do
avenger = Cadet.Accounts.get_user(avenger_cr.user_id)

ungraded_submissions =
Jason.decode!(
elem(
Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true),
1
if is_user_enabled(notification_type_id, avenger_cr.id) do
ungraded_submissions =
Jason.decode!(
elem(
Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true),
1
)
)
)

if length(ungraded_submissions) < ungraded_threshold do
IO.puts("[AVENGER_BACKLOG] below threshold!")
else
IO.puts("[AVENGER_BACKLOG] SENDING_OUT")
if length(ungraded_submissions) < ungraded_threshold do
Logger.info("[AVENGER_BACKLOG] below threshold!")
else
Logger.info("[AVENGER_BACKLOG] SENDING_OUT")

email =
Email.avenger_backlog_email(
ntype.template_file_name,
avenger,
ungraded_submissions
)
email =
Email.avenger_backlog_email(
ntype.template_file_name,
avenger,
ungraded_submissions
)

{status, email} = Mailer.deliver_now(email)
{status, email} = Mailer.deliver_now(email)

if status == :ok do
Notifications.create_sent_notification(avenger_cr.id, email.html_body)
if status == :ok do
Notifications.create_sent_notification(avenger_cr.id, email.html_body)
end
end
else
Logger.info("[ASSESSMENT_SUBMISSION] user-level disabled")
end
end
else
IO.puts("[AVENGER_BACKLOG] course-level disabled")
Logger.info("[AVENGER_BACKLOG] course-level disabled")
end
end
else
IO.puts("[AVENGER_BACKLOG] system-level disabled!")
Logger.info("[AVENGER_BACKLOG] system-level disabled!")
end

:ok
Expand All @@ -132,13 +137,13 @@ defmodule Cadet.Workers.NotificationWorker do

cond do
!is_course_enabled(notification_type.id, course_id, assessment_config_id) ->
IO.puts("[ASSESSMENT_SUBMISSION] course-level disabled")
Logger.info("[ASSESSMENT_SUBMISSION] course-level disabled")

!is_user_enabled(notification_type.id, avenger_cr.id) ->
IO.puts("[ASSESSMENT_SUBMISSION] user-level disabled")
Logger.info("[ASSESSMENT_SUBMISSION] user-level disabled")

true ->
IO.puts("[ASSESSMENT_SUBMISSION] SENDING_OUT")
Logger.info("[ASSESSMENT_SUBMISSION] SENDING_OUT")

email =
Email.assessment_submission_email(
Expand All @@ -155,7 +160,7 @@ defmodule Cadet.Workers.NotificationWorker do
end
end
else
IO.puts("[ASSESSMENT_SUBMISSION] system-level disabled!")
Logger.info("[ASSESSMENT_SUBMISSION] system-level disabled!")
end
end
end
Loading
Loading