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 Avenger Backlog Notifications #1070

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 60 additions & 0 deletions lib/cadet/assessments/assessments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,66 @@ defmodule Cadet.Assessments do
)
end

def get_ungraded_submission_for_email_notification(avenger_cr) do
submission_answers_query =
Answer
|> group_by([ans], ans.submission_id)
|> select([ans], %{
submission_id: ans.submission_id,
graded_count: filter(count(ans.id), not is_nil(ans.grader_id))
})

question_answers_query =
Question
|> group_by([q], q.assessment_id)
|> join(:left, [q], asst in assoc(q, :assessment))
|> select([q, asst], %{assessment_id: q.assessment_id, question_count: count(q.id)})

student_query =
CourseRegistration
|> join(:inner, [cr], g in assoc(cr, :group))
|> where([cr, g], g.leader_id == ^avenger_cr.id)
|> select([cr, _], cr.id)

submissions =
Submission
|> join(:inner, [s], ans in subquery(submission_answers_query),
on: s.id == ans.submission_id
)
|> join(:inner, [s, ans], asst in subquery(question_answers_query),
on: s.assessment_id == asst.assessment_id
)
|> where([s, _, _], s.status == "submitted")
|> where([_, ans, asst], asst.question_count > ans.graded_count)
|> where(
[s, _, _],
s.student_id in subquery(student_query) or s.student_id == ^avenger_cr.id
)
|> select([s, _, _], %{id: s.id, student_id: s.student_id, assessment_id: s.assessment_id})
|> Repo.all()

formatted_submissions =
Enum.map(submissions, fn submission ->
[student_course_id, student_name] =
CourseRegistration
|> join(:inner, [cr], u in assoc(cr, :user))
|> where([cr], cr.id == ^submission.student_id)
|> select([cr, u], [cr.course_id, u.name])
|> Repo.one()

assessment_title = Repo.get(Assessment, submission.assessment_id).title

%{
student_name: student_name,
student_course_id: student_course_id,
assessment_title: assessment_title,
submission_id: submission.id
}
end)

{:ok, formatted_submissions}
end

defp generate_grading_summary_view_model(submissions, course_id) do
users =
CourseRegistration
Expand Down
26 changes: 25 additions & 1 deletion lib/cadet/email.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ defmodule Cadet.Email do
if is_nil(avenger.email) do
nil
else
ungraded_submissions =
Enum.map(ungraded_submissions, fn submission ->
Map.put(
submission,
:submission_url,
build_submission_url(
submission[:student_course_id],
submission[:submission_id]
)
)
end)

base_email()
|> to(avenger.email)
|> assign(:avenger_name, avenger.name)
Expand All @@ -22,11 +34,18 @@ defmodule Cadet.Email do
if is_nil(avenger.email) do
nil
else
submission =
Map.put(
submission,
:submission_url,
build_submission_url(submission.assessment.course_id, submission.id)
)

base_email()
|> to(avenger.email)
|> assign(:avenger_name, avenger.name)
|> assign(:student_name, student.name)
|> assign(:assessment_title, submission.assessment.title)
|> assign(:submission, submission)
|> subject("New submission for #{submission.assessment.title}")
|> render("#{template_file_name}.html")
end
Expand All @@ -37,4 +56,9 @@ defmodule Cadet.Email do
|> from("noreply@sourceacademy.org")
|> put_html_layout({CadetWeb.LayoutView, "email.html"})
end

# TODO update this to use frontend url
defp build_submission_url(course_id, submission_id) do
"https://sourceacademy.org/courses/#{course_id}/grading/#{submission_id}"
end
end
7 changes: 2 additions & 5 deletions lib/cadet/workers/NotificationWorker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,8 @@ defmodule Cadet.Workers.NotificationWorker do
for avenger_cr <- avengers_crs do
avenger = Cadet.Accounts.get_user(avenger_cr.user_id)

{:ok, %{data: %{submissions: ungraded_submissions}}} =
Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{
"group" => "true",
"notFullyGraded" => "true"
})
{:ok, ungraded_submissions} =
Cadet.Assessments.get_ungraded_submission_for_email_notification(avenger_cr)

if Enum.count(ungraded_submissions) < ungraded_threshold do
IO.puts("[AVENGER_BACKLOG] below threshold!")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<p> Dear <%= @avenger_name %>,</p>

<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>
<p> There is a new submission by <%= @student_name %> for <a href="<%= @submission.submission_url %>"><%= @submission.assessment.title %></a>. Please Review and grade the submission.</p>

<a href="/">Unsubscribe</a> from this email topic.
2 changes: 1 addition & 1 deletion lib/cadet_web/templates/email/avenger_backlog.html.eex
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
You have ungraded submissions. Please review and grade the following submissions as soon as possible.

<%= for s <- @submissions do %>
<p> <a href="/"><%= s["assessment"]["title"] %></a> by <%= s["student"]["name"]%> </p>
<p> <a href="<%= s[:submission_url] %>"><%= s[:assessment_title] %></a> by <%= s[:student_name] %> </p>
<% end %>

<a href="/">Unsubscribe</a> from this email topic.
7 changes: 2 additions & 5 deletions test/cadet/email_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@ defmodule Cadet.EmailTest do
avenger_user = insert(:user, %{email: "test@gmail.com"})
avenger = insert(:course_registration, %{user: avenger_user, role: :staff})

{:ok, %{data: %{submissions: ungraded_submissions}}} =
Cadet.Assessments.submissions_by_grader_for_index(avenger, %{
"group" => "true",
"ungradedOnly" => "true"
})
{:ok, ungraded_submissions} =
Cadet.Assessments.get_ungraded_submission_for_email_notification(avenger)

email = Email.avenger_backlog_email("avenger_backlog", avenger_user, ungraded_submissions)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@
avenger_cr = assessments.course_regs.avenger1_cr

# setup for assessment submission
asssub_ntype = Cadet.Notifications.get_notification_type_by_name!("ASSESSMENT SUBMISSION")

Check warning on line 16 in test/cadet/jobs/notification_worker/notification_worker_test.exs

View workflow job for this annotation

GitHub Actions / Run CI

variable "asssub_ntype" is unused (if the variable is not meant to be used, prefix it with an underscore)
{_name, data} = Enum.at(assessments.assessments, 0)
submission = List.first(List.first(data.mcq_answers)).submission

# setup for avenger backlog
{:ok, %{data: %{submissions: ungraded_submissions}}} =
Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{
"group" => "true",
"notFullyGraded" => "true"
})
{:ok, ungraded_submissions} =
Cadet.Assessments.get_ungraded_submission_for_email_notification(avenger_cr)

Repo.update_all(NotificationType, set: [is_enabled: true])
Repo.update_all(NotificationConfig, set: [is_enabled: true])
Expand All @@ -41,6 +38,7 @@
perform_job(NotificationWorker, %{"notification_type" => "avenger_backlog"})

avenger_email = avenger_user.email

assert_delivered_email_matches(%{to: [{_, ^avenger_email}]})
end

Expand Down
4 changes: 3 additions & 1 deletion test/support/seeds.ex
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ defmodule Cadet.Test.Seeds do
submissions =
students
|> Enum.take(2)
|> Enum.map(&insert(:submission, %{assessment: assessment, student: &1}))
|> Enum.map(
&insert(:submission, %{assessment: assessment, student: &1, status: :submitted})
)

# Programming Answers
programming_answers =
Expand Down
Loading