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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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 22b05f99cc1ce52c28b727fbe04af43ca5c317e4 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:40:45 +0800 Subject: [PATCH 28/38] feat: Implement new seed for populating DB --- mix.exs | 2 +- priv/repo/seed_wip.exs | 101 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 priv/repo/seed_wip.exs diff --git a/mix.exs b/mix.exs index 992f71aed..ac1d1febe 100644 --- a/mix.exs +++ b/mix.exs @@ -121,7 +121,7 @@ defmodule Cadet.Mixfile do # See the documentation for `Mix` for more info on aliases. defp aliases do [ - "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"], + "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seed_wip.exs"], "ecto.reset": ["ecto.drop", "ecto.setup"], test: ["ecto.create --quiet", "ecto.migrate", "test"], "phx.server": ["cadet.server"], diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs new file mode 100644 index 000000000..a26ba28f7 --- /dev/null +++ b/priv/repo/seed_wip.exs @@ -0,0 +1,101 @@ +# Script for populating the database. You can run it as: +# TODO Change the following line to the correct path +# mix run priv/repo/seeds_wip.exs +# +use Cadet, [:context, :display] +import Cadet.Factory +import Ecto.Query +alias Cadet.Assessments.SubmissionStatus +alias Cadet.Accounts.{ + User, + CourseRegistration, +} + +if Cadet.Env.env() == :dev do + number_of_assessments = 10 + number_of_students = 100 + + # Course + course = insert(:course, %{course_name: "Mock Course", course_short_name: "CS0000S"}) + + # Admin and Group + admin_cr = + from(cr in CourseRegistration, + where: cr.user_id in subquery(from(u in User, where: u.name == ^"admin", select: u.id)), + select: cr + ) + |> Repo.one() + if admin_cr == nil do + admin = insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + _admin_cr = insert(:course_registration, %{user: admin, course: course, role: :admin}) + end + + group = insert(:group, %{name: "MockGroup", leader: admin_cr}) + + # Users + + students = for i <- 1..number_of_students do + student = insert(:user, %{latest_viewed_course: course}) + + student_cr = + insert(:course_registration, %{user: student, course: course, role: :student, group: group}) + + student_cr + end + + # Assessments and Submissions + valid_assessment_types = ["Mission", "Path", "Quest"] + + for i <- 1..number_of_assessments do + config = + insert(:assessment_config, %{ + type: Enum.random(valid_assessment_types), + order: i, + course: course + }) + + assessment = insert(:assessment, %{is_published: true, config: config, course: course}) + + questions = case assessment.config.type do + "Mission" -> + insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) + + "Path" -> + insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) + + "Quest" -> + insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) + end + + submissions = + students + |> Enum.map( + &insert(:submission, %{ + assessment: assessment, + student: &1, + status: Enum.random(SubmissionStatus.__enum_map__()) + }) + ) + + for submission <- submissions, + question <- questions do + case question.type do + :programming -> + insert(:answer, %{ + xp: Enum.random(0..1_000), + question: question, + submission: submission, + answer: build(:programming_answer) + }) + + :mcq -> + insert(:answer, %{ + xp: Enum.random(0..500), + question: question, + submission: submission, + answer: build(:mcq_answer) + }) + end + end + end + end From 013893ee5daed56cc35ffe6dbdf66895ce8c29ce Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 19 Feb 2024 13:02:30 +0800 Subject: [PATCH 29/38] refactor: Make only 3 types of assessments (Mission, Path, Quest) --- priv/repo/seed_wip.exs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs index a26ba28f7..58a9e475e 100644 --- a/priv/repo/seed_wip.exs +++ b/priv/repo/seed_wip.exs @@ -44,17 +44,15 @@ if Cadet.Env.env() == :dev do end # Assessments and Submissions - valid_assessment_types = ["Mission", "Path", "Quest"] + valid_assessment_types = [{1, "Mission"}, {2, "Path"}, {3, "Quest"}] + assessment_configs = Enum.map(valid_assessment_types, fn {order, type} -> + insert(:assessment_config, %{type: type, order: order, course: course}) + end + ) for i <- 1..number_of_assessments do - config = - insert(:assessment_config, %{ - type: Enum.random(valid_assessment_types), - order: i, - course: course - }) - - assessment = insert(:assessment, %{is_published: true, config: config, course: course}) + + assessment = insert(:assessment, %{is_published: true, config: Enum.random(assessment_configs), course: course}) questions = case assessment.config.type do "Mission" -> From 5dede855ccc16471476de8ed9e726e28e8695674 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 19 Feb 2024 13:02:59 +0800 Subject: [PATCH 30/38] refactor: Rename assessments to contain 's' --- priv/repo/seed_wip.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs index 58a9e475e..4aa03f8a4 100644 --- a/priv/repo/seed_wip.exs +++ b/priv/repo/seed_wip.exs @@ -44,7 +44,7 @@ if Cadet.Env.env() == :dev do end # Assessments and Submissions - valid_assessment_types = [{1, "Mission"}, {2, "Path"}, {3, "Quest"}] + valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] assessment_configs = Enum.map(valid_assessment_types, fn {order, type} -> insert(:assessment_config, %{type: type, order: order, course: course}) end @@ -55,13 +55,13 @@ if Cadet.Env.env() == :dev do assessment = insert(:assessment, %{is_published: true, config: Enum.random(assessment_configs), course: course}) questions = case assessment.config.type do - "Mission" -> + "Missions" -> insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) - "Path" -> + "Paths" -> insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) - "Quest" -> + "Quests" -> insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) end From 8c520056d0af42405a0097bf6128b49ad659c7fb Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 19 Feb 2024 13:43:35 +0800 Subject: [PATCH 31/38] fix: Fix bug where admin_cr was not being assigned the return value --- priv/repo/seed_wip.exs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs index 4aa03f8a4..fc0b3d6c3 100644 --- a/priv/repo/seed_wip.exs +++ b/priv/repo/seed_wip.exs @@ -25,10 +25,14 @@ if Cadet.Env.env() == :dev do select: cr ) |> Repo.one() - if admin_cr == nil do - admin = insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) - _admin_cr = insert(:course_registration, %{user: admin, course: course, role: :admin}) - end + + admin_cr = + if admin_cr == nil do + admin = insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + insert(:course_registration, %{user: admin, course: course, role: :admin}) + else + admin_cr + end group = insert(:group, %{name: "MockGroup", leader: admin_cr}) From d3f69bad6688f98ec3cb401d98651285ba8dab12 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Mon, 19 Feb 2024 14:50:51 +0800 Subject: [PATCH 32/38] refactor: Format code --- priv/repo/seed_wip.exs | 113 +++++++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 49 deletions(-) diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs index fc0b3d6c3..0d4947a3f 100644 --- a/priv/repo/seed_wip.exs +++ b/priv/repo/seed_wip.exs @@ -6,9 +6,10 @@ use Cadet, [:context, :display] import Cadet.Factory import Ecto.Query alias Cadet.Assessments.SubmissionStatus + alias Cadet.Accounts.{ User, - CourseRegistration, + CourseRegistration } if Cadet.Env.env() == :dev do @@ -28,7 +29,9 @@ if Cadet.Env.env() == :dev do admin_cr = if admin_cr == nil do - admin = insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + admin = + insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + insert(:course_registration, %{user: admin, course: course, role: :admin}) else admin_cr @@ -38,66 +41,78 @@ if Cadet.Env.env() == :dev do # Users - students = for i <- 1..number_of_students do - student = insert(:user, %{latest_viewed_course: course}) + students = + for i <- 1..number_of_students do + student = insert(:user, %{latest_viewed_course: course}) - student_cr = - insert(:course_registration, %{user: student, course: course, role: :student, group: group}) + student_cr = + insert(:course_registration, %{ + user: student, + course: course, + role: :student, + group: group + }) - student_cr - end + student_cr + end # Assessments and Submissions valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] - assessment_configs = Enum.map(valid_assessment_types, fn {order, type} -> - insert(:assessment_config, %{type: type, order: order, course: course}) - end - ) - for i <- 1..number_of_assessments do - - assessment = insert(:assessment, %{is_published: true, config: Enum.random(assessment_configs), course: course}) + assessment_configs = + Enum.map(valid_assessment_types, fn {order, type} -> + insert(:assessment_config, %{type: type, order: order, course: course}) + end) - questions = case assessment.config.type do - "Missions" -> + for i <- 1..number_of_assessments do + assessment = + insert(:assessment, %{ + is_published: true, + config: Enum.random(assessment_configs), + course: course + }) + + questions = + case assessment.config.type do + "Missions" -> insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) - "Paths" -> + "Paths" -> insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) - "Quests" -> + "Quests" -> insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) end - submissions = - students - |> Enum.map( - &insert(:submission, %{ - assessment: assessment, - student: &1, - status: Enum.random(SubmissionStatus.__enum_map__()) - }) - ) - - for submission <- submissions, - question <- questions do - case question.type do - :programming -> - insert(:answer, %{ - xp: Enum.random(0..1_000), - question: question, - submission: submission, - answer: build(:programming_answer) - }) - - :mcq -> - insert(:answer, %{ - xp: Enum.random(0..500), - question: question, - submission: submission, - answer: build(:mcq_answer) - }) - end - end + submissions = + students + |> Enum.map( + &insert(:submission, %{ + assessment: assessment, + student: &1, + status: Enum.random(SubmissionStatus.__enum_map__()) + }) + ) + + for submission <- submissions, + question <- questions do + case question.type do + :programming -> + insert(:answer, %{ + xp: Enum.random(0..1_000), + question: question, + submission: submission, + answer: build(:programming_answer) + }) + + :mcq -> + insert(:answer, %{ + xp: Enum.random(0..500), + question: question, + submission: submission, + answer: build(:mcq_answer) + }) + end end end +end From 6eb9c52debc774f7ffb00afeab32dbc2d3140805 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Tue, 20 Feb 2024 18:35:41 +0800 Subject: [PATCH 33/38] refactor: Update code to be in actual seed file --- mix.exs | 2 +- priv/repo/seed_wip.exs | 118 --------------------------------- priv/repo/seeds.exs | 146 ++++++++++++++++++++++++----------------- 3 files changed, 85 insertions(+), 181 deletions(-) delete mode 100644 priv/repo/seed_wip.exs diff --git a/mix.exs b/mix.exs index ac1d1febe..992f71aed 100644 --- a/mix.exs +++ b/mix.exs @@ -121,7 +121,7 @@ defmodule Cadet.Mixfile do # See the documentation for `Mix` for more info on aliases. defp aliases do [ - "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seed_wip.exs"], + "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"], "ecto.reset": ["ecto.drop", "ecto.setup"], test: ["ecto.create --quiet", "ecto.migrate", "test"], "phx.server": ["cadet.server"], diff --git a/priv/repo/seed_wip.exs b/priv/repo/seed_wip.exs deleted file mode 100644 index 0d4947a3f..000000000 --- a/priv/repo/seed_wip.exs +++ /dev/null @@ -1,118 +0,0 @@ -# Script for populating the database. You can run it as: -# TODO Change the following line to the correct path -# mix run priv/repo/seeds_wip.exs -# -use Cadet, [:context, :display] -import Cadet.Factory -import Ecto.Query -alias Cadet.Assessments.SubmissionStatus - -alias Cadet.Accounts.{ - User, - CourseRegistration -} - -if Cadet.Env.env() == :dev do - number_of_assessments = 10 - number_of_students = 100 - - # Course - course = insert(:course, %{course_name: "Mock Course", course_short_name: "CS0000S"}) - - # Admin and Group - admin_cr = - from(cr in CourseRegistration, - where: cr.user_id in subquery(from(u in User, where: u.name == ^"admin", select: u.id)), - select: cr - ) - |> Repo.one() - - admin_cr = - if admin_cr == nil do - admin = - insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) - - insert(:course_registration, %{user: admin, course: course, role: :admin}) - else - admin_cr - end - - group = insert(:group, %{name: "MockGroup", leader: admin_cr}) - - # Users - - students = - for i <- 1..number_of_students do - student = insert(:user, %{latest_viewed_course: course}) - - student_cr = - insert(:course_registration, %{ - user: student, - course: course, - role: :student, - group: group - }) - - student_cr - end - - # Assessments and Submissions - valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] - - assessment_configs = - Enum.map(valid_assessment_types, fn {order, type} -> - insert(:assessment_config, %{type: type, order: order, course: course}) - end) - - for i <- 1..number_of_assessments do - assessment = - insert(:assessment, %{ - is_published: true, - config: Enum.random(assessment_configs), - course: course - }) - - questions = - case assessment.config.type do - "Missions" -> - insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) - - "Paths" -> - insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) - - "Quests" -> - insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) - end - - submissions = - students - |> Enum.map( - &insert(:submission, %{ - assessment: assessment, - student: &1, - status: Enum.random(SubmissionStatus.__enum_map__()) - }) - ) - - for submission <- submissions, - question <- questions do - case question.type do - :programming -> - insert(:answer, %{ - xp: Enum.random(0..1_000), - question: question, - submission: submission, - answer: build(:programming_answer) - }) - - :mcq -> - insert(:answer, %{ - xp: Enum.random(0..500), - question: question, - submission: submission, - answer: build(:mcq_answer) - }) - end - end - end -end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index dee7be580..920a5be3a 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -9,72 +9,95 @@ # # We recommend using the bang functions (`insert!`, `update!` # and so on) as they will fail if something goes wrong. +use Cadet, [:context, :display] + +import Cadet.Factory import Cadet.Factory +import Ecto.Query alias Cadet.Assessments.SubmissionStatus +alias Cadet.Accounts.{ + User, + CourseRegistration +} # insert default source version # Cadet.Repo.insert!(%Cadet.Settings.Sublanguage{chapter: 1, variant: "default"}) if Cadet.Env.env() == :dev do + number_of_assessments = 10 + number_of_students = 100 + # Course - course1 = insert(:course) - course2 = insert(:course, %{course_name: "Algorithm", course_short_name: "CS2040S"}) - # Users - avenger1 = insert(:user, %{name: "avenger", latest_viewed_course: course1}) - admin1 = insert(:user, %{name: "admin", latest_viewed_course: course1}) + course = insert(:course, %{course_name: "Mock Course", course_short_name: "CS0000S"}) + + # Admin and Group + admin_cr = + from(cr in CourseRegistration, + where: cr.user_id in subquery(from(u in User, where: u.name == ^"admin", select: u.id)), + select: cr + ) + |> Repo.one() + + admin_cr = + if admin_cr == nil do + admin = + insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + + insert(:course_registration, %{user: admin, course: course, role: :admin}) + else + admin_cr + end - studenta1admin2 = insert(:user, %{name: "student a", latest_viewed_course: course1}) + group = insert(:group, %{name: "MockGroup", leader: admin_cr}) - studentb1 = insert(:user, %{latest_viewed_course: course1}) - studentc1 = insert(:user, %{latest_viewed_course: course1}) - # CourseRegistration and Group - avenger1_cr = insert(:course_registration, %{user: avenger1, course: course1, role: :staff}) - _admin1_cr = insert(:course_registration, %{user: admin1, course: course1, role: :admin}) - group = insert(:group, %{leader: avenger1_cr}) + # Users - student1a_cr = - insert(:course_registration, %{ - user: studenta1admin2, - course: course1, - role: :student, - group: group - }) + students = + for i <- 1..number_of_students do + student = insert(:user, %{latest_viewed_course: course}) - student1b_cr = - insert(:course_registration, %{user: studentb1, course: course1, role: :student, group: group}) + student_cr = + insert(:course_registration, %{ + user: student, + course: course, + role: :student, + group: group + }) - student1c_cr = - insert(:course_registration, %{user: studentc1, course: course1, role: :student, group: group}) + student_cr + end - students = [student1a_cr, student1b_cr, student1c_cr] + # Assessments and Submissions + valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] - _admin2cr = - insert(:course_registration, %{user: studenta1admin2, course: course2, role: :admin}) + assessment_configs = + Enum.map(valid_assessment_types, fn {order, type} -> + insert(:assessment_config, %{type: type, order: order, course: course}) + end) - # Assessments - for i <- 1..5 do - config = insert(:assessment_config, %{type: "Mission#{i}", order: i, course: course1}) - assessment = insert(:assessment, %{is_published: true, config: config, course: course1}) + for i <- 1..number_of_assessments do + assessment = + insert(:assessment, %{ + is_published: true, + config: Enum.random(assessment_configs), + course: course + }) - config2 = insert(:assessment_config, %{type: "Homework#{i}", order: i, course: course2}) - _assessment2 = insert(:assessment, %{is_published: true, config: config2, course: course2}) + questions = + case assessment.config.type do + "Missions" -> + insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) - programming_questions = - insert_list(3, :programming_question, %{ - assessment: assessment, - max_xp: 1_000 - }) + "Paths" -> + insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) - mcq_questions = - insert_list(3, :mcq_question, %{ - assessment: assessment, - max_xp: 500 - }) + "Quests" -> + insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) + end submissions = students - |> Enum.take(2) |> Enum.map( &insert(:submission, %{ assessment: assessment, @@ -83,26 +106,25 @@ if Cadet.Env.env() == :dev do }) ) - # Programming Answers - for submission <- submissions, - question <- programming_questions do - insert(:answer, %{ - xp: Enum.random(0..1_000), - question: question, - submission: submission, - answer: build(:programming_answer) - }) - end - - # MCQ Answers for submission <- submissions, - question <- mcq_questions do - insert(:answer, %{ - xp: Enum.random(0..500), - question: question, - submission: submission, - answer: build(:mcq_answer) - }) + question <- questions do + case question.type do + :programming -> + insert(:answer, %{ + xp: Enum.random(0..1_000), + question: question, + submission: submission, + answer: build(:programming_answer) + }) + + :mcq -> + insert(:answer, %{ + xp: Enum.random(0..500), + question: question, + submission: submission, + answer: build(:mcq_answer) + }) + end end # # Notifications From 7b016b67561872d098cd02c18c3b20ef826d68ee Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:12:29 +0800 Subject: [PATCH 34/38] Format code --- priv/repo/seeds.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 920a5be3a..a48e5b41e 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -16,6 +16,7 @@ import Cadet.Factory import Ecto.Query alias Cadet.Assessments.SubmissionStatus + alias Cadet.Accounts.{ User, CourseRegistration From bd5b93c3c5fb2722bc2402739619eb3ffa503ebe Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:23:00 +0800 Subject: [PATCH 35/38] fix: Fix bug where xp generated was more than max_xp --- priv/repo/seeds.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index a48e5b41e..a3d0ca823 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -94,7 +94,7 @@ if Cadet.Env.env() == :dev do insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) "Quests" -> - insert_list(3, :programming_question, %{assessment: assessment, max_xp: 500}) + insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) end submissions = From edfc3326527b1cbf0b457c3d9ff2cd8fe92db8f3 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:40:36 +0800 Subject: [PATCH 36/38] refactor: Add number of questions variable --- priv/repo/seeds.exs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index a3d0ca823..6251ad377 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -26,8 +26,9 @@ alias Cadet.Accounts.{ # Cadet.Repo.insert!(%Cadet.Settings.Sublanguage{chapter: 1, variant: "default"}) if Cadet.Env.env() == :dev do - number_of_assessments = 10 number_of_students = 100 + number_of_assessments = 10 + number_of_questions = 3 # Course course = insert(:course, %{course_name: "Mock Course", course_short_name: "CS0000S"}) @@ -88,13 +89,19 @@ if Cadet.Env.env() == :dev do questions = case assessment.config.type do "Missions" -> - insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) + insert_list(number_of_questions, :programming_question, %{ + assessment: assessment, + max_xp: 1_000 + }) "Paths" -> - insert_list(3, :mcq_question, %{assessment: assessment, max_xp: 500}) + insert_list(number_of_questions, :mcq_question, %{assessment: assessment, max_xp: 500}) "Quests" -> - insert_list(3, :programming_question, %{assessment: assessment, max_xp: 1_000}) + insert_list(number_of_questions, :programming_question, %{ + assessment: assessment, + max_xp: 1_000 + }) end submissions = From 715cc484b78d8e634e477f4e5a26fe9d5b924df7 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sat, 24 Feb 2024 12:29:38 +0800 Subject: [PATCH 37/38] refactor: Remove old comment --- lib/cadet/assessments/assessments.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 7e65a9e2f..0b49c0a90 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1244,8 +1244,6 @@ defmodule Cadet.Assessments do else it is {:error, {:forbidden, "Forbidden."}} """ - # 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, %{ @@ -1271,6 +1269,7 @@ defmodule Cadet.Assessments do graded_count: filter(count(ans.id), not is_nil(ans.grader_id)) } ) + question_answers_query = from(q in Question, group_by: q.assessment_id, From 697b8de6c35f312250bc45c6d6458d8e919cf206 Mon Sep 17 00:00:00 2001 From: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> Date: Sat, 24 Feb 2024 13:16:59 +0800 Subject: [PATCH 38/38] feat: Update seed to populate a staff course & group --- priv/repo/seeds.exs | 227 +++++++++++++++++++++++++------------------- 1 file changed, 128 insertions(+), 99 deletions(-) diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 6251ad377..c5661ee65 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -26,14 +26,15 @@ alias Cadet.Accounts.{ # Cadet.Repo.insert!(%Cadet.Settings.Sublanguage{chapter: 1, variant: "default"}) if Cadet.Env.env() == :dev do - number_of_students = 100 - number_of_assessments = 10 + number_of_students = 10 + number_of_assessments = 5 number_of_questions = 3 # Course - course = insert(:course, %{course_name: "Mock Course", course_short_name: "CS0000S"}) + admin_course = + insert(:course, %{course_name: "Mock Admin Course", course_short_name: "CS0000S"}) - # Admin and Group + # Admin, Staff and Group admin_cr = from(cr in CourseRegistration, where: cr.user_id in subquery(from(u in User, where: u.name == ^"admin", select: u.id)), @@ -44,122 +45,150 @@ if Cadet.Env.env() == :dev do admin_cr = if admin_cr == nil do admin = - insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: course}) + insert(:user, %{name: "Test Admin", username: "admin", latest_viewed_course: admin_course}) - insert(:course_registration, %{user: admin, course: course, role: :admin}) + insert(:course_registration, %{user: admin, course: admin_course, role: :admin}) else admin_cr end - group = insert(:group, %{name: "MockGroup", leader: admin_cr}) + avenger_course = + insert(:course, %{course_name: "Mock Avenger Course", course_short_name: "CS1111S"}) - # Users - - students = - for i <- 1..number_of_students do - student = insert(:user, %{latest_viewed_course: course}) + avenger_cr = + from(cr in CourseRegistration, + where: cr.user_id in subquery(from(u in User, where: u.name == ^"staff", select: u.id)), + select: cr + ) + |> Repo.one() - student_cr = - insert(:course_registration, %{ - user: student, - course: course, - role: :student, - group: group + avenger_cr = + if avenger_cr == nil do + avenger = + insert(:user, %{ + name: "Test Staff", + username: "staff", + latest_viewed_course: avenger_course }) - student_cr + insert(:course_registration, %{user: avenger, course: avenger_course, role: :staff}) + else + avenger_cr end - # Assessments and Submissions - valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] - - assessment_configs = - Enum.map(valid_assessment_types, fn {order, type} -> - insert(:assessment_config, %{type: type, order: order, course: course}) - end) - - for i <- 1..number_of_assessments do - assessment = - insert(:assessment, %{ - is_published: true, - config: Enum.random(assessment_configs), - course: course - }) - - questions = - case assessment.config.type do - "Missions" -> - insert_list(number_of_questions, :programming_question, %{ - assessment: assessment, - max_xp: 1_000 - }) + admin_group = insert(:group, %{name: "MockAdminGroup", leader: admin_cr}) + avenger_group = insert(:group, %{name: "MockAvengerGroup", leader: avenger_cr}) - "Paths" -> - insert_list(number_of_questions, :mcq_question, %{assessment: assessment, max_xp: 500}) + groups_and_courses = [{admin_group, admin_course}, {avenger_group, avenger_course}] + # Users - "Quests" -> - insert_list(number_of_questions, :programming_question, %{ - assessment: assessment, - max_xp: 1_000 + Enum.each(groups_and_courses, fn {group, course} -> + students = + for i <- 1..number_of_students do + student = insert(:user, %{latest_viewed_course: course}) + + student_cr = + insert(:course_registration, %{ + user: student, + course: course, + role: :student, + group: group }) + + student_cr end - submissions = - students - |> Enum.map( - &insert(:submission, %{ - assessment: assessment, - student: &1, - status: Enum.random(SubmissionStatus.__enum_map__()) + # Assessments and Submissions + valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] + + assessment_configs = + Enum.map(valid_assessment_types, fn {order, type} -> + insert(:assessment_config, %{type: type, order: order, course: course}) + end) + + for i <- 1..number_of_assessments do + assessment = + insert(:assessment, %{ + is_published: true, + config: Enum.random(assessment_configs), + course: course }) - ) - - for submission <- submissions, - question <- questions do - case question.type do - :programming -> - insert(:answer, %{ - xp: Enum.random(0..1_000), - question: question, - submission: submission, - answer: build(:programming_answer) - }) - :mcq -> - insert(:answer, %{ - xp: Enum.random(0..500), - question: question, - submission: submission, - answer: build(:mcq_answer) + questions = + case assessment.config.type do + "Missions" -> + insert_list(number_of_questions, :programming_question, %{ + assessment: assessment, + max_xp: 1_000 + }) + + "Paths" -> + insert_list(number_of_questions, :mcq_question, %{assessment: assessment, max_xp: 500}) + + "Quests" -> + insert_list(number_of_questions, :programming_question, %{ + assessment: assessment, + max_xp: 1_000 + }) + end + + submissions = + students + |> Enum.map( + &insert(:submission, %{ + assessment: assessment, + student: &1, + status: Enum.random(SubmissionStatus.__enum_map__()) }) + ) + + for submission <- submissions, + question <- questions do + case question.type do + :programming -> + insert(:answer, %{ + xp: Enum.random(0..1_000), + question: question, + submission: submission, + answer: build(:programming_answer) + }) + + :mcq -> + insert(:answer, %{ + xp: Enum.random(0..500), + question: question, + submission: submission, + answer: build(:mcq_answer) + }) + end end - end - # # Notifications - # for submission <- submissions do - # case submission.status do - # :submitted -> - # insert(:notification, %{ - # type: :submitted, - # read: false, - # user_id: avenger.id, - # submission_id: submission.id, - # assessment_id: assessment.id - # }) - - # _ -> - # nil - # end - # end - - # for student <- students do - # insert(:notification, %{ - # type: :new, - # user_id: student.id, - # assessment_id: assessment.id - # }) - # end - end + # # Notifications + # for submission <- submissions do + # case submission.status do + # :submitted -> + # insert(:notification, %{ + # type: :submitted, + # read: false, + # user_id: avenger.id, + # submission_id: submission.id, + # assessment_id: assessment.id + # }) + + # _ -> + # nil + # end + # end + + # for student <- students do + # insert(:notification, %{ + # type: :new, + # user_id: student.id, + # assessment_id: assessment.id + # }) + # end + end + end) # goal_0 = # insert(:goal, %{