Add grading controller (#101)

* Add factories and seeds

* Update api spec according to changes from Vignesh

* Implement index

* Hotfix tests

* Implement show sans test

* Add tests

* Update context and model in preparation of implementing update

* Fix duplicate factory

* Implement update sans test

* Add typespec and is_ecto_id macro guard clause

* Add and refactor tests

* Fix according to code review

* Change to list comprehension for readability

Author: indocomsoft

lib/cadet/accounts/query.ex

@@ -4,7 +4,8 @@ defmodule Cadet.Accounts.Query do
   """
   import Ecto.Query
 
-  alias Cadet.Accounts.Authorization
+  alias Cadet.Accounts.{Authorization, User}
+  alias Cadet.Course.Group
 
   def user_nusnet_ids(user_id) do
     Authorization
@@ -18,6 +19,13 @@ defmodule Cadet.Accounts.Query do
     |> of_uid(uid)
   end
 
+  @spec students_of(User.t()) :: User.t()
+  def students_of(%User{id: id, role: :staff}) do
+    User
+    |> join(:inner, [u], g in Group, u.group_id == g.id)
+    |> where([_, g], g.leader_id == ^id)
+  end
+
   defp nusnet_ids(query) do
     query |> where([a], a.provider == "nusnet_id")
   end

lib/cadet/assessments/answer.ex

@@ -12,7 +12,6 @@ defmodule Cadet.Assessments.Answer do
     field(:xp, :integer, default: 0)
     field(:answer, :map)
     field(:type, QuestionType, virtual: true)
-    field(:raw_answer, :string, virtual: true)
     field(:comment, :string)
     field(:adjustment, :integer, default: 0)
     belongs_to(:submission, Submission)
@@ -35,6 +34,24 @@ defmodule Cadet.Assessments.Answer do
     |> validate_answer_content()
   end
 
+  @spec grading_changeset(%__MODULE__{} | Ecto.Changeset.t(), map()) :: Ecto.Changeset.t()
+  def grading_changeset(answer, params) do
+    answer
+    |> cast(params, ~w(adjustment comment)a)
+    |> validate_xp_adjustment_total()
+  end
+
+  @spec validate_xp_adjustment_total(Ecto.Changeset.t()) :: Ecto.Changeset.t()
+  defp validate_xp_adjustment_total(changeset) do
+    answer = apply_changes(changeset)
+
+    if answer.xp + answer.adjustment >= 0 do
+      changeset
+    else
+      add_error(changeset, :adjustment, "should not make total point < 0")
+    end
+  end
+
   defp add_question_type_from_model(changeset, params) do
     with question when is_map(question) <- Map.get(params, :question),
          nil <- get_change(changeset, :type),

lib/cadet/assessments/assessment.ex

@@ -12,6 +12,7 @@ defmodule Cadet.Assessments.Assessment do
   alias Cadet.Assessments.Upload
 
   schema "assessments" do
+    field(:max_xp, :integer, virtual: true)
     field(:title, :string)
     field(:is_published, :boolean, default: false)
     field(:type, AssessmentType)

lib/cadet/assessments/assessments.ex

@@ -10,9 +10,10 @@ defmodule Cadet.Assessments do
   alias Timex.Duration
 
   alias Cadet.Accounts.User
-  alias Cadet.Assessments.{Answer, Assessment, Question, Submission}
+  alias Cadet.Assessments.{Answer, Assessment, Query, Question, Submission}
 
   @submit_answer_roles ~w(student)a
+  @grading_roles ~w(staff)a
 
   def all_assessments() do
     Repo.all(Assessment)
@@ -89,7 +90,7 @@ defmodule Cadet.Assessments do
   end
 
   def create_question_for_assessment(params, assessment_id)
-      when is_binary(assessment_id) or is_number(assessment_id) do
+      when is_ecto_id(assessment_id) do
     assessment = get_assessment(assessment_id)
     create_question_for_assessment(params, assessment)
   end
@@ -151,6 +152,95 @@ defmodule Cadet.Assessments do
     end
   end
 
+  @spec all_submissions_by_grader(User.t()) ::
+          {:ok, [Submission.t()]} | {:error, {:unauthorized, String.t()}}
+  def all_submissions_by_grader(grader = %User{role: role}) do
+    if role in @grading_roles do
+      students = Cadet.Accounts.Query.students_of(grader)
+
+      submissions =
+        Submission
+        |> join(:inner, [s], x in subquery(Query.submissions_xp()), s.id == x.submission_id)
+        |> join(:inner, [s], st in subquery(students), s.student_id == st.id)
+        |> join(
+          :inner,
+          [s],
+          a in subquery(Query.all_assessments_with_max_xp()),
+          s.assessment_id == a.id
+        )
+        |> select([s, x, st, a], %Submission{s | xp: x.xp, student: st, assessment: a})
+        |> Repo.all()
+
+      {:ok, submissions}
+    else
+      {:error, {:unauthorized, "User is not permitted to grade."}}
+    end
+  end
+
+  @spec get_answers_in_submission(integer() | String.t(), User.t()) ::
+          {:ok, [Answer.t()]} | {:error, {:unauthorized, String.t()}}
+  def get_answers_in_submission(id, grader = %User{role: role}) when is_ecto_id(id) do
+    if role in @grading_roles do
+      students = Cadet.Accounts.Query.students_of(grader)
+
+      answers =
+        Answer
+        |> where(submission_id: ^id)
+        |> join(:inner, [a], s in Submission, a.submission_id == s.id)
+        |> join(:inner, [a, s], t in subquery(students), t.id == s.student_id)
+        |> join(:inner, [a], q in assoc(a, :question))
+        |> preload([a, ..., q], question: q)
+        |> Repo.all()
+
+      {:ok, answers}
+    else
+      {:error, {:unauthorized, "User is not permitted to grade."}}
+    end
+  end
+
+  @spec update_grading_info(
+          %{submission_id: integer() | String.t(), question_id: integer() | String.t()},
+          %{},
+          User.t()
+        ) ::
+          {:ok, nil}
+          | {:error, {:unauthorized | :bad_request | :internal_server_error, String.t()}}
+  def update_grading_info(
+        %{submission_id: submission_id, question_id: question_id},
+        attrs,
+        grader = %User{role: role}
+      )
+      when is_ecto_id(submission_id) and is_ecto_id(question_id) do
+    if role in @grading_roles do
+      students = Cadet.Accounts.Query.students_of(grader)
+
+      answer =
+        Answer
+        |> where([a], a.submission_id == ^submission_id and a.question_id == ^question_id)
+        |> join(:inner, [a], s in assoc(a, :submission))
+        |> join(:inner, [a, s], t in subquery(students), t.id == s.student_id)
+        |> Repo.one()
+
+      with {:answer_found?, true} <- {:answer_found?, is_map(answer)},
+           {:valid, changeset = %Ecto.Changeset{valid?: true}} <-
+             {:valid, Answer.grading_changeset(answer, attrs)},
+           {:ok, _} <- Repo.update(changeset) do
+        {:ok, nil}
+      else
+        {:answer_found?, false} ->
+          {:error, {:bad_request, "Answer not found or user not permitted to grade."}}
+
+        {:valid, changeset} ->
+          {:error, {:bad_request, full_error_messages(changeset.errors)}}
+
+        {:error, _} ->
+          {:error, {:internal_server_error, "Please try again later."}}
+      end
+    else
+      {:error, {:unauthorized, "User is not permitted to grade."}}
+    end
+  end
+
   defp find_submission(user = %User{}, assessment = %Assessment{}) do
     submission =
       Submission
@@ -211,25 +301,4 @@ defmodule Cadet.Assessments do
         %{code: raw_answer}
     end
   end
-
-  # TODO: Decide what to do with these methods
-  # def create_multiple_choice_question(json_attr) when is_binary(json_attr) do
-  #  %MCQQuestion{}
-  #  |> MCQQuestion.changeset(%{raw_mcqquestion: json_attr})
-  # end
-
-  # def create_multiple_choice_question(attr = %{}) do
-  #  %MCQQuestion{}
-  #  |> MCQQuestion.changeset(attr)
-  # end
-
-  # def create_programming_question(json_attr) when is_binary(json_attr) do
-  #  %ProgrammingQuestion{}
-  #  |> ProgrammingQuestion.changeset(%{raw_programmingquestion: json_attr})
-  # end
-
-  # def create_programming_question(attr = %{}) do
-  #  %ProgrammingQuestion{}
-  #  |> ProgrammingQuestion.changeset(attr)
-  # end
 end

lib/cadet/assessments/library.ex

@@ -5,13 +5,13 @@ defmodule Cadet.Assessments.Library do
   use Cadet, :model
 
   embedded_schema do
-    field(:version, :integer, default: 1)
+    field(:chapter, :integer, default: 1)
     field(:globals, {:array, :string}, default: [])
     field(:externals, {:array, :string}, default: [])
     field(:files, {:array, :string}, default: [])
   end
 
-  @required_fields ~w(version)a
+  @required_fields ~w(chapter)a
   @optional_fields ~w(globals externals files)a
 
   def changeset(library, params \\ %{}) do

lib/cadet/assessments/query.ex

@@ -0,0 +1,39 @@
+defmodule Cadet.Assessments.Query do
+  @moduledoc """
+  Generate queries related to the Assessments context
+  """
+  import Ecto.Query
+
+  alias Cadet.Assessments.{Answer, Assessment, Question, Submission}
+
+  @spec all_submissions_with_xp :: Submission.t()
+  def all_submissions_with_xp do
+    Submission
+    |> join(:inner, [s], q in subquery(submissions_xp()), s.id == q.submission_id)
+    |> select([s, q], %Submission{s | xp: q.xp})
+  end
+
+  @spec all_assessments_with_max_xp :: Assessment.t()
+  def all_assessments_with_max_xp do
+    Assessment
+    |> join(:inner, [a], q in subquery(assessments_max_xp()), a.id == q.assessment_id)
+    |> select([a, q], %Assessment{a | max_xp: q.max_xp})
+  end
+
+  @spec submissions_xp :: %{submission_id: integer(), xp: integer()}
+  def submissions_xp do
+    Answer
+    |> group_by(:submission_id)
+    |> select([a], %{
+      submission_id: a.submission_id,
+      xp: fragment("? + ?", sum(a.xp), sum(a.adjustment))
+    })
+  end
+
+  @spec assessments_max_xp :: %{assessment_id: integer(), max_xp: integer()}
+  def assessments_max_xp do
+    Question
+    |> group_by(:assessment_id)
+    |> select([q], %{assessment_id: q.assessment_id, max_xp: sum(q.max_xp)})
+  end
+end

lib/cadet/course/course.ex

@@ -7,7 +7,7 @@ defmodule Cadet.Course do
 
   alias Cadet.Accounts.User
   alias Cadet.Course.Announcement
-  alias Cadet.Course.Group
+  # alias Cadet.Course.Group
   alias Cadet.Course.Material
   alias Cadet.Course.Upload
 

lib/cadet/course/query.ex

@@ -4,7 +4,6 @@ defmodule Cadet.Course.Query do
   """
   import Ecto.Query
 
-  alias Cadet.Accounts.User
   alias Cadet.Course.Material
 
   def material_folder_files(folder_id) do

lib/cadet/factory.ex

@@ -7,12 +7,19 @@ defmodule Cadet.Factory do
   @dialyzer {:no_return, fields_for: 1}
 
   alias Cadet.Accounts.{Authorization, User}
-  alias Cadet.Course.{Announcement, Material}
-  alias Cadet.Assessments.{Assessment, Question, Submission}
+  alias Cadet.Assessments.{Answer, Assessment, Question, Submission}
+  alias Cadet.Course.{Announcement, Group, Material}
 
   def user_factory do
     %User{
       name: "John Smith",
+      role: :staff
+    }
+  end
+
+  def student_factory do
+    %User{
+      name: sequence("student"),
       role: :student
     }
   end
@@ -25,6 +32,12 @@ defmodule Cadet.Factory do
     }
   end
 
