Optimise SQL queries for /user and /assessments

lib/cadet/assessments/assessments.ex

@@ -73,28 +73,6 @@ defmodule Cadet.Assessments do
     Repo.delete_all(submissions)
   end
 
-  @spec user_total_xp(%User{}) :: integer()
-  def user_total_xp(%User{id: user_id}) when is_ecto_id(user_id) do
-    total_xp_bonus =
-      Submission
-      |> where(student_id: ^user_id)
-      |> Repo.aggregate(:sum, :xp_bonus)
-      |> case do
-        nil -> 0
-        xp when is_integer(xp) -> xp
-      end
-
-    total_xp =
-      Query.all_submissions_with_xp()
-      |> subquery()
-      |> where(student_id: ^user_id)
-      |> select([q], fragment("? + ?", sum(q.xp), sum(q.xp_adjustment)))
-      |> Repo.one()
-      |> decimal_to_integer()
-
-    total_xp_bonus + total_xp
-  end
-
   @spec user_max_grade(%User{}) :: integer()
   def user_max_grade(%User{id: user_id}) when is_ecto_id(user_id) do
     Submission
@@ -111,13 +89,29 @@ defmodule Cadet.Assessments do
     |> decimal_to_integer()
   end
 
-  def user_total_grade(%User{id: user_id}) do
-    Query.all_submissions_with_grade()
-    |> subquery()
-    |> where(student_id: ^user_id)
-    |> select([q], fragment("? + ?", sum(q.grade), sum(q.adjustment)))
-    |> Repo.one()
-    |> decimal_to_integer()
+  def user_total_grade_xp(%User{id: user_id}) do
+    submission_grade_xp =
+      Submission
+      |> where(student_id: ^user_id)
+      |> join(:inner, [s], a in Answer, on: s.id == a.submission_id)
+      |> group_by([s], s.id)
+      |> select([s, a], %{
+        total_grade: sum(a.grade) + sum(a.adjustment),
+        # grouping by submission, so s.xp_bonus will be the same, but we need an
+        # aggregate function
+        total_xp: sum(a.xp) + sum(a.xp_adjustment) + max(s.xp_bonus)
+      })
+
+    total =
+      submission_grade_xp
+      |> subquery
+      |> select([s], %{
+        total_grade: sum(s.total_grade),
+        total_xp: sum(s.total_xp)
+      })
+      |> Repo.one()
+
+    for {key, val} <- total, into: %{}, do: {key, decimal_to_integer(val)}
   end
 
   defp decimal_to_integer(decimal) do
@@ -277,34 +271,40 @@ defmodule Cadet.Assessments do
   by the supplied user
   """
   def all_assessments(user = %User{}) do
+    submission_aggregates =
+      Submission
+      |> join(:left, [s], ans in Answer, on: ans.submission_id == s.id)
+      |> where([s], s.student_id == ^user.id)
+      |> group_by([s], s.assessment_id)
+      |> select([s, ans], %{
+        assessment_id: s.assessment_id,
+        grade: fragment("? + ?", sum(ans.grade), sum(ans.adjustment)),
+        # s.xp_bonus should be the same across the group, but we need an aggregate function here
+        xp: fragment("? + ? + ?", sum(ans.xp), sum(ans.xp_adjustment), max(s.xp_bonus)),
+        graded_count: ans.id |> count() |> filter(not is_nil(ans.grader_id))
+      })
+
+    submission_status =
+      Submission
+      |> where([s], s.student_id == ^user.id)
+      |> select([s], [:assessment_id, :status])
+
     assessments =
-      Query.all_assessments_with_max_xp_and_grade()
+      Query.all_assessments_with_aggregates()
       |> subquery()
       |> join(
         :left,
         [a],
-        s in subquery(Query.all_submissions_with_xp_and_grade()),
-        on: a.id == s.assessment_id and s.student_id == ^user.id
-      )
-      |> join(
-        :left,
-        [a, _],
-        q_count in subquery(Query.assessments_question_count()),
-        on: a.id == q_count.assessment_id
-      )
-      |> join(
-        :left,
-        [_, s, _],
-        a_count in subquery(Query.submissions_graded_count()),
-        on: s.id == a_count.submission_id
+        sa in subquery(submission_aggregates),
+        on: a.id == sa.assessment_id
       )
-      |> select([a, s, q_count, a_count], %{
+      |> join(:left, [a, _], s in subquery(submission_status), on: a.id == s.assessment_id)
+      |> select([a, sa, s], %{
         a
-        | xp: fragment("? + ? + ?", s.xp, s.xp_adjustment, s.xp_bonus),
-          grade: fragment("? + ?", s.grade, s.adjustment),
-          user_status: s.status,
-          question_count: q_count.count,
-          graded_count: a_count.count
+        | xp: sa.xp,
+          grade: sa.grade,
+          graded_count: sa.graded_count,
+          user_status: s.status
       })
       |> filter_published_assessments(user)
       |> order_by(:open_at)

lib/cadet/assessments/query.ex

@@ -4,58 +4,22 @@ defmodule Cadet.Assessments.Query do
   """
   import Ecto.Query
 
