From 29246b00c95da3c57db92bfa4e96cbf952eb24a2 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 28 Jan 2024 09:51:22 +0800 Subject: [PATCH 01/31] feat: implement paginated submissions query function --- lib/cadet/assessments/assessments.ex | 74 ++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 6bb932160..1f3637d95 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1318,6 +1318,80 @@ defmodule Cadet.Assessments do {:ok, generate_grading_summary_view_model(submissions, course_id)} end + # ! Paginated Submissions WIP + @spec paginated_submissions_by_grader_for_index(CourseRegistration.t()) :: + {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}} + def paginated_submissions_by_grader_for_index( + grader = %CourseRegistration{course_id: course_id}, + group_only \\ false, + page \\ "0", + page_size \\ "10" + ) do + show_all = not group_only + IO.puts("page in assessments.ex: #{page} and page_size: #{page_size}") + parsed_page = elem(Integer.parse(page), 0) + parsed_page_size = elem(Integer.parse(page_size), 0) + offset = to_string(parsed_page * parsed_page_size) + group_filter = + if show_all, + do: "", + else: + "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}" + + 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} + LIMIT #{page_size} + OFFSET #{offset}; + """) 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 From a60e39f8d3c797cad2570cd4566c1abf6e55e3ef Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 28 Jan 2024 09:52:25 +0800 Subject: [PATCH 02/31] feat: implement /grading route accepting pagination query parameters --- .../admin_controllers/admin_grading_controller.ex | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index caa9771a1..d3e39eb01 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -4,9 +4,22 @@ defmodule CadetWeb.AdminGradingController do alias Cadet.Assessments - def index(conn, %{"group" => group}) when group in ["true", "false"] do + def index(conn, %{"group" => group, "page" => page, "pageSize" => page_size}) + when group in["true", "false"] do course_reg = conn.assigns[:course_reg] + group = String.to_atom(group) + case Assessments.paginated_submissions_by_grader_for_index(course_reg, group, page, page_size) do + {:ok, view_model} -> + conn + |> put_status(:ok) + |> put_resp_content_type("application/json") + |> render("gradingsummaries.json", view_model) + end + end + def index(conn, %{"group" => group}) + when group in ["true", "false"] do + course_reg = conn.assigns[:course_reg] group = String.to_atom(group) case Assessments.all_submissions_by_grader_for_index(course_reg, group) do From 29057e764f905c4af1746beb7f16a2e001a4c4bc Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Tue, 30 Jan 2024 19:41:26 +0800 Subject: [PATCH 03/31] fix: fix paginated submissions with group filter * Changed group_filter query to work with the new paginated query --- lib/cadet/assessments/assessments.ex | 66 ++++++++++++++-------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 1f3637d95..663bf89f0 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1328,7 +1328,6 @@ defmodule Cadet.Assessments do page_size \\ "10" ) do show_all = not group_only - IO.puts("page in assessments.ex: #{page} and page_size: #{page_size}") parsed_page = elem(Integer.parse(page), 0) parsed_page_size = elem(Integer.parse(page_size), 0) offset = to_string(parsed_page * parsed_page_size) @@ -1336,49 +1335,48 @@ defmodule Cadet.Assessments do if show_all, do: "", else: - "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}" + "WHERE 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}" 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 + 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 + (SELECT * FROM submissions WHERE - s.assessment_id IN ( + assessment_id IN ( SELECT id FROM assessments WHERE assessments.course_id = #{course_id} - ) #{group_filter} - LIMIT #{page_size} - OFFSET #{offset}; - """) do + ) + ORDER BY inserted_at DESC LIMIT #{page_size} OFFSET #{offset * page_size}) 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 #{group_filter}; + """) do {:ok, %{columns: columns, rows: result}} -> result |> Enum.map( From c662fbc67a8f0632f7cc561e84d0d4c635ca5506 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:47:10 +0800 Subject: [PATCH 04/31] refactor: change page to offset for pagination --- lib/cadet/assessments/assessments.ex | 7 ++----- .../admin_controllers/admin_grading_controller.ex | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 663bf89f0..aafcb4e28 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1324,13 +1324,10 @@ defmodule Cadet.Assessments do def paginated_submissions_by_grader_for_index( grader = %CourseRegistration{course_id: course_id}, group_only \\ false, - page \\ "0", + offset \\ "0", page_size \\ "10" ) do show_all = not group_only - parsed_page = elem(Integer.parse(page), 0) - parsed_page_size = elem(Integer.parse(page_size), 0) - offset = to_string(parsed_page * parsed_page_size) group_filter = if show_all, do: "", @@ -1361,7 +1358,7 @@ defmodule Cadet.Assessments do WHERE assessments.course_id = #{course_id} ) - ORDER BY inserted_at DESC LIMIT #{page_size} OFFSET #{offset * page_size}) AS s + ORDER BY inserted_at DESC LIMIT #{page_size} OFFSET #{offset}) AS s LEFT JOIN ( SELECT ans.submission_id, diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index d3e39eb01..d222ef74a 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -4,11 +4,11 @@ defmodule CadetWeb.AdminGradingController do alias Cadet.Assessments - def index(conn, %{"group" => group, "page" => page, "pageSize" => page_size}) + def index(conn, %{"group" => group, "offset" => offset, "pageSize" => page_size}) when group in["true", "false"] do course_reg = conn.assigns[:course_reg] group = String.to_atom(group) - case Assessments.paginated_submissions_by_grader_for_index(course_reg, group, page, page_size) do + case Assessments.paginated_submissions_by_grader_for_index(course_reg, group, offset, page_size) do {:ok, view_model} -> conn |> put_status(:ok) From 6adbf8c3f0611502f8fab463d401ff4390ad579d Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Fri, 9 Feb 2024 16:50:08 +0800 Subject: [PATCH 05/31] feat: Update admin grading controller to take variable amount of params --- lib/cadet/assessments/assessments.ex | 8 +++----- .../admin_controllers/admin_grading_controller.ex | 9 ++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index aafcb4e28..665849682 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1323,11 +1323,9 @@ defmodule Cadet.Assessments do {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}} def paginated_submissions_by_grader_for_index( grader = %CourseRegistration{course_id: course_id}, - group_only \\ false, - offset \\ "0", - page_size \\ "10" + params \\ %{"group" => "false", "page_size" => "10", "offset" => "0"} ) do - show_all = not group_only + show_all = not String.to_atom(params["group"]) group_filter = if show_all, do: "", @@ -1358,7 +1356,7 @@ defmodule Cadet.Assessments do WHERE assessments.course_id = #{course_id} ) - ORDER BY inserted_at DESC LIMIT #{page_size} OFFSET #{offset}) AS s + ORDER BY inserted_at DESC LIMIT 1 OFFSET 0) AS s LEFT JOIN ( SELECT ans.submission_id, diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index d222ef74a..5ffbc3998 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -4,18 +4,17 @@ defmodule CadetWeb.AdminGradingController do alias Cadet.Assessments - def index(conn, %{"group" => group, "offset" => offset, "pageSize" => page_size}) - when group in["true", "false"] do + def index(conn, %{"group" => group} = params) + when group in ["true", "false"] do course_reg = conn.assigns[:course_reg] - group = String.to_atom(group) - case Assessments.paginated_submissions_by_grader_for_index(course_reg, group, offset, page_size) do + case Assessments.paginated_submissions_by_grader_for_index(course_reg, params) do {:ok, view_model} -> conn |> put_status(:ok) |> put_resp_content_type("application/json") |> render("gradingsummaries.json", view_model) - end end + end def index(conn, %{"group" => group}) when group in ["true", "false"] do From 3b621afb467a5b1356a4cc6724ab737023b37593 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 11:55:02 +0800 Subject: [PATCH 06/31] refactor: Refactor code from using raw sql to using ecto query --- lib/cadet/assessments/assessments.ex | 103 ++++++++++++++------------- 1 file changed, 52 insertions(+), 51 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 665849682..0018b15ae 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1323,65 +1323,66 @@ defmodule Cadet.Assessments do {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}} def paginated_submissions_by_grader_for_index( grader = %CourseRegistration{course_id: course_id}, - params \\ %{"group" => "false", "page_size" => "10", "offset" => "0"} + params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} ) do show_all = not String.to_atom(params["group"]) + + # TODO Refactor group filter group_filter = if show_all, do: "", else: "WHERE 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}" - 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 - (SELECT * FROM submissions - WHERE - assessment_id IN ( - SELECT - id - FROM - assessments - WHERE - assessments.course_id = #{course_id} - ) - ORDER BY inserted_at DESC LIMIT 1 OFFSET 0) 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 #{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 - + # student_ids_query = + # from(cr in CourseRegistration, + # join: g in Group, on: cr.group_id == g.id, + # where: g.leader_id == ^grader.id, + # select: cr.id + # ) + + # query = + # from(s in Submission, + # where: s.student_id in subquery(student_ids_query) or s.student_id == ^grader.id + # ) + + submission_answers_query = + from ans in Answer, + group_by: ans.submission_id, + select: %{ + submission_id: ans.submission_id, + xp: sum(ans.xp), + xp_adjustment: sum(ans.xp_adjustment), + graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) + } + + assessments_query = + from a in Assessment, + where: a.course_id == ^course_id, + select: a.id + + # TODO param defaults not working. + query = + from s in Submission, + where: s.assessment_id in subquery(assessments_query), + order_by: [desc: s.inserted_at], + limit: ^elem(Integer.parse(params["pageSize"]), 0), + offset: ^elem(Integer.parse(params["offset"]), 0), + left_join: ans in subquery(submission_answers_query), + on: ans.submission_id == s.id, + select: %{ + id: s.id, + status: s.status, + xp_bonus: s.xp_bonus, + unsubmitted_at: s.unsubmitted_at, + unsubmitted_by_id: s.unsubmitted_by_id, + student_id: s.student_id, + assessment_id: s.assessment_id, + xp: ans.xp, + xp_adjustment: ans.xp_adjustment, + graded_count: ans.graded_count + } + submissions = Repo.all(query) {:ok, generate_grading_summary_view_model(submissions, course_id)} end From fc55c5544c8c9f0bde28089ef7ae16bea90b4626 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 11:57:16 +0800 Subject: [PATCH 07/31] feat: Implement filtering by mission name --- lib/cadet/assessments/assessments.ex | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 0018b15ae..b7a63836c 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1356,9 +1356,13 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } + # TODO submission filtering + # submission_query = + assessments_query = from a in Assessment, where: a.course_id == ^course_id, + where: ^build_assessment_filter(params), select: a.id # TODO param defaults not working. @@ -1386,6 +1390,16 @@ defmodule Cadet.Assessments do {:ok, generate_grading_summary_view_model(submissions, course_id)} end + defp build_assessment_filter(params) do + Enum.reduce(params, dynamic(true), fn + {"title", value}, dynamic -> + dynamic([assessment], ^dynamic and assessment.title == ^value) + + # TODO grading progress filter + {_, _}, dynamic -> dynamic + end) + end + defp generate_grading_summary_view_model(submissions, course_id) do users = CourseRegistration From 610ffbfc3de371a1a7af69d970638078134c0877 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 12:13:25 +0800 Subject: [PATCH 08/31] feat: Implement filtering by progress for submission --- lib/cadet/assessments/assessments.ex | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index b7a63836c..beb564e27 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1356,9 +1356,6 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } - # TODO submission filtering - # submission_query = - assessments_query = from a in Assessment, where: a.course_id == ^course_id, @@ -1369,6 +1366,7 @@ defmodule Cadet.Assessments do query = from s in Submission, where: s.assessment_id in subquery(assessments_query), + where: ^build_submission_filter(params), order_by: [desc: s.inserted_at], limit: ^elem(Integer.parse(params["pageSize"]), 0), offset: ^elem(Integer.parse(params["offset"]), 0), @@ -1400,6 +1398,16 @@ defmodule Cadet.Assessments do end) end + defp build_submission_filter(params) do + Enum.reduce(params, dynamic(true), fn + {"status", value}, dynamic -> + dynamic([submission], ^dynamic and submission.status == ^value) + + # TODO graded/ungraded for submission + {_, _}, dynamic -> dynamic + end) + end + defp generate_grading_summary_view_model(submissions, course_id) do users = CourseRegistration From 7de988220f75a154fbf8d4c328df32efb2b224ff Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 12:30:13 +0800 Subject: [PATCH 09/31] refactor: show all and show only grader's groups now use ecto query --- lib/cadet/assessments/assessments.ex | 36 +++++++++++++--------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index beb564e27..0552ec692 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1325,26 +1325,6 @@ defmodule Cadet.Assessments do grader = %CourseRegistration{course_id: course_id}, params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} ) do - show_all = not String.to_atom(params["group"]) - - # TODO Refactor group filter - group_filter = - if show_all, - do: "", - else: - "WHERE 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}" - - # student_ids_query = - # from(cr in CourseRegistration, - # join: g in Group, on: cr.group_id == g.id, - # where: g.leader_id == ^grader.id, - # select: cr.id - # ) - - # query = - # from(s in Submission, - # where: s.student_id in subquery(student_ids_query) or s.student_id == ^grader.id - # ) submission_answers_query = from ans in Answer, @@ -1367,6 +1347,7 @@ defmodule Cadet.Assessments do from s in Submission, where: s.assessment_id in subquery(assessments_query), where: ^build_submission_filter(params), + where: ^build_group_filter(grader, params), order_by: [desc: s.inserted_at], limit: ^elem(Integer.parse(params["pageSize"]), 0), offset: ^elem(Integer.parse(params["offset"]), 0), @@ -1408,6 +1389,21 @@ defmodule Cadet.Assessments do end) end + defp build_group_filter(grader, params) do + group_query = + from(cr in CourseRegistration, + join: g in Group, on: cr.group_id == g.id, + where: g.leader_id == ^grader.id, + select: cr.id + ) + Enum.reduce(params, dynamic(true), fn + {"group", "true"}, dynamic -> + dynamic([submission], ^dynamic and submission.student_id in subquery(group_query) or submission.student_id == ^grader.id) + + {_, _}, dynamic -> dynamic + end) + end + defp generate_grading_summary_view_model(submissions, course_id) do users = CourseRegistration From c9ba93ea160897399a41cdfc40bfc0c08306126f Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 12:36:44 +0800 Subject: [PATCH 10/31] feat: Implement filter by group id --- lib/cadet/assessments/assessments.ex | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 0552ec692..0b26929f3 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1390,16 +1390,19 @@ defmodule Cadet.Assessments do end defp build_group_filter(grader, params) do - group_query = - from(cr in CourseRegistration, + Enum.reduce(params, dynamic(true), fn + {"group", "true"}, dynamic -> + dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, join: g in Group, on: cr.group_id == g.id, where: g.leader_id == ^grader.id, select: cr.id - ) - Enum.reduce(params, dynamic(true), fn - {"group", "true"}, dynamic -> - dynamic([submission], ^dynamic and submission.student_id in subquery(group_query) or submission.student_id == ^grader.id) + )) or submission.student_id == ^grader.id) + {"groupID", value}, dynamic -> + dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, + where: cr.group_id == ^value, + select: cr.id + ))) {_, _}, dynamic -> dynamic end) end From 451cea8424c9167cbb5bb993989fccfbcc7e949e Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 14:14:06 +0800 Subject: [PATCH 11/31] fix: Fix limit and offset default for missing query param --- lib/cadet/assessments/assessments.ex | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 0b26929f3..d0e67360d 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1342,15 +1342,14 @@ defmodule Cadet.Assessments do where: ^build_assessment_filter(params), select: a.id - # TODO param defaults not working. query = from s in Submission, where: s.assessment_id in subquery(assessments_query), where: ^build_submission_filter(params), where: ^build_group_filter(grader, params), order_by: [desc: s.inserted_at], - limit: ^elem(Integer.parse(params["pageSize"]), 0), - offset: ^elem(Integer.parse(params["offset"]), 0), + limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), + offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), left_join: ans in subquery(submission_answers_query), on: ans.submission_id == s.id, select: %{ From 940e282d5f1211c7ed448ef723137b5b406eb232 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:46:01 +0800 Subject: [PATCH 12/31] refactor: Rename group filter to course registration filter --- lib/cadet/assessments/assessments.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index d0e67360d..66a087b5e 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1346,7 +1346,7 @@ defmodule Cadet.Assessments do from s in Submission, where: s.assessment_id in subquery(assessments_query), where: ^build_submission_filter(params), - where: ^build_group_filter(grader, params), + where: ^build_course_registration_filter(grader, params), order_by: [desc: s.inserted_at], limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), @@ -1388,7 +1388,7 @@ defmodule Cadet.Assessments do end) end - defp build_group_filter(grader, params) do + defp build_course_registration_filter(grader, params) do Enum.reduce(params, dynamic(true), fn {"group", "true"}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, From 2f5c910683e8f63ac06efd2a4d10aa43c7858bbf Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 18:07:46 +0800 Subject: [PATCH 13/31] feat: Implement filter by name --- lib/cadet/assessments/assessments.ex | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 66a087b5e..0f7c19b9a 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1345,6 +1345,7 @@ defmodule Cadet.Assessments do query = from s in Submission, where: s.assessment_id in subquery(assessments_query), + where: ^build_user_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), order_by: [desc: s.inserted_at], @@ -1406,6 +1407,24 @@ defmodule Cadet.Assessments do end) end + defp build_user_filter(params) do + Enum.reduce(params, dynamic(true), fn + {"name", value}, dynamic -> + dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, + where: ilike(user.name, ^"%#{value}%"), + select: user.id + ))) + + {"username", value}, dynamic -> + dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, + where: ilike(user.username, ^"%#{value}%"), + select: user.id + ))) + + {_, _}, dynamic -> dynamic + end) + end + defp generate_grading_summary_view_model(submissions, course_id) do users = CourseRegistration From ca768aa3c11912d752283547f8af6d67c48c7adb Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 11 Feb 2024 18:27:45 +0800 Subject: [PATCH 14/31] feat: Implement filter by assessment type --- lib/cadet/assessments/assessments.ex | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 0f7c19b9a..ea1cc3697 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1346,6 +1346,7 @@ defmodule Cadet.Assessments do from s in Submission, where: s.assessment_id in subquery(assessments_query), where: ^build_user_filter(params), + where: ^build_assessment_config_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), order_by: [desc: s.inserted_at], @@ -1425,6 +1426,19 @@ defmodule Cadet.Assessments do end) end + defp build_assessment_config_filter(params) do + Enum.reduce(params, dynamic(true), fn + {"type", value}, dynamic -> + dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, + inner_join: assessment_config in AssessmentConfig, on: assessment.config_id == assessment_config.id, + where: assessment_config.type == ^value, + select: assessment.id + ))) + + {_, _}, dynamic -> dynamic + end) + end + defp generate_grading_summary_view_model(submissions, course_id) do users = CourseRegistration From 9ff410839f5ca446d60bd7a1680cdc12c3da5510 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 12 Feb 2024 15:17:01 +0800 Subject: [PATCH 15/31] Implement filter by is_manually_graded --- lib/cadet/assessments/assessments.ex | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index ea1cc3697..fbc5cdc0c 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1336,15 +1336,23 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } + # TODO refactor this code assessments_query = from a in Assessment, where: a.course_id == ^course_id, where: ^build_assessment_filter(params), select: a.id + # assessment_config_query = + # from a in Assessment, + # inner_join: assessment_config in AssessmentConfig, on: a.config_id == assessment_config.id, + # where: assessment_config.type == ^value, + # select: a.id + query = from s in Submission, where: s.assessment_id in subquery(assessments_query), + # where: s.assessment_id in subquery(assessment_config_query), where: ^build_user_filter(params), where: ^build_assessment_config_filter(params), where: ^build_submission_filter(params), @@ -1385,7 +1393,6 @@ defmodule Cadet.Assessments do {"status", value}, dynamic -> dynamic([submission], ^dynamic and submission.status == ^value) - # TODO graded/ungraded for submission {_, _}, dynamic -> dynamic end) end @@ -1435,6 +1442,15 @@ defmodule Cadet.Assessments do select: assessment.id ))) + #TODO Refactor code + {"isManuallyGraded", value}, dynamic -> + # dynamic([assessment_config], ^dynamic and assessment_config.is_manually_graded == ^value) + dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, + inner_join: assessment_config in AssessmentConfig, on: assessment.config_id == assessment_config.id, + where: assessment_config.is_manually_graded == ^value, + select: assessment.id + ))) + {_, _}, dynamic -> dynamic end) end From 8afcc22de58d08a724f4ebc2aa3dd8f02deed3ae Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Tue, 13 Feb 2024 19:12:36 +0800 Subject: [PATCH 16/31] feat: Include total count of submissions in response --- lib/cadet/assessments/assessments.ex | 21 ++++++++++++++++--- .../admin_views/admin_grading_view.ex | 15 ++++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index fbc5cdc0c..62494d661 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1320,7 +1320,7 @@ defmodule Cadet.Assessments do # ! Paginated Submissions WIP @spec paginated_submissions_by_grader_for_index(CourseRegistration.t()) :: - {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}} + {:ok, %{:count => integer, :data => %{:assessments => [any()], :submissions => [any()], :users => [any()]}}} def paginated_submissions_by_grader_for_index( grader = %CourseRegistration{course_id: course_id}, params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} @@ -1375,7 +1375,20 @@ defmodule Cadet.Assessments do graded_count: ans.graded_count } submissions = Repo.all(query) - {:ok, generate_grading_summary_view_model(submissions, course_id)} + + count_query = + from s in Submission, + where: s.assessment_id in subquery(assessments_query), + # where: s.assessment_id in subquery(assessment_config_query), + where: ^build_user_filter(params), + where: ^build_assessment_config_filter(params), + where: ^build_submission_filter(params), + where: ^build_course_registration_filter(grader, params), + select: count(s.id) + + count = Repo.one(count_query) + + {:ok, %{count: count, data: generate_grading_summary_view_model(submissions, course_id)}} end defp build_assessment_filter(params) do @@ -1399,6 +1412,7 @@ defmodule Cadet.Assessments do defp build_course_registration_filter(grader, params) do Enum.reduce(params, dynamic(true), fn + # TODO Refactor code {"group", "true"}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, join: g in Group, on: cr.group_id == g.id, @@ -1417,6 +1431,7 @@ defmodule Cadet.Assessments do defp build_user_filter(params) do Enum.reduce(params, dynamic(true), fn + # TODO Refactor code {"name", value}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, where: ilike(user.name, ^"%#{value}%"), @@ -1435,6 +1450,7 @@ defmodule Cadet.Assessments do defp build_assessment_config_filter(params) do Enum.reduce(params, dynamic(true), fn + # TODO Refactor code {"type", value}, dynamic -> dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, inner_join: assessment_config in AssessmentConfig, on: assessment.config_id == assessment_config.id, @@ -1442,7 +1458,6 @@ defmodule Cadet.Assessments do select: assessment.id ))) - #TODO Refactor code {"isManuallyGraded", value}, dynamic -> # dynamic([assessment_config], ^dynamic and assessment_config.is_manually_graded == ^value) dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, diff --git a/lib/cadet_web/admin_views/admin_grading_view.ex b/lib/cadet_web/admin_views/admin_grading_view.ex index 128bfb59d..a145e1f61 100644 --- a/lib/cadet_web/admin_views/admin_grading_view.ex +++ b/lib/cadet_web/admin_views/admin_grading_view.ex @@ -8,11 +8,16 @@ defmodule CadetWeb.AdminGradingView do end def render("gradingsummaries.json", %{ - users: users, - assessments: assessments, - submissions: submissions + count: count, + data: %{ + users: users, + assessments: assessments, + submissions: submissions + } }) do - for submission <- submissions do + %{ + count: count, + data: for submission <- submissions do user = users |> Enum.find(&(&1.id == submission.student_id)) assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id)) @@ -30,7 +35,7 @@ defmodule CadetWeb.AdminGradingView do end } ) - end + end} end def render("gradingsummary.json", %{ From c7dc2ca61d8ca46eb438c67a4585f64a69b48ca9 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:12:43 +0800 Subject: [PATCH 17/31] refactor: Use one join query for assessment config queries --- lib/cadet/assessments/assessments.ex | 33 ++++++++++------------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 62494d661..a760810ef 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1343,18 +1343,18 @@ defmodule Cadet.Assessments do where: ^build_assessment_filter(params), select: a.id - # assessment_config_query = - # from a in Assessment, - # inner_join: assessment_config in AssessmentConfig, on: a.config_id == assessment_config.id, - # where: assessment_config.type == ^value, - # select: a.id + assessment_config_query = + from a in Assessment, + inner_join: config in AssessmentConfig, on: a.config_id == config.id, as: :assessment_config, + where: ^build_assessment_config_filter(params), + select: a.id + # TODO See if can combine all into one where query query = from s in Submission, where: s.assessment_id in subquery(assessments_query), - # where: s.assessment_id in subquery(assessment_config_query), + where: s.assessment_id in subquery(assessment_config_query), where: ^build_user_filter(params), - where: ^build_assessment_config_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), order_by: [desc: s.inserted_at], @@ -1379,9 +1379,8 @@ defmodule Cadet.Assessments do count_query = from s in Submission, where: s.assessment_id in subquery(assessments_query), - # where: s.assessment_id in subquery(assessment_config_query), + where: s.assessment_id in subquery(assessment_config_query), where: ^build_user_filter(params), - where: ^build_assessment_config_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), select: count(s.id) @@ -1396,7 +1395,6 @@ defmodule Cadet.Assessments do {"title", value}, dynamic -> dynamic([assessment], ^dynamic and assessment.title == ^value) - # TODO grading progress filter {_, _}, dynamic -> dynamic end) end @@ -1450,21 +1448,12 @@ defmodule Cadet.Assessments do defp build_assessment_config_filter(params) do Enum.reduce(params, dynamic(true), fn - # TODO Refactor code + {"type", value}, dynamic -> - dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, - inner_join: assessment_config in AssessmentConfig, on: assessment.config_id == assessment_config.id, - where: assessment_config.type == ^value, - select: assessment.id - ))) + dynamic([assessment_config: assessment_config], ^dynamic and assessment_config.type == ^value) {"isManuallyGraded", value}, dynamic -> - # dynamic([assessment_config], ^dynamic and assessment_config.is_manually_graded == ^value) - dynamic([submission], ^dynamic and submission.assessment_id in subquery(from(assessment in Assessment, - inner_join: assessment_config in AssessmentConfig, on: assessment.config_id == assessment_config.id, - where: assessment_config.is_manually_graded == ^value, - select: assessment.id - ))) + dynamic([assessment_config: assessment_config], ^dynamic and assessment_config.is_manually_graded == ^value) {_, _}, dynamic -> dynamic end) From 2e9b1ea976129dadd03e3adfa658497c185d64c2 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 14 Feb 2024 19:28:49 +0800 Subject: [PATCH 18/31] refactor: Use one join query for user queries --- lib/cadet/assessments/assessments.ex | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index a760810ef..db1b297af 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1336,7 +1336,6 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } - # TODO refactor this code assessments_query = from a in Assessment, where: a.course_id == ^course_id, @@ -1349,11 +1348,18 @@ defmodule Cadet.Assessments do where: ^build_assessment_config_filter(params), select: a.id - # TODO See if can combine all into one where query + user_query = + from user in User, + where: ^build_user_filter(params), + select: user.id + + # TODO Reorganise such that queries which likely filters the most are on top + # TODO Split submission query from limit so we don't repeat it in count query = from s in Submission, where: s.assessment_id in subquery(assessments_query), where: s.assessment_id in subquery(assessment_config_query), + # where: s.student_id in subquery(user_query), # TODO this is breaking the test where: ^build_user_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), @@ -1380,6 +1386,7 @@ defmodule Cadet.Assessments do from s in Submission, where: s.assessment_id in subquery(assessments_query), where: s.assessment_id in subquery(assessment_config_query), + # where: s.student_id in subquery(user_query), # TODO this is breaking the test where: ^build_user_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(grader, params), @@ -1429,7 +1436,15 @@ defmodule Cadet.Assessments do defp build_user_filter(params) do Enum.reduce(params, dynamic(true), fn - # TODO Refactor code + # TODO Breaking Change + # {"name", value}, dynamic -> + # dynamic([user], ^dynamic and ilike(user.name, ^"%#{value}%") + # ) + + # {"username", value}, dynamic -> + # dynamic([user], ^dynamic and ilike(user.username, ^"%#{value}%") + # ) + {"name", value}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, where: ilike(user.name, ^"%#{value}%"), @@ -1450,10 +1465,10 @@ defmodule Cadet.Assessments do Enum.reduce(params, dynamic(true), fn {"type", value}, dynamic -> - dynamic([assessment_config: assessment_config], ^dynamic and assessment_config.type == ^value) + dynamic([assessment_config: config], ^dynamic and config.type == ^value) {"isManuallyGraded", value}, dynamic -> - dynamic([assessment_config: assessment_config], ^dynamic and assessment_config.is_manually_graded == ^value) + dynamic([assessment_config: config], ^dynamic and config.is_manually_graded == ^value) {_, _}, dynamic -> dynamic end) From 70b7f59707405f947f44bedfe665ab6cac90e370 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:26:38 +0800 Subject: [PATCH 19/31] refactor: Move queries for assessment and assessment config to builder --- lib/cadet/assessments/assessments.ex | 64 +++++++++++----------------- 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index db1b297af..c4167a044 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1336,33 +1336,15 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } - assessments_query = - from a in Assessment, - where: a.course_id == ^course_id, - where: ^build_assessment_filter(params), - select: a.id - assessment_config_query = - from a in Assessment, - inner_join: config in AssessmentConfig, on: a.config_id == config.id, as: :assessment_config, - where: ^build_assessment_config_filter(params), - select: a.id - - user_query = - from user in User, - where: ^build_user_filter(params), - select: user.id - - # TODO Reorganise such that queries which likely filters the most are on top - # TODO Split submission query from limit so we don't repeat it in count + # Queries which likely filters more submissions are on top. query = from s in Submission, - where: s.assessment_id in subquery(assessments_query), - where: s.assessment_id in subquery(assessment_config_query), - # where: s.student_id in subquery(user_query), # TODO this is breaking the test where: ^build_user_filter(params), + where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), + where: s.assessment_id in subquery(build_assessment_config_filter(params)), where: ^build_submission_filter(params), - where: ^build_course_registration_filter(grader, params), + where: ^build_course_registration_filter(params, grader), order_by: [desc: s.inserted_at], limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), @@ -1384,12 +1366,11 @@ defmodule Cadet.Assessments do count_query = from s in Submission, - where: s.assessment_id in subquery(assessments_query), - where: s.assessment_id in subquery(assessment_config_query), - # where: s.student_id in subquery(user_query), # TODO this is breaking the test + where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), + where: s.assessment_id in subquery(build_assessment_config_filter(params)), where: ^build_user_filter(params), where: ^build_submission_filter(params), - where: ^build_course_registration_filter(grader, params), + where: ^build_course_registration_filter(params, grader), select: count(s.id) count = Repo.one(count_query) @@ -1397,13 +1378,20 @@ defmodule Cadet.Assessments do {:ok, %{count: count, data: generate_grading_summary_view_model(submissions, course_id)}} end - defp build_assessment_filter(params) do - Enum.reduce(params, dynamic(true), fn + defp build_assessment_filter(params, course_id) do + + assessments_filters = + Enum.reduce(params, dynamic(true), fn {"title", value}, dynamic -> dynamic([assessment], ^dynamic and assessment.title == ^value) {_, _}, dynamic -> dynamic end) + + from a in Assessment, + where: a.course_id == ^course_id, + where: ^assessments_filters, + select: a.id end defp build_submission_filter(params) do @@ -1415,9 +1403,9 @@ defmodule Cadet.Assessments do end) end - defp build_course_registration_filter(grader, params) do + defp build_course_registration_filter(params, grader) do Enum.reduce(params, dynamic(true), fn - # TODO Refactor code + {"group", "true"}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, join: g in Group, on: cr.group_id == g.id, @@ -1436,14 +1424,6 @@ defmodule Cadet.Assessments do defp build_user_filter(params) do Enum.reduce(params, dynamic(true), fn - # TODO Breaking Change - # {"name", value}, dynamic -> - # dynamic([user], ^dynamic and ilike(user.name, ^"%#{value}%") - # ) - - # {"username", value}, dynamic -> - # dynamic([user], ^dynamic and ilike(user.username, ^"%#{value}%") - # ) {"name", value}, dynamic -> dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, @@ -1462,7 +1442,8 @@ defmodule Cadet.Assessments do end defp build_assessment_config_filter(params) do - Enum.reduce(params, dynamic(true), fn + + assessment_config_filters = Enum.reduce(params, dynamic(true), fn {"type", value}, dynamic -> dynamic([assessment_config: config], ^dynamic and config.type == ^value) @@ -1472,6 +1453,11 @@ defmodule Cadet.Assessments do {_, _}, dynamic -> dynamic end) + + from a in Assessment, + inner_join: config in AssessmentConfig, on: a.config_id == config.id, as: :assessment_config, + where: ^assessment_config_filters, + select: a.id end defp generate_grading_summary_view_model(submissions, course_id) do From f5f651dcf812e8fb43dd40923dcb3f89f449584c Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:28:17 +0800 Subject: [PATCH 20/31] fix: Update admin_grading_controller test to account for change in response --- .../admin_grading_controller_test.exs | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs index 2c08e27d7..9b159fa56 100644 --- a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs @@ -113,7 +113,7 @@ defmodule CadetWeb.AdminGradingControllerTest do conn = get(conn, build_url(course.id)) - expected = + expected = %{"count" => length(submissions), "data" => Enum.map(submissions, fn submission -> %{ "xp" => 5000, @@ -142,8 +142,10 @@ defmodule CadetWeb.AdminGradingControllerTest do "unsubmittedAt" => nil } end) + } - assert expected == Enum.sort_by(json_response(conn, 200), & &1["id"]) + res = json_response(conn, 200) + assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} end end @@ -161,7 +163,7 @@ defmodule CadetWeb.AdminGradingControllerTest do |> get(build_url(test_cr.course_id), %{"group" => "true"}) |> json_response(200) - assert resp == [] + assert resp == %{"count" => 0, "data" => []} end @tag authenticate: :staff @@ -177,8 +179,7 @@ defmodule CadetWeb.AdminGradingControllerTest do seed_db(conn, new_staff) conn = get(conn, build_url(course.id), %{"group" => "true"}) - - expected = + expected = %{"count" => length(submissions), "data" => Enum.map(submissions, fn submission -> %{ "xp" => 5000, @@ -207,8 +208,9 @@ defmodule CadetWeb.AdminGradingControllerTest do "unsubmittedAt" => nil } end) - - assert expected == Enum.sort_by(json_response(conn, 200), & &1["id"]) + } + res = json_response(conn, 200) + assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} end end @@ -780,7 +782,7 @@ defmodule CadetWeb.AdminGradingControllerTest do |> sign_in(admin.user) |> get(build_url(course.id)) - expected = + expected = %{ "count" => length(submissions), "data" => Enum.map(submissions, fn submission -> %{ "xp" => 5000, @@ -809,8 +811,9 @@ defmodule CadetWeb.AdminGradingControllerTest do "unsubmittedAt" => nil } end) - - assert expected == Enum.sort_by(json_response(conn, 200), & &1["id"]) + } + res = json_response(conn, 200) + assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} end end @@ -825,7 +828,7 @@ defmodule CadetWeb.AdminGradingControllerTest do conn = get(conn, build_url(course.id), %{"group" => "true"}) - expected = + expected = %{ "count" => length(submissions), "data" => Enum.map(submissions, fn submission -> %{ "xp" => 5000, @@ -854,8 +857,10 @@ defmodule CadetWeb.AdminGradingControllerTest do "unsubmittedAt" => nil } end) + } - assert expected == Enum.sort_by(json_response(conn, 200), & &1["id"]) + res = json_response(conn, 200) + assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} end end From 37c7a4efa263d1770699a2da6976d28357abe5f9 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:38:22 +0800 Subject: [PATCH 21/31] refactor: Rename function --- lib/cadet/assessments/assessments.ex | 191 +++++++++++------- .../admin_grading_controller.ex | 17 +- 2 files changed, 123 insertions(+), 85 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index c4167a044..44dbea5cf 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1318,16 +1318,19 @@ defmodule Cadet.Assessments do {:ok, generate_grading_summary_view_model(submissions, course_id)} end - # ! Paginated Submissions WIP - @spec paginated_submissions_by_grader_for_index(CourseRegistration.t()) :: - {:ok, %{:count => integer, :data => %{:assessments => [any()], :submissions => [any()], :users => [any()]}}} - def paginated_submissions_by_grader_for_index( - grader = %CourseRegistration{course_id: course_id}, - params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} - ) do + @spec submissions_by_grader_for_index(CourseRegistration.t()) :: + {:ok, + %{ + :count => integer, + :data => %{:assessments => [any()], :submissions => [any()], :users => [any()]} + }} + def submissions_by_grader_for_index( + grader = %CourseRegistration{course_id: course_id}, + params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} + ) do submission_answers_query = - from ans in Answer, + from(ans in Answer, group_by: ans.submission_id, select: %{ submission_id: ans.submission_id, @@ -1335,43 +1338,46 @@ defmodule Cadet.Assessments do xp_adjustment: sum(ans.xp_adjustment), graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } + ) - # Queries which likely filters more submissions are on top. query = - from s in Submission, - where: ^build_user_filter(params), - where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), - where: s.assessment_id in subquery(build_assessment_config_filter(params)), - where: ^build_submission_filter(params), - where: ^build_course_registration_filter(params, grader), - order_by: [desc: s.inserted_at], - limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), - offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), - left_join: ans in subquery(submission_answers_query), - on: ans.submission_id == s.id, - select: %{ - id: s.id, - status: s.status, - xp_bonus: s.xp_bonus, - unsubmitted_at: s.unsubmitted_at, - unsubmitted_by_id: s.unsubmitted_by_id, - student_id: s.student_id, - assessment_id: s.assessment_id, - xp: ans.xp, - xp_adjustment: ans.xp_adjustment, - graded_count: ans.graded_count - } + from(s in Submission, + where: ^build_user_filter(params), + where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), + where: s.assessment_id in subquery(build_assessment_config_filter(params)), + where: ^build_submission_filter(params), + where: ^build_course_registration_filter(params, grader), + order_by: [desc: s.inserted_at], + limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), + offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), + left_join: ans in subquery(submission_answers_query), + on: ans.submission_id == s.id, + select: %{ + id: s.id, + status: s.status, + xp_bonus: s.xp_bonus, + unsubmitted_at: s.unsubmitted_at, + unsubmitted_by_id: s.unsubmitted_by_id, + student_id: s.student_id, + assessment_id: s.assessment_id, + xp: ans.xp, + xp_adjustment: ans.xp_adjustment, + graded_count: ans.graded_count + } + ) + submissions = Repo.all(query) count_query = - from s in Submission, + from(s in Submission, where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), where: s.assessment_id in subquery(build_assessment_config_filter(params)), where: ^build_user_filter(params), where: ^build_submission_filter(params), where: ^build_course_registration_filter(params, grader), select: count(s.id) + ) count = Repo.one(count_query) @@ -1379,19 +1385,20 @@ defmodule Cadet.Assessments do end defp build_assessment_filter(params, course_id) do - assessments_filters = Enum.reduce(params, dynamic(true), fn - {"title", value}, dynamic -> - dynamic([assessment], ^dynamic and assessment.title == ^value) + {"title", value}, dynamic -> + dynamic([assessment], ^dynamic and assessment.title == ^value) - {_, _}, dynamic -> dynamic - end) + {_, _}, dynamic -> + dynamic + end) - from a in Assessment, - where: a.course_id == ^course_id, - where: ^assessments_filters, - select: a.id + from(a in Assessment, + where: a.course_id == ^course_id, + where: ^assessments_filters, + select: a.id + ) end defp build_submission_filter(params) do @@ -1399,65 +1406,95 @@ defmodule Cadet.Assessments do {"status", value}, dynamic -> dynamic([submission], ^dynamic and submission.status == ^value) - {_, _}, dynamic -> dynamic + {_, _}, dynamic -> + dynamic end) end defp build_course_registration_filter(params, grader) do Enum.reduce(params, dynamic(true), fn - {"group", "true"}, dynamic -> - dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, - join: g in Group, on: cr.group_id == g.id, - where: g.leader_id == ^grader.id, - select: cr.id - )) or submission.student_id == ^grader.id) + dynamic( + [submission], + (^dynamic and + submission.student_id in subquery( + from(cr in CourseRegistration, + join: g in Group, + on: cr.group_id == g.id, + where: g.leader_id == ^grader.id, + select: cr.id + ) + )) or submission.student_id == ^grader.id + ) {"groupID", value}, dynamic -> - dynamic([submission], ^dynamic and submission.student_id in subquery(from(cr in CourseRegistration, - where: cr.group_id == ^value, - select: cr.id - ))) - {_, _}, dynamic -> dynamic + dynamic( + [submission], + ^dynamic and + submission.student_id in subquery( + from(cr in CourseRegistration, + where: cr.group_id == ^value, + select: cr.id + ) + ) + ) + + {_, _}, dynamic -> + dynamic end) end defp build_user_filter(params) do Enum.reduce(params, dynamic(true), fn - {"name", value}, dynamic -> - dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, - where: ilike(user.name, ^"%#{value}%"), - select: user.id - ))) + dynamic( + [submission], + ^dynamic and + submission.student_id in subquery( + from(user in User, + where: ilike(user.name, ^"%#{value}%"), + select: user.id + ) + ) + ) {"username", value}, dynamic -> - dynamic([submission], ^dynamic and submission.student_id in subquery(from(user in User, - where: ilike(user.username, ^"%#{value}%"), - select: user.id - ))) + dynamic( + [submission], + ^dynamic and + submission.student_id in subquery( + from(user in User, + where: ilike(user.username, ^"%#{value}%"), + select: user.id + ) + ) + ) - {_, _}, dynamic -> dynamic + {_, _}, dynamic -> + dynamic end) end defp build_assessment_config_filter(params) do + assessment_config_filters = + Enum.reduce(params, dynamic(true), fn + {"type", value}, dynamic -> + dynamic([assessment_config: config], ^dynamic and config.type == ^value) - assessment_config_filters = Enum.reduce(params, dynamic(true), fn - - {"type", value}, dynamic -> - dynamic([assessment_config: config], ^dynamic and config.type == ^value) - - {"isManuallyGraded", value}, dynamic -> - dynamic([assessment_config: config], ^dynamic and config.is_manually_graded == ^value) + {"isManuallyGraded", value}, dynamic -> + dynamic([assessment_config: config], ^dynamic and config.is_manually_graded == ^value) - {_, _}, dynamic -> dynamic - end) + {_, _}, dynamic -> + dynamic + end) - from a in Assessment, - inner_join: config in AssessmentConfig, on: a.config_id == config.id, as: :assessment_config, - where: ^assessment_config_filters, - select: a.id + from(a in Assessment, + inner_join: config in AssessmentConfig, + on: a.config_id == config.id, + as: :assessment_config, + where: ^assessment_config_filters, + select: a.id + ) end defp generate_grading_summary_view_model(submissions, course_id) do diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index 5ffbc3998..fb1c4881f 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -5,19 +5,20 @@ defmodule CadetWeb.AdminGradingController do alias Cadet.Assessments def index(conn, %{"group" => group} = params) - when group in ["true", "false"] do + when group in ["true", "false"] do course_reg = conn.assigns[:course_reg] - case Assessments.paginated_submissions_by_grader_for_index(course_reg, params) do - {:ok, view_model} -> - conn - |> put_status(:ok) - |> put_resp_content_type("application/json") - |> render("gradingsummaries.json", view_model) + + case Assessments.submissions_by_grader_for_index(course_reg, params) do + {:ok, view_model} -> + conn + |> put_status(:ok) + |> put_resp_content_type("application/json") + |> render("gradingsummaries.json", view_model) end end def index(conn, %{"group" => group}) - when group in ["true", "false"] do + when group in ["true", "false"] do course_reg = conn.assigns[:course_reg] group = String.to_atom(group) From b7ea15b9c0c4bc2da7d3d047965f36682e81d1b3 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:39:14 +0800 Subject: [PATCH 22/31] docs: Include a description of the function --- lib/cadet/assessments/assessments.ex | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 44dbea5cf..b5b495c5d 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1318,7 +1318,22 @@ defmodule Cadet.Assessments do {:ok, generate_grading_summary_view_model(submissions, course_id)} end + @doc """ + Function returning submissions under a grader. This function returns only the + fields that are exposed in the /grading endpoint. + + The input parameters are the user and query parameters. Query parameters are + used to filter the submissions. Queries are arranged such that filters which + likely filter more submissions are on top. + The group parameter is used to check whether only the groups under the grader + should be returned. If pageSize and offset are not provided, the default + values are 10 and 0 respectively. + + The return value is + {:ok, %{"count": count, "data": submissions}} if no errors, + else it is {:error, {:forbidden, "Forbidden."}} + """ @spec submissions_by_grader_for_index(CourseRegistration.t()) :: {:ok, %{ From 0fd3e6539cb8ec836a47c3ab317053928df838b4 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:40:02 +0800 Subject: [PATCH 23/31] refactor: Format code --- lib/cadet/assessments/assessments.ex | 1 - .../admin_views/admin_grading_view.ex | 40 +-- .../admin_grading_controller_test.exs | 267 ++++++++++-------- 3 files changed, 168 insertions(+), 140 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index b5b495c5d..275fade7d 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1355,7 +1355,6 @@ defmodule Cadet.Assessments do } ) - query = from(s in Submission, where: ^build_user_filter(params), diff --git a/lib/cadet_web/admin_views/admin_grading_view.ex b/lib/cadet_web/admin_views/admin_grading_view.ex index a145e1f61..29c493a35 100644 --- a/lib/cadet_web/admin_views/admin_grading_view.ex +++ b/lib/cadet_web/admin_views/admin_grading_view.ex @@ -13,29 +13,31 @@ defmodule CadetWeb.AdminGradingView do users: users, assessments: assessments, submissions: submissions - } + } }) do %{ count: count, - data: for submission <- submissions do - user = users |> Enum.find(&(&1.id == submission.student_id)) - assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id)) + data: + 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} + 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", %{ diff --git a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs index 9b159fa56..cf46bcd45 100644 --- a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs @@ -113,39 +113,45 @@ defmodule CadetWeb.AdminGradingControllerTest do conn = get(conn, build_url(course.id)) - expected = %{"count" => length(submissions), "data" => - Enum.map(submissions, fn submission -> - %{ - "xp" => 5000, - "xpAdjustment" => -2500, - "xpBonus" => 100, - "id" => submission.id, - "student" => %{ - "name" => submission.student.user.name, - "username" => submission.student.user.username, - "id" => submission.student.id, - "groupName" => submission.student.group.name, - "groupLeaderId" => submission.student.group.leader_id - }, - "assessment" => %{ - "type" => mission.config.type, - "isManuallyGraded" => mission.config.is_manually_graded, - "maxXp" => 5000, - "id" => mission.id, - "title" => mission.title, - "questionCount" => 5, - "assessmentNumber" => mission.number - }, - "status" => Atom.to_string(submission.status), - "gradedCount" => 5, - "unsubmittedBy" => nil, - "unsubmittedAt" => nil - } - end) + expected = %{ + "count" => length(submissions), + "data" => + Enum.map(submissions, fn submission -> + %{ + "xp" => 5000, + "xpAdjustment" => -2500, + "xpBonus" => 100, + "id" => submission.id, + "student" => %{ + "name" => submission.student.user.name, + "username" => submission.student.user.username, + "id" => submission.student.id, + "groupName" => submission.student.group.name, + "groupLeaderId" => submission.student.group.leader_id + }, + "assessment" => %{ + "type" => mission.config.type, + "isManuallyGraded" => mission.config.is_manually_graded, + "maxXp" => 5000, + "id" => mission.id, + "title" => mission.title, + "questionCount" => 5, + "assessmentNumber" => mission.number + }, + "status" => Atom.to_string(submission.status), + "gradedCount" => 5, + "unsubmittedBy" => nil, + "unsubmittedAt" => nil + } + end) } res = json_response(conn, 200) - assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} + + assert expected == %{ + "count" => res["count"], + "data" => Enum.sort_by(res["data"], & &1["id"]) + } end end @@ -179,38 +185,46 @@ defmodule CadetWeb.AdminGradingControllerTest do seed_db(conn, new_staff) conn = get(conn, build_url(course.id), %{"group" => "true"}) - expected = %{"count" => length(submissions), "data" => - Enum.map(submissions, fn submission -> - %{ - "xp" => 5000, - "xpAdjustment" => -2500, - "xpBonus" => 100, - "id" => submission.id, - "student" => %{ - "name" => submission.student.user.name, - "username" => submission.student.user.username, - "id" => submission.student.id, - "groupName" => submission.student.group.name, - "groupLeaderId" => submission.student.group.leader_id - }, - "assessment" => %{ - "type" => mission.config.type, - "isManuallyGraded" => mission.config.is_manually_graded, - "maxXp" => 5000, - "id" => mission.id, - "title" => mission.title, - "questionCount" => 5, - "assessmentNumber" => mission.number - }, - "status" => Atom.to_string(submission.status), - "gradedCount" => 5, - "unsubmittedBy" => nil, - "unsubmittedAt" => nil - } - end) + + expected = %{ + "count" => length(submissions), + "data" => + Enum.map(submissions, fn submission -> + %{ + "xp" => 5000, + "xpAdjustment" => -2500, + "xpBonus" => 100, + "id" => submission.id, + "student" => %{ + "name" => submission.student.user.name, + "username" => submission.student.user.username, + "id" => submission.student.id, + "groupName" => submission.student.group.name, + "groupLeaderId" => submission.student.group.leader_id + }, + "assessment" => %{ + "type" => mission.config.type, + "isManuallyGraded" => mission.config.is_manually_graded, + "maxXp" => 5000, + "id" => mission.id, + "title" => mission.title, + "questionCount" => 5, + "assessmentNumber" => mission.number + }, + "status" => Atom.to_string(submission.status), + "gradedCount" => 5, + "unsubmittedBy" => nil, + "unsubmittedAt" => nil + } + end) } + res = json_response(conn, 200) - assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} + + assert expected == %{ + "count" => res["count"], + "data" => Enum.sort_by(res["data"], & &1["id"]) + } end end @@ -782,38 +796,45 @@ defmodule CadetWeb.AdminGradingControllerTest do |> sign_in(admin.user) |> get(build_url(course.id)) - expected = %{ "count" => length(submissions), "data" => - Enum.map(submissions, fn submission -> - %{ - "xp" => 5000, - "xpAdjustment" => -2500, - "xpBonus" => 100, - "id" => submission.id, - "student" => %{ - "name" => submission.student.user.name, - "username" => submission.student.user.username, - "id" => submission.student.id, - "groupName" => submission.student.group.name, - "groupLeaderId" => submission.student.group.leader_id - }, - "assessment" => %{ - "type" => mission.config.type, - "isManuallyGraded" => mission.config.is_manually_graded, - "maxXp" => 5000, - "id" => mission.id, - "title" => mission.title, - "questionCount" => 5, - "assessmentNumber" => mission.number - }, - "status" => Atom.to_string(submission.status), - "gradedCount" => 5, - "unsubmittedBy" => nil, - "unsubmittedAt" => nil - } - end) + expected = %{ + "count" => length(submissions), + "data" => + Enum.map(submissions, fn submission -> + %{ + "xp" => 5000, + "xpAdjustment" => -2500, + "xpBonus" => 100, + "id" => submission.id, + "student" => %{ + "name" => submission.student.user.name, + "username" => submission.student.user.username, + "id" => submission.student.id, + "groupName" => submission.student.group.name, + "groupLeaderId" => submission.student.group.leader_id + }, + "assessment" => %{ + "type" => mission.config.type, + "isManuallyGraded" => mission.config.is_manually_graded, + "maxXp" => 5000, + "id" => mission.id, + "title" => mission.title, + "questionCount" => 5, + "assessmentNumber" => mission.number + }, + "status" => Atom.to_string(submission.status), + "gradedCount" => 5, + "unsubmittedBy" => nil, + "unsubmittedAt" => nil + } + end) } + res = json_response(conn, 200) - assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} + + assert expected == %{ + "count" => res["count"], + "data" => Enum.sort_by(res["data"], & &1["id"]) + } end end @@ -828,39 +849,45 @@ defmodule CadetWeb.AdminGradingControllerTest do conn = get(conn, build_url(course.id), %{"group" => "true"}) - expected = %{ "count" => length(submissions), "data" => - Enum.map(submissions, fn submission -> - %{ - "xp" => 5000, - "xpAdjustment" => -2500, - "xpBonus" => 100, - "id" => submission.id, - "student" => %{ - "name" => submission.student.user.name, - "username" => submission.student.user.username, - "id" => submission.student.id, - "groupName" => submission.student.group.name, - "groupLeaderId" => submission.student.group.leader_id - }, - "assessment" => %{ - "type" => mission.config.type, - "isManuallyGraded" => mission.config.is_manually_graded, - "maxXp" => 5000, - "id" => mission.id, - "title" => mission.title, - "questionCount" => 5, - "assessmentNumber" => mission.number - }, - "status" => Atom.to_string(submission.status), - "gradedCount" => 5, - "unsubmittedBy" => nil, - "unsubmittedAt" => nil - } - end) + expected = %{ + "count" => length(submissions), + "data" => + Enum.map(submissions, fn submission -> + %{ + "xp" => 5000, + "xpAdjustment" => -2500, + "xpBonus" => 100, + "id" => submission.id, + "student" => %{ + "name" => submission.student.user.name, + "username" => submission.student.user.username, + "id" => submission.student.id, + "groupName" => submission.student.group.name, + "groupLeaderId" => submission.student.group.leader_id + }, + "assessment" => %{ + "type" => mission.config.type, + "isManuallyGraded" => mission.config.is_manually_graded, + "maxXp" => 5000, + "id" => mission.id, + "title" => mission.title, + "questionCount" => 5, + "assessmentNumber" => mission.number + }, + "status" => Atom.to_string(submission.status), + "gradedCount" => 5, + "unsubmittedBy" => nil, + "unsubmittedAt" => nil + } + end) } res = json_response(conn, 200) - assert expected == %{"count" => res["count"], "data" => Enum.sort_by(res["data"], & &1["id"])} + + assert expected == %{ + "count" => res["count"], + "data" => Enum.sort_by(res["data"], & &1["id"]) + } end end From 00e48f2baa003fb54fc53ba9d2519f0f52f9c163 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sat, 17 Feb 2024 22:11:54 +0800 Subject: [PATCH 24/31] fix: Update filter by groupID to groupName --- lib/cadet/assessments/assessments.ex | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 275fade7d..63cdc8f79 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1441,16 +1441,18 @@ defmodule Cadet.Assessments do )) or submission.student_id == ^grader.id ) - {"groupID", value}, dynamic -> + {"groupName", value}, dynamic -> dynamic( [submission], ^dynamic and - submission.student_id in subquery( - from(cr in CourseRegistration, - where: cr.group_id == ^value, - select: cr.id - ) + submission.student_id in subquery( + from(cr in CourseRegistration, + join: g in Group, + on: cr.group_id == g.id, + where: g.name == ^value, + select: cr.id ) + ) ) {_, _}, dynamic -> From 80e2f5e4d8f57c165490cc4a0777b9a2d48b8d33 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 18 Feb 2024 19:06:43 +0800 Subject: [PATCH 25/31] feat: Assessment title filter using LIKE --- lib/cadet/assessments/assessments.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 63cdc8f79..fda483e45 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1402,7 +1402,7 @@ defmodule Cadet.Assessments do assessments_filters = Enum.reduce(params, dynamic(true), fn {"title", value}, dynamic -> - dynamic([assessment], ^dynamic and assessment.title == ^value) + dynamic([assessment], ^dynamic and ilike(assessment.title, ^"%#{value}%")) {_, _}, dynamic -> dynamic From b1a249cbbecd392202fcd7f7a2bd53469166c128 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 18 Feb 2024 19:15:13 +0800 Subject: [PATCH 26/31] fix: Change Notification/Email tests to use the new response --- lib/cadet/workers/NotificationWorker.ex | 7 +++++-- test/cadet/email_test.exs | 7 +++++-- .../notification_worker/notification_worker_test.exs | 11 ++++++++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/cadet/workers/NotificationWorker.ex b/lib/cadet/workers/NotificationWorker.ex index 609aab13c..67604ecc5 100644 --- a/lib/cadet/workers/NotificationWorker.ex +++ b/lib/cadet/workers/NotificationWorker.ex @@ -73,8 +73,11 @@ defmodule Cadet.Workers.NotificationWorker do for avenger_cr <- avengers_crs do avenger = Cadet.Accounts.get_user(avenger_cr.user_id) - {:ok, %{submissions: ungraded_submissions}} = - Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true) + {:ok, %{data: %{submissions: ungraded_submissions}}} = + Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ + "group" => "true", + "ungradedOnly" => "true" + }) if Enum.count(ungraded_submissions) < ungraded_threshold do IO.puts("[AVENGER_BACKLOG] below threshold!") diff --git a/test/cadet/email_test.exs b/test/cadet/email_test.exs index 2ee826979..e7c1ec3e6 100644 --- a/test/cadet/email_test.exs +++ b/test/cadet/email_test.exs @@ -24,8 +24,11 @@ defmodule Cadet.EmailTest do avenger_user = insert(:user, %{email: "test@gmail.com"}) avenger = insert(:course_registration, %{user: avenger_user, role: :staff}) - {:ok, %{submissions: ungraded_submissions}} = - Cadet.Assessments.all_submissions_by_grader_for_index(avenger, true, true) + {:ok, %{data: %{submissions: ungraded_submissions}}} = + Cadet.Assessments.submissions_by_grader_for_index(avenger, %{ + "group" => "true", + "ungradedOnly" => "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 626c46cac..f192b8867 100644 --- a/test/cadet/jobs/notification_worker/notification_worker_test.exs +++ b/test/cadet/jobs/notification_worker/notification_worker_test.exs @@ -18,9 +18,14 @@ defmodule Cadet.NotificationWorker.NotificationWorkerTest do submission = List.first(List.first(data.mcq_answers)).submission # setup for avenger backlog - {:ok, %{submissions: ungraded_submissions}} = - Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true) - + {:ok, %{data: %{submissions: ungraded_submissions}}} = + Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ + "group" => "true", + "ungradedOnly" => "true" + }) + + # {: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]) From 82b2a1040874ae7fc20be09c3af70fe8fb4d9d5d Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sun, 18 Feb 2024 19:15:34 +0800 Subject: [PATCH 27/31] refactor: Remove old code for submissions query --- lib/cadet/assessments/assessments.ex | 121 ++++-------------- .../admin_grading_controller.ex | 14 -- 2 files changed, 23 insertions(+), 112 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index fda483e45..b9474e393 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1228,96 +1228,6 @@ defmodule Cadet.Assessments do normalized_voting_score - :math.pow(2, min(1023.5, tokens / token_divider)) end - @doc """ - Function returning submissions under a grader. This function returns only the - fields that are exposed in the /grading endpoint. The reason we select only - those fields is to reduce the memory usage especially when the number of - submissions is large i.e. > 25000 submissions. - - The input parameters are the user and group_only. group_only is used to check - whether only the groups under the grader should be returned. The parameter is - a boolean which is false by default. - - The return value is {:ok, submissions} if no errors, else it is {:error, - {:forbidden, "Forbidden."}} - """ - @spec all_submissions_by_grader_for_index(CourseRegistration.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 - ) do - show_all = not group_only - - group_filter = - if show_all, - do: "", - else: - "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}" - - # 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. - - 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 - @doc """ Function returning submissions under a grader. This function returns only the fields that are exposed in the /grading endpoint. @@ -1334,6 +1244,16 @@ defmodule Cadet.Assessments do {:ok, %{"count": count, "data": submissions}} if no errors, else it is {:error, {:forbidden, "Forbidden."}} """ + + # 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. @spec submissions_by_grader_for_index(CourseRegistration.t()) :: {:ok, %{ @@ -1342,7 +1262,12 @@ defmodule Cadet.Assessments do }} def submissions_by_grader_for_index( grader = %CourseRegistration{course_id: course_id}, - params \\ %{"group" => "false", "pageSize" => "10", "offset" => "0"} + params \\ %{ + "group" => "false", + "ungradedOnly" => "false", + "pageSize" => "10", + "offset" => "0" + } ) do submission_answers_query = from(ans in Answer, @@ -1445,14 +1370,14 @@ defmodule Cadet.Assessments do dynamic( [submission], ^dynamic and - submission.student_id in subquery( - from(cr in CourseRegistration, - join: g in Group, - on: cr.group_id == g.id, - where: g.name == ^value, - select: cr.id + submission.student_id in subquery( + from(cr in CourseRegistration, + join: g in Group, + on: cr.group_id == g.id, + where: g.name == ^value, + select: cr.id + ) ) - ) ) {_, _}, dynamic -> diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index fb1c4881f..c09e47d04 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -17,20 +17,6 @@ defmodule CadetWeb.AdminGradingController do end end - def index(conn, %{"group" => group}) - when group in ["true", "false"] do - course_reg = conn.assigns[:course_reg] - group = String.to_atom(group) - - case Assessments.all_submissions_by_grader_for_index(course_reg, group) do - {:ok, view_model} -> - conn - |> put_status(:ok) - |> put_resp_content_type("application/json") - |> render("gradingsummaries.json", view_model) - end - end - def index(conn, _) do index(conn, %{"group" => "false"}) end From 2e3be04febd2e79db3e5fe752667ba653a200a01 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:26:20 +0800 Subject: [PATCH 28/31] feat: Implement notFullyGraded feature --- lib/cadet/assessments/assessments.ex | 33 ++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index b9474e393..5adf759cd 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1264,7 +1264,7 @@ defmodule Cadet.Assessments do grader = %CourseRegistration{course_id: course_id}, params \\ %{ "group" => "false", - "ungradedOnly" => "false", + "notFullyGraded" => "true", "pageSize" => "10", "offset" => "0" } @@ -1280,8 +1280,25 @@ defmodule Cadet.Assessments do } ) + question_answers_query = + from(q in Question, + group_by: q.assessment_id, + join: a in Assessment, + on: q.assessment_id == a.id, + select: %{ + assessment_id: q.assessment_id, + question_count: count(q.id) + } + ) + query = from(s in Submission, + left_join: ans in subquery(submission_answers_query), + on: ans.submission_id == s.id, + as: :ans, + left_join: asst in subquery(question_answers_query), + on: asst.assessment_id == s.assessment_id, + as: :asst, where: ^build_user_filter(params), where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), where: s.assessment_id in subquery(build_assessment_config_filter(params)), @@ -1290,8 +1307,6 @@ defmodule Cadet.Assessments do order_by: [desc: s.inserted_at], limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0), offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0), - left_join: ans in subquery(submission_answers_query), - on: ans.submission_id == s.id, select: %{ id: s.id, status: s.status, @@ -1302,7 +1317,8 @@ defmodule Cadet.Assessments do assessment_id: s.assessment_id, xp: ans.xp, xp_adjustment: ans.xp_adjustment, - graded_count: ans.graded_count + graded_count: ans.graded_count, + question_count: asst.question_count } ) @@ -1310,6 +1326,12 @@ defmodule Cadet.Assessments do count_query = from(s in Submission, + left_join: ans in subquery(submission_answers_query), + on: ans.submission_id == s.id, + as: :ans, + left_join: asst in subquery(question_answers_query), + on: asst.assessment_id == s.assessment_id, + as: :asst, where: s.assessment_id in subquery(build_assessment_filter(params, course_id)), where: s.assessment_id in subquery(build_assessment_config_filter(params)), where: ^build_user_filter(params), @@ -1345,6 +1367,9 @@ defmodule Cadet.Assessments do {"status", value}, dynamic -> dynamic([submission], ^dynamic and submission.status == ^value) + {"notFullyGraded", "true"}, dynamic -> + dynamic([ans: ans, asst: asst], ^dynamic and asst.question_count > ans.graded_count) + {_, _}, dynamic -> dynamic end) From 2ea2f55014fdcb9f17fd149b1870fee1ff4d057d Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:27:12 +0800 Subject: [PATCH 29/31] refactor: Remove TODO comment for what's now known as notFullyGraded --- lib/cadet/assessments/assessments.ex | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 5adf759cd..b244a93e0 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1233,8 +1233,7 @@ defmodule Cadet.Assessments do fields that are exposed in the /grading endpoint. The input parameters are the user and query parameters. Query parameters are - used to filter the submissions. Queries are arranged such that filters which - likely filter more submissions are on top. + used to filter the submissions. The group parameter is used to check whether only the groups under the grader should be returned. If pageSize and offset are not provided, the default @@ -1245,13 +1244,6 @@ defmodule Cadet.Assessments do else it is {:error, {:forbidden, "Forbidden."}} """ - # 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. @spec submissions_by_grader_for_index(CourseRegistration.t()) :: From 4d2f3b2fb43963ad2477612d0ddf34b0bfe72226 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Thu, 22 Feb 2024 18:36:56 +0800 Subject: [PATCH 30/31] refactor: Change param name for notification worker to filter notFullyGraded --- lib/cadet/workers/NotificationWorker.ex | 2 +- .../cadet/jobs/notification_worker/notification_worker_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cadet/workers/NotificationWorker.ex b/lib/cadet/workers/NotificationWorker.ex index 67604ecc5..67913a033 100644 --- a/lib/cadet/workers/NotificationWorker.ex +++ b/lib/cadet/workers/NotificationWorker.ex @@ -76,7 +76,7 @@ defmodule Cadet.Workers.NotificationWorker do {:ok, %{data: %{submissions: ungraded_submissions}}} = Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ "group" => "true", - "ungradedOnly" => "true" + "notFullyGraded" => "true" }) if Enum.count(ungraded_submissions) < ungraded_threshold do diff --git a/test/cadet/jobs/notification_worker/notification_worker_test.exs b/test/cadet/jobs/notification_worker/notification_worker_test.exs index f192b8867..7c6582d72 100644 --- a/test/cadet/jobs/notification_worker/notification_worker_test.exs +++ b/test/cadet/jobs/notification_worker/notification_worker_test.exs @@ -21,7 +21,7 @@ defmodule Cadet.NotificationWorker.NotificationWorkerTest do {:ok, %{data: %{submissions: ungraded_submissions}}} = Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ "group" => "true", - "ungradedOnly" => "true" + "notFullyGraded" => "true" }) # {:ok, %{submissions: ungraded_submissions}} = From fa2cb802a8999146a3716b9d87559ab2b9ac1f62 Mon Sep 17 00:00:00 2001 From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com> Date: Sat, 24 Feb 2024 01:43:40 +0800 Subject: [PATCH 31/31] Remove commented code --- .../cadet/jobs/notification_worker/notification_worker_test.exs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cadet/jobs/notification_worker/notification_worker_test.exs b/test/cadet/jobs/notification_worker/notification_worker_test.exs index 7c6582d72..9368a033c 100644 --- a/test/cadet/jobs/notification_worker/notification_worker_test.exs +++ b/test/cadet/jobs/notification_worker/notification_worker_test.exs @@ -24,8 +24,6 @@ defmodule Cadet.NotificationWorker.NotificationWorkerTest do "notFullyGraded" => "true" }) - # {: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])