+  def group_factory do
+    %Group{
+      name: sequence("group")
+    }
+  end
+
   def announcement_factory do
     %Announcement{
       title: sequence(:title, &"Announcement #{&1}"),
@@ -78,10 +91,30 @@ defmodule Cadet.Factory do
 
   def question_factory do
     %Question{
-      title: "question",
+      title: sequence("question"),
       question: %{},
       type: Enum.random([:programming, :multiple_choice]),
       assessment: build(:assessment, %{is_published: true})
     }
   end
+
+  def programming_question_factory do
+    %{
+      content: sequence("ProgrammingQuestion"),
+      solution_template: "f => f(f);",
+      solution: "(f => f(f))(f => f(f));"
+    }
+  end
+
+  def answer_factory do
+    %Answer{
+      answer: %{}
+    }
+  end
+
+  def programming_answer_factory do
+    %{
+      code: sequence(:code, &"alert(#{&1})")
+    }
+  end
 end

lib/cadet/helpers/context_helper.ex

@@ -17,4 +17,10 @@ defmodule Cadet.ContextHelper do
       Repo.update(changeset)
     end
   end
+
+  defmacro is_ecto_id(id) do
+    quote do
+      is_integer(unquote(id)) or is_binary(unquote(id))
+    end
+  end
 end