Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Auto submission cron job for team assessments bug #1200

Merged
merged 10 commits into from
Oct 14, 2024
4 changes: 2 additions & 2 deletions lib/cadet/accounts/teams.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
40 changes: 18 additions & 22 deletions lib/cadet/assessments/assessments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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

Expand Down
52 changes: 45 additions & 7 deletions lib/cadet/jobs/autograder/grading_job.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -102,13 +103,50 @@ 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()

if !team do
# 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!()
end

%Submission{}
|> Submission.changeset(%{
team_id: team.id,
assessment: assessment,
status: :submitted
})
|> Repo.insert!()
else
# Individual assessment
%Submission{}
|> Submission.changeset(%{
student_id: student_id,
assessment: assessment,
status: :submitted
})
|> Repo.insert!()
end
end

defp update_submission_status_to_submitted(submission = %Submission{status: status}) do
Expand Down
36 changes: 34 additions & 2 deletions lib/cadet/jobs/autograder/utilities.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
96 changes: 87 additions & 9 deletions test/cadet/jobs/autograder/utilities_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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})

Expand All @@ -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)

Expand All @@ -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
Expand Down
Loading