-  alias Cadet.Assessments.{Answer, Assessment, Question, Submission}
-
-  @doc """
-  Returns a query with the following bindings:
-  [submissions_with_xp_and_grade, answers]
-  """
-  @spec all_submissions_with_xp_and_grade :: Ecto.Query.t()
-  def all_submissions_with_xp_and_grade do
-    Submission
-    |> join(:inner, [s], q in subquery(submissions_xp_and_grade()), on: s.id == q.submission_id)
-    |> select([s, q], %Submission{
-      s
-      | xp: q.xp,
-        xp_adjustment: q.xp_adjustment,
-        grade: q.grade,
-        adjustment: q.adjustment
-    })
-  end
-
-  @doc """
-  Returns a query with the following bindings:
-  [submissions_with_xp, answers]
-  """
-  @spec all_submissions_with_xp :: Ecto.Query.t()
-  def all_submissions_with_xp do
-    Submission
-    |> join(:inner, [s], q in subquery(submissions_xp()), on: s.id == q.submission_id)
-    |> select([s, q], %Submission{s | xp: q.xp, xp_adjustment: q.xp_adjustment})
-  end
-
-  @doc """
-  Returns a query with the following bindings:
-  [submissions_with_grade, answers]
-  """
-  @spec all_submissions_with_grade :: Ecto.Query.t()
-  def all_submissions_with_grade do
-    Submission
-    |> join(:inner, [s], q in subquery(submissions_grade()), on: s.id == q.submission_id)
-    |> select([s, q], %Submission{s | grade: q.grade, adjustment: q.adjustment})
-  end
+  alias Cadet.Assessments.{Assessment, Question}
 
   @doc """
   Returns a query with the following bindings:
   [assessments_with_xp_and_grade, questions]
   """
-  @spec all_assessments_with_max_xp_and_grade :: Ecto.Query.t()
-  def all_assessments_with_max_xp_and_grade do
+  @spec all_assessments_with_aggregates :: Ecto.Query.t()
+  def all_assessments_with_aggregates do
     Assessment
