diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index fc8db2902..d8f9bc67f 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/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()) :: diff --git a/lib/cadet/workers/NotificationWorker.ex b/lib/cadet/workers/NotificationWorker.ex index d96a3df22..609aab13c 100644 --- a/lib/cadet/workers/NotificationWorker.ex +++ b/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") diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index 9658b531a..caa9771a1 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/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 diff --git a/lib/cadet_web/admin_views/admin_grading_view.ex b/lib/cadet_web/admin_views/admin_grading_view.ex index 363d63ccc..128bfb59d 100644 --- a/lib/cadet_web/admin_views/admin_grading_view.ex +++ b/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: diff --git a/priv/repo/migrations/20231105164101_create_submissions_assessment_index.exs b/priv/repo/migrations/20231105164101_create_submissions_assessment_index.exs new file mode 100644 index 000000000..04016dcb1 --- /dev/null +++ b/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 diff --git a/test/cadet/email_test.exs b/test/cadet/email_test.exs index 462daad65..2ee826979 100644 --- a/test/cadet/email_test.exs +++ b/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) diff --git a/test/cadet/jobs/notification_worker/notification_worker_test.exs b/test/cadet/jobs/notification_worker/notification_worker_test.exs index 41606d4ce..626c46cac 100644 --- a/test/cadet/jobs/notification_worker/notification_worker_test.exs +++ b/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])