Merge branch 'master' into No-GC-For-Staff

Author: josh1248

lib/cadet/accounts/teams.ex

@@ -9,8 +9,8 @@ defmodule Cadet.Accounts.Teams do
   import Ecto.{Changeset, Query}
 
   alias Cadet.Repo
-  alias Cadet.Accounts.{Team, TeamMember, CourseRegistration, Notification}
-  alias Cadet.Assessments.{Answer, Assessment, Submission}
+  alias Cadet.Accounts.{Team, TeamMember, Notification}
+  alias Cadet.Assessments.{Answer, Submission}
 
   @doc """
   Creates a new team and assigns an assessment and team members to it.

lib/cadet/assessments/assessments.ex

@@ -966,7 +966,17 @@ defmodule Cadet.Assessments do
     end
   end
 
-  defp find_teams(cr_id) do
+  def is_team_assessment?(assessment_id) when is_ecto_id(assessment_id) do
+    max_team_size =
+      Assessment
+      |> where(id: ^assessment_id)
+      |> select([a], a.max_team_size)
+      |> Repo.one()
+
+    max_team_size > 1
+  end
+
+  defp find_teams(cr_id) when is_ecto_id(cr_id) do
     query =
       from(t in Team,
         join: tm in assoc(t, :team_members),
@@ -986,28 +996,14 @@ defmodule Cadet.Assessments do
         limit: 1
       )
 
-    assessment_team_size =
-      Map.get(
-        Repo.one(
-          from(a in Assessment,
-            where: a.id == ^assessment_id,
-            select: %{max_team_size: a.max_team_size}
-          )
-        ),
-        :max_team_size,
-        0
-      )
-
-    case assessment_team_size > 1 do
-      true ->
-        case Repo.one(query) do
-          nil -> {:error, :team_not_found}
-          team -> {:ok, team}
-        end
-
+    if is_team_assessment?(assessment_id) do
+      case Repo.one(query) do
+        nil -> {:error, :team_not_found}
+        team -> {:ok, team}
+      end
+    else
       # team is nil for individual assessments
-      false ->
-        {:ok, nil}
+      {:ok, nil}
     end
   end
 

lib/cadet/assessments/submission.ex

@@ -43,6 +43,7 @@ defmodule Cadet.Assessments.Submission do
     |> validate_xor_relationship
     |> validate_required(@required_fields)
     |> foreign_key_constraint(:student_id)
+    |> foreign_key_constraint(:team_id)
     |> foreign_key_constraint(:assessment_id)
     |> foreign_key_constraint(:unsubmitted_by_id)
   end

lib/cadet/jobs/autograder/grading_job.ex

@@ -9,6 +9,7 @@ defmodule Cadet.Autograder.GradingJob do
 
   require Logger
 
+  alias Cadet.Accounts.{Team, TeamMember}
   alias Cadet.Assessments
   alias Cadet.Assessments.{Answer, Assessment, Question, Submission, SubmissionVotes}
   alias Cadet.Autograder.Utilities
@@ -102,13 +103,68 @@ defmodule Cadet.Autograder.GradingJob do
   end
 
   defp insert_empty_submission(%{student_id: student_id, assessment: assessment}) do
-    %Submission{}
-    |> Submission.changeset(%{
-      student_id: student_id,
-      assessment: assessment,
-      status: :submitted
-    })
-    |> Repo.insert!()
+    if Assessments.is_team_assessment?(assessment.id) do
+      # Get current team if any
+      team =
+        Team
+        |> where(assessment_id: ^assessment.id)
+        |> join(:inner, [t], tm in assoc(t, :team_members))
+        |> where([_, tm], tm.student_id == ^student_id)
+        |> Repo.one()
+
+      team =
+        if team do
+          team
+        else
+          # Student is not in any team
+          # Create new team just for the student
+          team =
+            %Team{}
+            |> Team.changeset(%{
+              assessment_id: assessment.id
+            })
+            |> Repo.insert!()
+
+          %TeamMember{}
+          |> TeamMember.changeset(%{
+            team_id: team.id,
+            student_id: student_id
+          })
+          |> Repo.insert!()
+
+          team
+        end
+
+      find_or_create_team_submission(team.id, assessment)
+    else
+      # Individual assessment
+      %Submission{}
+      |> Submission.changeset(%{
+        student_id: student_id,
+        assessment: assessment,
+        status: :submitted
+      })
+      |> Repo.insert!()
+    end
+  end
+
+  defp find_or_create_team_submission(team_id, assessment) when is_ecto_id(team_id) do
+    submission =
+      Submission
+      |> where(team_id: ^team_id, assessment_id: ^assessment.id)
+      |> Repo.one()
+
+    if submission do
+      submission
+    else
+      %Submission{}
+      |> Submission.changeset(%{
+        team_id: team_id,
+        assessment: assessment,
+        status: :submitted
+      })
+      |> Repo.insert!()
+    end
   end
 
   defp update_submission_status_to_submitted(submission = %Submission{status: status}) do

lib/cadet/jobs/autograder/utilities.ex

@@ -8,7 +8,9 @@ defmodule Cadet.Autograder.Utilities do
 
   import Ecto.Query
 
-  alias Cadet.Accounts.CourseRegistration
+  alias Cadet.Accounts.{CourseRegistration, TeamMember}
+
+  alias Cadet.Assessments
   alias Cadet.Assessments.{Answer, Assessment, Question, Submission}
 
   def dispatch_programming_answer(answer = %Answer{}, question = %Question{}, overwrite \\ false) do
@@ -25,7 +27,37 @@ defmodule Cadet.Autograder.Utilities do
     })
   end
 
-  def fetch_submissions(assessment_id, course_id) when is_ecto_id(assessment_id) do
+  def fetch_submissions(assessment_id, course_id)
+      when is_ecto_id(assessment_id) and is_ecto_id(course_id) do
+    if Assessments.is_team_assessment?(assessment_id) do
+      fetch_team_submissions(assessment_id, course_id)
+    else
+      fetch_student_submissions(assessment_id, course_id)
+    end
+  end
+
+  defp fetch_team_submissions(assessment_id, course_id)
+       when is_ecto_id(assessment_id) and is_ecto_id(course_id) do
+    CourseRegistration
+    |> where(role: "student", course_id: ^course_id)
+    |> join(
+      :left,
+      [cr],
+      tm in TeamMember,
+      on: cr.id == tm.student_id
+    )
+    |> join(
+      :left,
+      [cr, tm],
+      s in Submission,
+      on: tm.team_id == s.team_id and s.assessment_id == ^assessment_id
+    )
+    |> select([cr, tm, s], %{student_id: cr.id, submission: s})
+    |> Repo.all()
+  end
+
+  defp fetch_student_submissions(assessment_id, course_id)
+       when is_ecto_id(assessment_id) and is_ecto_id(course_id) do
     CourseRegistration
     |> where(role: "student", course_id: ^course_id)
     |> join(

mix.lock

@@ -4,7 +4,7 @@
   "artificery": {:hex, :artificery, "0.4.3", "0bc4260f988dcb9dda4b23f9fc3c6c8b99a6220a331534fdf5bf2fd0d4333b02", [:mix], [], "hexpm", "12e95333a30e20884e937abdbefa3e7f5e05609c2ba8cf37b33f000b9ffc0504"},
   "bamboo": {:hex, :bamboo, "2.3.1", "85029339f01c3dd59071cfd2b7b18e826aa7fc172cc324d75603bb99d95b6544", [:mix], [{:hackney, ">= 1.15.2", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:mime, "~> 1.4 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "895b2993ed195b2b0fa79c0d5a1d36aa529e817b6df257e4a10745459048d505"},
   "bamboo_phoenix": {:hex, :bamboo_phoenix, "1.0.0", "f3cc591ffb163ed0bf935d256f1f4645cd870cf436545601215745fb9cc9953f", [:mix], [{:bamboo, ">= 2.0.0", [hex: :bamboo, repo: "hexpm", optional: false]}, {:phoenix, ">= 1.3.0", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "6db88fbb26019c84a47994bb2bd879c0887c29ce6c559bc6385fd54eb8b37dee"},
-  "bamboo_ses": {:hex, :bamboo_ses, "0.4.4", "143afad1cffe07db9c68592d41a0685a2f950d3c31369f9da6030773a61b7566", [:mix], [{:bamboo, "~> 2.0", [hex: :bamboo, repo: "hexpm", optional: false]}, {:ex_aws, "~> 2.4", [hex: :ex_aws, repo: "hexpm", optional: false]}, {:gen_smtp, "~> 1.2.0", [hex: :gen_smtp, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "91740b6b1ab432ca2e41274d097bcf2239149d797acb2b1c4b0d07318bf8aa7e"},
+  "bamboo_ses": {:hex, :bamboo_ses, "0.4.5", "f974bc8321b1520ff042c3415b1f296a5fb3f77c8240bc6721ac73554cf6fbd2", [:mix], [{:bamboo, "~> 2.0", [hex: :bamboo, repo: "hexpm", optional: false]}, {:ex_aws, "~> 2.4", [hex: :ex_aws, repo: "hexpm", optional: false]}, {:gen_smtp, "~> 1.2.0", [hex: :gen_smtp, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "ea3e82b35a7a255690753824392e8eb25f5bf884cfec416deb9a81bbeb1b503b"},
   "blankable": {:hex, :blankable, "1.0.0", "89ab564a63c55af117e115144e3b3b57eb53ad43ba0f15553357eb283e0ed425", [:mix], [], "hexpm", "7cf11aac0e44f4eedbee0c15c1d37d94c090cb72a8d9fddf9f7aec30f9278899"},
   "bunt": {:hex, :bunt, "0.2.1", "e2d4792f7bc0ced7583ab54922808919518d0e57ee162901a16a1b6664ef3b14", [:mix], [], "hexpm", "a330bfb4245239787b15005e66ae6845c9cd524a288f0d141c148b02603777a5"},
   "bypass": {:hex, :bypass, "2.1.0", "909782781bf8e20ee86a9cabde36b259d44af8b9f38756173e8f5e2e1fabb9b1", [:mix], [{:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "d9b5df8fa5b7a6efa08384e9bbecfe4ce61c77d28a4282f79e02f1ef78d96b80"},
@@ -31,7 +31,7 @@
   "erlex": {:hex, :erlex, "0.2.7", "810e8725f96ab74d17aac676e748627a07bc87eb950d2b83acd29dc047a30595", [:mix], [], "hexpm", "3ed95f79d1a844c3f6bf0cea61e0d5612a42ce56da9c03f01df538685365efb0"},
   "esaml": {:hex, :esaml, "4.6.0", "8fb5a3a0d56ccfce3e081a2f72b29058511047a2abbafb64cb6f595bf7465124", [:rebar3], [{:cowboy, "< 3.0.0", [hex: :cowboy, repo: "hexpm", optional: false]}], "hexpm", "d34d0b259cd8ac8215fd2c333fac9dbbb91b5f5da5a9304508612ff3ac0afa7a"},
   "ex2ms": {:hex, :ex2ms, "1.7.0", "45b9f523d0b777667ded60070d82d871a37e294f0b6c5b8eca86771f00f82ee1", [:mix], [], "hexpm", "2589eee51f81f1b1caa6d08c990b1ad409215fe6f64c73f73c67d36ed10be827"},
-  "ex_aws": {:hex, :ex_aws, "2.5.5", "5dc378eff99c3c46c917b7a96a75ad0d4c300ab7250df668d0819bcd18c0213d", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8 or ~> 3.0", [hex: :jsx, repo: "hexpm", optional: true]}, {:mime, "~> 1.2 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:req, "~> 0.3", [hex: :req, repo: "hexpm", optional: true]}, {:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ed7ee39c56012c56600e021953c6487ecce9c49320ec3b4655a15d785f221ca6"},
+  "ex_aws": {:hex, :ex_aws, "2.5.6", "6f642e0f82eff10a9b470044f084b81a791cf15b393d647ea5f3e65da2794e3d", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8 or ~> 3.0", [hex: :jsx, repo: "hexpm", optional: true]}, {:mime, "~> 1.2 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:req, "~> 0.3", [hex: :req, repo: "hexpm", optional: true]}, {:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c69eec59e31fdd89d0beeb1d97e16518dd1b23ad95b3d5c9f1dcfec23d97f960"},
   "ex_aws_lambda": {:hex, :ex_aws_lambda, "2.1.0", "f28bffae6dde34ba17ef815ef50a82a1e7f3af26d36fe31dbda3a7657e0de449", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}], "hexpm", "25608630b8b45fe22b0237696662f88be724bc89f77ee42708a5871511f531a0"},
   "ex_aws_s3": {:hex, :ex_aws_s3, "2.4.0", "ce8decb6b523381812798396bc0e3aaa62282e1b40520125d1f4eff4abdff0f4", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}, {:sweet_xml, ">= 0.0.0", [hex: :sweet_xml, repo: "hexpm", optional: true]}], "hexpm", "85dda6e27754d94582869d39cba3241d9ea60b6aa4167f9c88e309dc687e56bb"},
   "ex_aws_secretsmanager": {:hex, :ex_aws_secretsmanager, "2.0.0", "deff8c12335f0160882afeb9687e55a97fddcd7d9a82fc3a6fbb270797374773", [:mix], [{:ex_aws, "~> 2.0", [hex: :ex_aws, repo: "hexpm", optional: false]}], "hexpm", "8b2838af536c32263ff797012b29e87bad73ef34f43cfa60ebca8e84576f6d45"},

priv/repo/migrations/20241014200600_create_teams_submission_constraint.exs

@@ -0,0 +1,8 @@
+defmodule Cadet.Repo.Migrations.CreateTeamsSubmissionConstraint do
+  use Ecto.Migration
+
+  def change do
+    create(index(:submissions, :team_id))
+    create(unique_index(:submissions, [:assessment_id, :team_id]))
+  end
+end

test/cadet/jobs/autograder/utilities_test.exs

@@ -72,18 +72,58 @@ defmodule Cadet.Autograder.UtilitiesTest do
     setup do
       course = insert(:course)
       config = insert(:assessment_config, %{course: course})
-      assessment = insert(:assessment, %{is_published: true, course: course, config: config})
+
+      # Individual assessment
+      assessment =
+        insert(:assessment, %{
+          is_published: true,
+          course: course,
+          config: config,
+          max_team_size: 1
+        })
+
       students = insert_list(5, :course_registration, %{role: :student, course: course})
       insert(:course_registration, %{course: build(:course), role: :student})
-      %{students: students, assessment: assessment}
+
+      # Team assessment
+      team_assessment =
+        insert(:assessment, %{
+          is_published: true,
+          course: course,
+          config: config,
+          max_team_size: 2
+        })
+
+      team_member1 = insert(:team_member, %{student: Enum.at(students, 0)})
+      team_member2 = insert(:team_member, %{student: Enum.at(students, 1)})
+
+      team1 =
+        insert(:team, %{assessment: team_assessment, team_members: [team_member1, team_member2]})
+
+      team_member3 = insert(:team_member, %{student: Enum.at(students, 2)})
+      team_member4 = insert(:team_member, %{student: Enum.at(students, 3)})
+
+      team2 =
+        insert(:team, %{assessment: team_assessment, team_members: [team_member3, team_member4]})
+
+      %{
+        individual: %{students: students, assessment: assessment},
+        team: %{
+          teams: [team1, team2],
+          assessment: team_assessment,
+          teamless: [Enum.at(students, 4)]
+        }
+      }
     end
 
-    test "it returns list of students with matching submissions", %{
-      students: students,
-      assessment: assessment
+    test "it returns list of students with matching submissions for individual assessments", %{
+      individual: %{students: students, assessment: assessment}
     } do
       submissions =
-        Enum.map(students, &insert(:submission, %{assessment: assessment, student: &1}))
+        Enum.map(
+          students,
+          &insert(:submission, %{assessment: assessment, student: &1, team: nil})
+        )
 
       expected = Enum.map(submissions, &%{student_id: &1.student_id, submission_id: &1.id})
 
@@ -95,9 +135,8 @@ defmodule Cadet.Autograder.UtilitiesTest do
       assert results == expected
     end
 
-    test "it returns list of students with without matching submissions", %{
-      students: students,
-      assessment: assessment
+    test "it returns list of students without matching submissions for individual assessments", %{
+      individual: %{students: students, assessment: assessment}
     } do
       expected_student_ids = Enum.map(students, & &1.id)
 
@@ -108,6 +147,45 @@ defmodule Cadet.Autograder.UtilitiesTest do
 
       assert results |> Enum.map(& &1.submission) |> Enum.uniq() == [nil]
     end
+
+    test "it returns list of students both with and without matching submissions for team assessments",
+         %{
+           team: %{teams: teams, assessment: assessment, teamless: teamless}
+         } do
+      submissions =
+        Enum.map(
+          teams,
+          &%{
+            team_id: &1.id,
+            submission: insert(:submission, %{assessment: assessment, team: &1, student: nil})
+          }
+        )
+
+      expected =
+        teams
+        |> Enum.flat_map(& &1.team_members)
+        |> Enum.map(
+          &%{
+            student_id: &1.student_id,
+            submission_id:
+              Enum.find(submissions, fn s -> s.team_id == &1.team_id end).submission.id
+          }
+        )
+
+      expected = expected ++ Enum.map(teamless, &%{student_id: &1.id, submission_id: nil})
+
+      results =
+        assessment.id
+        |> Utilities.fetch_submissions(assessment.course_id)
+        |> Enum.map(
+          &%{
+            student_id: &1.student_id,
+            submission_id: if(&1.submission, do: &1.submission.id, else: nil)
+          }
+        )
+
+      assert results == expected
+    end
   end
 
   defp get_assessments_ids(assessments) do