-    |> join(:inner, [a], q in subquery(assessments_max_xp_and_grade()),
-      on: a.id == q.assessment_id
-    )
-    |> select([a, q], %Assessment{a | max_grade: q.max_grade, max_xp: q.max_xp})
+    |> join(:inner, [a], q in subquery(assessments_aggregates()), on: a.id == q.assessment_id)
+    |> select([a, q], %Assessment{
+      a
+      | max_grade: q.max_grade,
+        max_xp: q.max_xp,
+        question_count: q.question_count
+    })
   end
 
   @doc """
@@ -69,75 +33,22 @@ defmodule Cadet.Assessments.Query do
     |> select([a, q], %Assessment{a | max_grade: q.max_grade})
   end
 
-  @spec submissions_xp_and_grade :: Ecto.Query.t()
-  def submissions_xp_and_grade do
-    Answer
-    |> group_by(:submission_id)
-    |> select([a], %{
-      submission_id: a.submission_id,
-      grade: sum(a.grade),
-      adjustment: sum(a.adjustment),
-      xp: sum(a.xp),
-      xp_adjustment: sum(a.xp_adjustment)
-    })
-  end
-
-  @spec submissions_grade :: Ecto.Query.t()
-  def submissions_grade do
-    Answer
-    |> group_by(:submission_id)
-    |> select([a], %{
-      submission_id: a.submission_id,
-      grade: sum(a.grade),
-      adjustment: sum(a.adjustment)
-    })
-  end
-
-  @spec submissions_xp :: Ecto.Query.t()
-  def submissions_xp do
-    Answer
-    |> group_by(:submission_id)
-    |> select([a], %{
-      submission_id: a.submission_id,
-      xp: sum(a.xp),
-      xp_adjustment: sum(a.xp_adjustment)
-    })
-  end
-
   @spec assessments_max_grade :: Ecto.Query.t()
   def assessments_max_grade do
     Question
     |> group_by(:assessment_id)
     |> select([q], %{assessment_id: q.assessment_id, max_grade: sum(q.max_grade)})
   end
 
-  @spec assessments_max_xp_and_grade :: Ecto.Query.t()
-  def assessments_max_xp_and_grade do
+  @spec assessments_aggregates :: Ecto.Query.t()
+  def assessments_aggregates do
     Question
     |> group_by(:assessment_id)
     |> select([q], %{
       assessment_id: q.assessment_id,
       max_grade: sum(q.max_grade),
-      max_xp: sum(q.max_xp)
-    })
-  end
-
-  def assessments_question_count do
-    Question
-    |> group_by(:assessment_id)
-    |> select([q], %{
-      assessment_id: q.assessment_id,
-      count: count(q.id)
-    })
-  end
-
-  def submissions_graded_count do
-    Answer
-    |> where([a], not is_nil(a.grader_id))
-    |> group_by(:submission_id)
-    |> select([a], %{
-      submission_id: a.submission_id,
-      count: count(a.id)
+      max_xp: sum(q.max_xp),
+      question_count: count(q.id)
     })
   end
 end

lib/cadet_web/controllers/user_controller.ex

@@ -10,10 +10,9 @@ defmodule CadetWeb.UserController do
 
   def index(conn, _) do
     user = user_with_group(conn.assigns.current_user)
-    grade = user_total_grade(user)
+    %{total_grade: grade, total_xp: xp} = user_total_grade_xp(user)
     max_grade = user_max_grade(user)
     story = user_current_story(user)
-    xp = user_total_xp(user)
     game_states = user_game_states(user)
 
     render(

test/cadet/assessments/query_test.exs

@@ -3,26 +3,6 @@ defmodule Cadet.Assessments.QueryTest do
 
   alias Cadet.Assessments.Query
 
-  test "all_submissions_with_grade/1" do
-    submission = insert(:submission)
-    assessment = insert(:assessment)
-    questions = insert_list(5, :question, assessment: assessment)
-
-    Enum.each(
-      questions,
-      &insert(:answer, submission: submission, grade: 200, adjustment: -100, question: &1)
-    )
-
-    result =
-      Query.all_submissions_with_grade()
-      |> where(id: ^submission.id)
-      |> Repo.one()
-
-    submission_id = submission.id
-
-    assert %{grade: 1000, adjustment: -500, id: ^submission_id} = result
-  end
-
   test "all_assessments_with_max_grade" do
     assessment = insert(:assessment)
     insert_list(5, :question, assessment: assessment, max_grade: 200)
@@ -37,24 +17,6 @@ defmodule Cadet.Assessments.QueryTest do
     assert %{max_grade: 1000, id: ^assessment_id} = result
   end
 
-  test "submissions_grade" do
-    submission = insert(:submission)
-    assessment = insert(:assessment)
-    questions = insert_list(5, :question, assessment: assessment)
-
-    Enum.each(
-      questions,
-      &insert(:answer, submission: submission, grade: 200, adjustment: -100, question: &1)
-    )
-
-    result =
-      Query.submissions_grade()
-      |> Repo.all()
-      |> Enum.find(&(&1.submission_id == submission.id))
-
-    assert %{grade: 1000, adjustment: -500, submission_id: submission.id} == result
-  end
-
   test "assessments_max_grade" do
     assessment = insert(:assessment)
     insert_list(5, :question, assessment: assessment, max_grade: 200)