Migrate filtering to the Backend (#1065)

* feat: implement paginated submissions query function

* feat: implement /grading route accepting pagination query parameters

* fix: fix paginated submissions with group filter

* Changed group_filter query to work with the new paginated query

* refactor: change page to offset for pagination

* feat: Update admin grading controller to take variable amount of params

* refactor: Refactor code from using raw sql to using ecto query

* feat: Implement filtering by mission name

* feat: Implement filtering by progress for submission

* refactor: show all and show only grader's groups now use ecto query

* feat: Implement filter by group id

* fix: Fix limit and offset default for missing query param

* refactor: Rename group filter to course registration filter

* feat: Implement filter by name

* feat: Implement filter by assessment type

* Implement filter by is_manually_graded

* feat: Include total count of submissions in response

* refactor: Use one join query for assessment config queries

* refactor: Use one join query for user queries

* refactor: Move queries for assessment and assessment config to builder

* fix: Update admin_grading_controller test to account for change in response

* refactor: Rename function

* docs: Include a description of the function

* refactor: Format code

* fix: Update filter by groupID to groupName

* feat: Assessment title filter using LIKE

* fix: Change Notification/Email tests to use the new response

* refactor: Remove old code for submissions query

* feat: Implement notFullyGraded feature

* refactor: Remove TODO comment for what's now known as notFullyGraded

* refactor: Change param name for notification worker to filter notFullyGraded

* Remove commented code

---------

Co-authored-by: Richard Dominick <[email protected]>

Author: GabrielCWT

lib/cadet/assessments/assessments.ex

@@ -1230,92 +1230,229 @@ defmodule Cadet.Assessments do
 
   @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.
+  fields that are exposed in the /grading endpoint.
 
-  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 input parameters are the user and query parameters. Query parameters are
+  used to filter the submissions.
 
-  The return value is {:ok, submissions} if no errors, else it is {:error,
-  {:forbidden, "Forbidden."}}
+  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 all_submissions_by_grader_for_index(CourseRegistration.t()) ::
-          {:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}}
-  def all_submissions_by_grader_for_index(
+
+  # 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,
+           %{
+             :count => integer,
+             :data => %{:assessments => [any()], :submissions => [any()], :users => [any()]}
+           }}
+  def submissions_by_grader_for_index(
         grader = %CourseRegistration{course_id: course_id},
-        group_only \\ false,
-        _ungraded_only \\ false
+        params \\ %{
+          "group" => "false",
+          "notFullyGraded" => "true",
+          "pageSize" => "10",
+          "offset" => "0"
+        }
       ) 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
+    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))
+        }
+      )
+
+    question_answers_query =
+      from(q in Question,
+        group_by: q.assessment_id,
+        join: a in Assessment,
+        on: q.assessment_id == a.id,
+        select: %{
+          assessment_id: q.assessment_id,
+          question_count: count(q.id)
+        }
+      )
+
+    query =
+      from(s in Submission,
+        left_join: ans in subquery(submission_answers_query),
+        on: ans.submission_id == s.id,
+        as: :ans,
+        left_join: asst in subquery(question_answers_query),
+        on: asst.assessment_id == s.assessment_id,
+        as: :asst,
+        where: ^build_user_filter(params),
+        where: s.assessment_id in subquery(build_assessment_filter(params, course_id)),
+        where: s.assessment_id in subquery(build_assessment_config_filter(params)),
+        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),
+        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,
+          question_count: asst.question_count
+        }
+      )
+
+    submissions = Repo.all(query)
+
+    count_query =
+      from(s in Submission,
+        left_join: ans in subquery(submission_answers_query),
+        on: ans.submission_id == s.id,
+        as: :ans,
+        left_join: asst in subquery(question_answers_query),
+        on: asst.assessment_id == s.assessment_id,
+        as: :asst,
+        where: s.assessment_id in subquery(build_assessment_filter(params, course_id)),
+        where: s.assessment_id in subquery(build_assessment_config_filter(params)),
+        where: ^build_user_filter(params),
+        where: ^build_submission_filter(params),
+        where: ^build_course_registration_filter(params, grader),
+        select: count(s.id)
+      )
+
+    count = Repo.one(count_query)
+
+    {:ok, %{count: count, data: generate_grading_summary_view_model(submissions, course_id)}}
+  end
 
-    {:ok, generate_grading_summary_view_model(submissions, course_id)}
+  defp build_assessment_filter(params, course_id) do
+    assessments_filters =
+      Enum.reduce(params, dynamic(true), fn
+        {"title", value}, dynamic ->
+          dynamic([assessment], ^dynamic and ilike(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
+    Enum.reduce(params, dynamic(true), fn
+      {"status", value}, dynamic ->
+        dynamic([submission], ^dynamic and submission.status == ^value)
+
+      {"notFullyGraded", "true"}, dynamic ->
+        dynamic([ans: ans, asst: asst], ^dynamic and asst.question_count > ans.graded_count)
+
+      {_, _}, dynamic ->
+        dynamic
+    end)
+  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
+        )
+
+      {"groupName", value}, 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.name == ^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
+              )
+            )
+        )
+
+      {"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 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)
+
+        {"isManuallyGraded", value}, dynamic ->
+          dynamic([assessment_config: config], ^dynamic and config.is_manually_graded == ^value)
+
+        {_, _}, 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

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",
+                "notFullyGraded" => "true"
+              })
 
             if Enum.count(ungraded_submissions) < ungraded_threshold do
               IO.puts("[AVENGER_BACKLOG] below threshold!")

lib/cadet_web/admin_controllers/admin_grading_controller.ex

@@ -4,12 +4,11 @@ defmodule CadetWeb.AdminGradingController do
 
   alias Cadet.Assessments
 
-  def index(conn, %{"group" => group}) 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.all_submissions_by_grader_for_index(course_reg, group) do
+    case Assessments.submissions_by_grader_for_index(course_reg, params) do
       {:ok, view_model} ->
         conn
         |> put_status(:ok)

lib/cadet_web/admin_views/admin_grading_view.ex

@@ -25,29 +25,36 @@ 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
-      user = users |> Enum.find(&(&1.id == submission.student_id))
-      assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id))
+    %{
+      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))
 
-      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", %{