Fix DB crash due to timeout (#1031)

* Create index on assessment ID in submissions table

Improves read performance.

* Create separate view layer for grading summary

* Move views under AdminGradingView

* Migrate to more optimised query

* Use subquery instead of join for answers
* Split main model query and view model generation
* Split queries into 3
* Use ORM where possible

* Fix typing

* Restore group filtering functionality

* Update Avenger backlog notification workers

Fixes compatibilty with new function signature.

* Remove old query

* Remove comment

lib/cadet/assessments/assessments.ex

@@ -1238,114 +1238,106 @@ defmodule Cadet.Assessments do
  {:forbidden, "Forbidden."}}
  """
  @spec all_submissions_by_grader_for_index(CourseRegistration.t()) ::
- {:ok, String.t()}
+ {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}}
  def all_submissions_by_grader_for_index(
  grader = %CourseRegistration{course_id: course_id},
  group_only \\ false,
- ungraded_only \\ false
+ _ungraded_only \\ false
  ) do
  show_all = not group_only
 
- group_where =
+ group_filter =
  if show_all,
  do: "",
  else:
- "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"
+ "AND s.student_id IN (SELECT cr.id FROM course_registrations AS cr INNER JOIN groups AS g ON cr.group_id = g.id WHERE g.leader_id = #{grader.id}) OR s.student_id = #{grader.id}"
 
- ungraded_where =
-  if ungraded_only,
-  do: "where s.\"gradedCount\" < assts.\"questionCount\"",
-  else: ""
-
- params = if show_all, do: [course_id], else: [course_id, grader.id]
+ # TODO: Restore ungraded filtering
+ # ... or more likely, decouple email logic from this function
+ # ungraded_where =
+ # if ungraded_only,
+ # do: "where s.\"gradedCount\" < assts.\"questionCount\"",
+ # else: ""
 
  # We bypass Ecto here and use a raw query to generate JSON directly from
  # PostgreSQL, because doing it in Elixir/Erlang is too inefficient.
 
- case Repo.query(
- """
- select json_agg(q)::TEXT from
- (
- select
- s.id,
- s.status,
- s."unsubmittedAt",
- s.xp,
- s."xpAdjustment",
- s."xpBonus",
- s."gradedCount",
- assts.jsn as assessment,
- students.jsn as student,
- unsubmitters.jsn as "unsubmittedBy"
- from
- (select
- s.id,
- s.student_id,
- s.assessment_id,
- s.status,
- s.unsubmitted_at as "unsubmittedAt",
- s.unsubmitted_by_id,
- sum(ans.xp) as xp,
- sum(ans.xp_adjustment) as "xpAdjustment",
- s.xp_bonus as "xpBonus",
- count(ans.id) filter (where ans.grader_id is not null) as "gradedCount"
- from submissions s
- left join
- answers ans on s.id = ans.submission_id
- #{group_where}
- group by s.id) s
- inner join
- (select
- a.id, a."questionCount", to_json(a) as jsn
- from
- (select
- a.id,
- a.title,
- a.number as "assessmentNumber",
- bool_or(ac.is_manually_graded) as "isManuallyGraded",
- max(ac.type) as "type",
- sum(q.max_xp) as "maxXp",
- count(q.id) as "questionCount"
- from assessments a
- left join
- questions q on a.id = q.assessment_id
- inner join
- assessment_configs ac on ac.id = a.config_id
- where a.course_id = $1
- group by a.id) a) assts on assts.id = s.assessment_id
- inner join
- (select
- cr.id, to_json(cr) as jsn
- from
- (select
- cr.id,
- u.name as "name",
- u.username as "username",
- g.name as "groupName",
- g.leader_id as "groupLeaderId"
- from course_registrations cr
- left join
- groups g on g.id = cr.group_id
- inner join
- users u on u.id = cr.user_id) cr) students on students.id = s.student_id
- left join
- (select
- cr.id, to_json(cr) as jsn
- from
- (select
- cr.id,
- u.name
- from course_registrations cr
- inner join
- users u on u.id = cr.user_id) cr) unsubmitters on s.unsubmitted_by_id = unsubmitters.id
- #{ungraded_where}
- ) q
- """,
- params
- ) do
- {:ok, %{rows: [[nil]]}} -> {:ok, "[]"}
- {:ok, %{rows: [[json]]}} -> {:ok, json}
- end
+ submissions =
+ case Repo.query("""
+ SELECT
+ s.id,
+ s.status,
+ s.unsubmitted_at,
+ s.unsubmitted_by_id,
+ s_ans.xp,
+ s_ans.xp_adjustment,
+ s.xp_bonus,
+ s_ans.graded_count,
+ s.student_id,
+ s.assessment_id
+ FROM
+ submissions AS s
+ LEFT JOIN (
+ SELECT
+ ans.submission_id,
+ SUM(ans.xp) AS xp,
+ SUM(ans.xp_adjustment) AS xp_adjustment,
+ COUNT(ans.id) FILTER (
+ WHERE
+ ans.grader_id IS NOT NULL
+ ) AS graded_count
+ FROM
+ answers AS ans
+ GROUP BY
+ ans.submission_id
+ ) AS s_ans ON s_ans.submission_id = s.id
+ WHERE
+ s.assessment_id IN (
+ SELECT
+ id
+ FROM
+ assessments
+ WHERE
+ assessments.course_id = #{course_id}
+ ) #{group_filter};
+ """) do
+ {:ok, %{columns: columns, rows: result}} ->
+ result
+ |> Enum.map(
+ &(columns
+ |> Enum.map(fn c -> String.to_atom(c) end)
+ |> Enum.zip(&1)
+ |> Enum.into(%{}))
+ )
+ end
+
+ {:ok, generate_grading_summary_view_model(submissions, course_id)}
+ end
+
+ defp generate_grading_summary_view_model(submissions, course_id) do
+ users =
+ CourseRegistration
+ |> where([cr], cr.course_id == ^course_id)
+ |> join(:inner, [cr], u in assoc(cr, :user))
+ |> join(:left, [cr, u], g in assoc(cr, :group))
+ |> preload([cr, u, g], user: u, group: g)
+ |> Repo.all()
+
+ assessment_ids = submissions |> Enum.map(& &1.assessment_id) |> Enum.uniq()
+
+ assessments =
+ Assessment
+ |> where([a], a.id in ^assessment_ids)
+ |> join(:left, [a], q in assoc(a, :questions))
+ |> join(:inner, [a], ac in assoc(a, :config))
+ |> preload([a, q, ac], questions: q, config: ac)
+ |> Repo.all()
+
+ %{
+ users: users,
+ assessments: assessments,
+ submissions: submissions
+ }
  end
 
  @spec get_answers_in_submission(integer() | String.t()) ::

lib/cadet/workers/NotificationWorker.ex

@@ -73,15 +73,10 @@ 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
- )
- )
+ {:ok, %{submissions: ungraded_submissions}} =
+ Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true)
 
- if length(ungraded_submissions) < ungraded_threshold do
+ if Enum.count(ungraded_submissions) < ungraded_threshold do
  IO.puts("[AVENGER_BACKLOG] below threshold!")
  else
  IO.puts("[AVENGER_BACKLOG] SENDING_OUT")

lib/cadet_web/admin_controllers/admin_grading_controller.ex

@@ -10,11 +10,11 @@ defmodule CadetWeb.AdminGradingController do
  group = String.to_atom(group)
 
  case Assessments.all_submissions_by_grader_for_index(course_reg, group) do
- {:ok, submissions} ->
+ {:ok, view_model} ->
  conn
  |> put_status(:ok)
  |> put_resp_content_type("application/json")
- |> text(submissions)
+ |> render("gradingsummaries.json", view_model)
  end
  end
 

lib/cadet_web/admin_views/admin_grading_view.ex

@@ -7,6 +7,94 @@ defmodule CadetWeb.AdminGradingView do
  render_many(answers, CadetWeb.AdminGradingView, "grading_info.json", as: :answer)
  end
 
+ def render("gradingsummaries.json", %{
+ users: users,
+ assessments: assessments,
+ submissions: submissions
+ }) do
+ for submission <- submissions do
+ user = users |> Enum.find(&(&1.id == submission.student_id))
+ assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id))
+
+ render(
+ CadetWeb.AdminGradingView,
+ "gradingsummary.json",
+ %{
+ user: user,
+ assessment: assessment,
+ submission: submission,
+ unsubmitter:
+ case submission.unsubmitted_by_id do
+ nil -> nil
+ _ -> users |> Enum.find(&(&1.id == submission.unsubmitted_by_id))
+ end
+ }
+ )
+ end
+ end
+
+ def render("gradingsummary.json", %{
+ user: user,
+ assessment: a,
+ submission: s,
+ unsubmitter: unsubmitter
+ }) do
+ s
+ |> transform_map_for_view(%{
+ id: :id,
+ status: :status,
+ unsubmittedAt: :unsubmitted_at,
+ xp: :xp,
+ xpAdjustment: :xp_adjustment,
+ xpBonus: :xp_bonus,
+ gradedCount:
+ &case &1.graded_count do
+ nil -> 0
+ x -> x
+ end
+ })
+ |> Map.merge(%{
+ assessment:
+ render_one(a, CadetWeb.AdminGradingView, "gradingsummaryassessment.json", as: :assessment),
+ student: render_one(user, CadetWeb.AdminGradingView, "gradingsummaryuser.json", as: :cr),
+ unsubmittedBy:
+ case unsubmitter do
+ nil -> nil
+ cr -> transform_map_for_view(cr, %{id: :id, name: & &1.user.name})
+ end
+ })
+ end
+
+ def render("gradingsummaryassessment.json", %{assessment: a}) do
+ %{
+ id: a.id,
+ title: a.title,
+ assessmentNumber: a.number,
+ isManuallyGraded: a.config.is_manually_graded,
+ type: a.config.type,
+ maxXp: a.questions |> Enum.map(& &1.max_xp) |> Enum.sum(),
+ questionCount: a.questions |> Enum.count()
+ }
+ end
+
+ def render("gradingsummaryuser.json", %{cr: cr}) do
+ %{
+ id: cr.id,
+ name: cr.user.name,
+ username: cr.user.username,
+ groupName:
+ case cr.group do
+ nil -> nil
+ _ -> cr.group.name
+ end,
+ groupLeaderId:
+ case cr.group do
+ nil -> nil
+ _ -> cr.group.leader_id
+ end
+ }
+ end
+
  def render("grading_info.json", %{answer: answer}) do
  transform_map_for_view(answer, %{
  student:

priv/repo/migrations/20231105164101_create_submissions_assessment_index.exs

@@ -0,0 +1,11 @@
+defmodule Cadet.Repo.Migrations.CreateSubmissionsAssessmentIndex do
+ use Ecto.Migration
+
+ def up do
+ create(index(:submissions, [:assessment_id]))
+ end
+
+ def down do
+ drop(index(:submissions, [:assessment_id]))
+ end
+end

test/cadet/email_test.exs

@@ -24,10 +24,8 @@ defmodule Cadet.EmailTest do
  avenger_user = insert(:user, %{email: "test@gmail.com"})
  avenger = insert(:course_registration, %{user: avenger_user, role: :staff})
 
- ungraded_submissions =
- Jason.decode!(
- elem(Cadet.Assessments.all_submissions_by_grader_for_index(avenger, true, true), 1)
- )
+ {:ok, %{submissions: ungraded_submissions}} =
+ Cadet.Assessments.all_submissions_by_grader_for_index(avenger, true, true)
 
  email = Email.avenger_backlog_email("avenger_backlog", avenger_user, ungraded_submissions)
 

test/cadet/jobs/notification_worker/notification_worker_test.exs

@@ -18,10 +18,8 @@ defmodule Cadet.NotificationWorker.NotificationWorkerTest do
  submission = List.first(List.first(data.mcq_answers)).submission
 
  # setup for avenger backlog
- ungraded_submissions =
- Jason.decode!(
- elem(Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true), 1)
- )
+ {:ok, %{submissions: ungraded_submissions}} =
+ Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true)
 
  Repo.update_all(NotificationType, set: [is_enabled: true])
  Repo.update_all(NotificationConfig, set: [is_enabled: true])