From bab5d3921e2cc40a4f3c3e28e89e13b4527c97e4 Mon Sep 17 00:00:00 2001 From: thortol Date: Wed, 28 Feb 2024 21:18:30 +0800 Subject: [PATCH 01/15] bonus exp removed and awarded through grader --- lib/cadet/assessments/assessments.ex | 26 ++---------- lib/cadet/jobs/autograder/grading_job.ex | 52 +++++++++++++++++++++++- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index e1327bf0a..2dfecaaf7 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -807,7 +807,7 @@ defmodule Cadet.Assessments do def finalise_submission(submission = %Submission{}) do with {:status, :attempted} <- {:status, submission.status}, - {:ok, updated_submission} <- update_submission_status_and_xp_bonus(submission) do + {:ok, updated_submission} <- update_submission_status(submission) do # Couple with update_submission_status_and_xp_bonus to ensure notification is sent Notifications.write_notification_when_student_submits(submission) # Send email notification to avenger @@ -920,29 +920,11 @@ defmodule Cadet.Assessments do end end - @spec update_submission_status_and_xp_bonus(Submission.t()) :: + @spec update_submission_status(Submission.t()) :: {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} - defp update_submission_status_and_xp_bonus(submission = %Submission{}) do - assessment = submission.assessment - assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) - - max_bonus_xp = assessment_conifg.early_submission_xp - early_hours = assessment_conifg.hours_before_early_xp_decay - - xp_bonus = - if Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: early_hours)) do - max_bonus_xp - else - # This logic interpolates from max bonus at early hour to 0 bonus at close time - decaying_hours = Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours - remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) - proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) - bonus_xp = round(max_bonus_xp * proportion) - Enum.max([0, bonus_xp]) - end - + defp update_submission_status(submission = %Submission{}) do submission - |> Submission.changeset(%{status: :submitted, xp_bonus: xp_bonus}) + |> Submission.changeset(%{status: :submitted}) |> Repo.update() end diff --git a/lib/cadet/jobs/autograder/grading_job.ex b/lib/cadet/jobs/autograder/grading_job.ex index 89afb69ba..112814128 100644 --- a/lib/cadet/jobs/autograder/grading_job.ex +++ b/lib/cadet/jobs/autograder/grading_job.ex @@ -9,10 +9,12 @@ defmodule Cadet.Autograder.GradingJob do require Logger - alias Cadet.Assessments.{Answer, Assessment, Question, Submission, SubmissionVotes} + alias Cadet.Assessments.{Answer, Assessment, Query, Question, Submission, SubmissionVotes} alias Cadet.Autograder.Utilities + alias Cadet.Courses.{Group, AssessmentConfig} alias Cadet.Jobs.Log + @spec close_and_make_empty_submission(Cadet.Assessments.Assessment.t()) :: list() def close_and_make_empty_submission(assessment = %Assessment{id: id}) do id |> Utilities.fetch_submissions(assessment.course_id) @@ -60,6 +62,54 @@ defmodule Cadet.Autograder.GradingJob do assessment = preprocess_assessment_for_grading(assessment) grade_individual_submission(submission, assessment, true, overwrite) + update_xp_bonus(submission) + end + + @spec update_xp_bonus(Submission.t()) :: + {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} + + defp update_xp_bonus(submission = %Submission{id: submission_id}) do + assessment = submission.assessment + assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) + + max_bonus_xp = assessment_conifg.early_submission_xp + early_hours = assessment_conifg.hours_before_early_xp_decay + ans_xp = + Answer + |> where(submission_id: ^submission_id) + |> order_by(:question_id) + |> group_by([a], a.id) + |> select([a], %{ + # 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) + }) + total = + ans_xp + |> subquery + |> select([a], %{ + total_xp: sum(a.total_xp) + }) + |> Repo.one() + xp = Decimal.to_integer(total.total_xp) + xp_bonus = + if xp <= 0 do + 0 + else + if Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: early_hours)) do + max_bonus_xp + else + # This logic interpolates from max bonus at early hour to 0 bonus at close time + decaying_hours = Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours + remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) + proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) + bonus_xp = round(max_bonus_xp * proportion) + Enum.max([0, bonus_xp]) + end + end + submission + |> Submission.changeset(%{xp_bonus: xp_bonus}) + |> Repo.update() end # This function requires that assessment questions are already preloaded in sorted From c7e13ed9d6affcb8f4ec46691ff4186d7f0a1179 Mon Sep 17 00:00:00 2001 From: thortol Date: Tue, 12 Mar 2024 23:19:26 +0800 Subject: [PATCH 02/15] implemented give bonus exp after tutor is done marking --- lib/cadet/assessments/assessments.ex | 53 ++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 2dfecaaf7..08b40166d 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1665,6 +1665,12 @@ defmodule Cadet.Assessments do {:ok, _} <- Repo.update(changeset) do if is_fully_graded?(answer) and not is_own_submission do # Every answer in this submission has been graded manually + submission = + Submission + |> join(:inner, [s], a in assoc(s, :assessment)) + |> preload([_, a], assessment: a) + |> Repo.get(submission_id) + update_xp_bonus(submission) Notifications.write_notification_when_graded(submission_id, :graded) else {:ok, nil} @@ -1692,6 +1698,53 @@ defmodule Cadet.Assessments do {:error, {:forbidden, "User is not permitted to grade."}} end + @spec update_xp_bonus(Submission.t()) :: + {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} + + defp update_xp_bonus(submission = %Submission{id: submission_id}) do + assessment = submission.assessment + assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) + + max_bonus_xp = assessment_conifg.early_submission_xp + early_hours = assessment_conifg.hours_before_early_xp_decay + ans_xp = + Answer + |> where(submission_id: ^submission_id) + |> order_by(:question_id) + |> group_by([a], a.id) + |> select([a], %{ + # 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) + }) + total = + ans_xp + |> subquery + |> select([a], %{ + total_xp: sum(a.total_xp) + }) + |> Repo.one() + xp = Decimal.to_integer(total.total_xp) + xp_bonus = + if xp <= 0 do + 0 + else + if Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: early_hours)) do + max_bonus_xp + else + # This logic interpolates from max bonus at early hour to 0 bonus at close time + decaying_hours = Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours + remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) + proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) + bonus_xp = round(max_bonus_xp * proportion) + Enum.max([0, bonus_xp]) + end + end + submission + |> Submission.changeset(%{xp_bonus: xp_bonus}) + |> Repo.update() + end + @spec force_regrade_submission(integer() | String.t(), CourseRegistration.t()) :: {:ok, nil} | {:error, {:forbidden | :not_found, String.t()}} def force_regrade_submission( From c65fa09bcae0791e522cc3e9ede55b6789cc5f8f Mon Sep 17 00:00:00 2001 From: thortol Date: Wed, 13 Mar 2024 00:03:16 +0800 Subject: [PATCH 03/15] moved update_xp_bonus to assessments.ex --- lib/cadet/assessments/assessments.ex | 1 + lib/cadet/jobs/autograder/grading_job.ex | 48 ------------------------ 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 08b40166d..fead55836 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -817,6 +817,7 @@ defmodule Cadet.Assessments do # Begin autograding job GradingJob.force_grade_individual_submission(updated_submission) + update_xp_bonus(submission) {:ok, nil} else diff --git a/lib/cadet/jobs/autograder/grading_job.ex b/lib/cadet/jobs/autograder/grading_job.ex index 112814128..9be441986 100644 --- a/lib/cadet/jobs/autograder/grading_job.ex +++ b/lib/cadet/jobs/autograder/grading_job.ex @@ -62,54 +62,6 @@ defmodule Cadet.Autograder.GradingJob do assessment = preprocess_assessment_for_grading(assessment) grade_individual_submission(submission, assessment, true, overwrite) - update_xp_bonus(submission) - end - - @spec update_xp_bonus(Submission.t()) :: - {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} - - defp update_xp_bonus(submission = %Submission{id: submission_id}) do - assessment = submission.assessment - assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) - - max_bonus_xp = assessment_conifg.early_submission_xp - early_hours = assessment_conifg.hours_before_early_xp_decay - ans_xp = - Answer - |> where(submission_id: ^submission_id) - |> order_by(:question_id) - |> group_by([a], a.id) - |> select([a], %{ - # 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) - }) - total = - ans_xp - |> subquery - |> select([a], %{ - total_xp: sum(a.total_xp) - }) - |> Repo.one() - xp = Decimal.to_integer(total.total_xp) - xp_bonus = - if xp <= 0 do - 0 - else - if Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: early_hours)) do - max_bonus_xp - else - # This logic interpolates from max bonus at early hour to 0 bonus at close time - decaying_hours = Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours - remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) - proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) - bonus_xp = round(max_bonus_xp * proportion) - Enum.max([0, bonus_xp]) - end - end - submission - |> Submission.changeset(%{xp_bonus: xp_bonus}) - |> Repo.update() end # This function requires that assessment questions are already preloaded in sorted From 998b73e8b0c38571a0b7a27ed2628ae77fb3d6aa Mon Sep 17 00:00:00 2001 From: thortol Date: Sun, 17 Mar 2024 14:41:09 +0800 Subject: [PATCH 04/15] fixed formatting --- lib/cadet/assessments/assessments.ex | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index fead55836..780723526 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1671,6 +1671,7 @@ defmodule Cadet.Assessments do |> join(:inner, [s], a in assoc(s, :assessment)) |> preload([_, a], assessment: a) |> Repo.get(submission_id) + update_xp_bonus(submission) Notifications.write_notification_when_graded(submission_id, :graded) else @@ -1708,6 +1709,7 @@ defmodule Cadet.Assessments do max_bonus_xp = assessment_conifg.early_submission_xp early_hours = assessment_conifg.hours_before_early_xp_decay + ans_xp = Answer |> where(submission_id: ^submission_id) @@ -1718,14 +1720,17 @@ defmodule Cadet.Assessments do # aggregate function total_xp: sum(a.xp) + sum(a.xp_adjustment) }) - total = - ans_xp - |> subquery - |> select([a], %{ - total_xp: sum(a.total_xp) - }) - |> Repo.one() + + total = + ans_xp + |> subquery + |> select([a], %{ + total_xp: sum(a.total_xp) + }) + |> Repo.one() + xp = Decimal.to_integer(total.total_xp) + xp_bonus = if xp <= 0 do 0 @@ -1734,13 +1739,16 @@ defmodule Cadet.Assessments do max_bonus_xp else # This logic interpolates from max bonus at early hour to 0 bonus at close time - decaying_hours = Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours + decaying_hours = + Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours + remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) bonus_xp = round(max_bonus_xp * proportion) Enum.max([0, bonus_xp]) end end + submission |> Submission.changeset(%{xp_bonus: xp_bonus}) |> Repo.update() From 527619f2ce8e1eb3474d5d5139e164616b996069 Mon Sep 17 00:00:00 2001 From: thortol Date: Sun, 17 Mar 2024 19:49:01 +0800 Subject: [PATCH 05/15] removed unnecessary code from grading job --- lib/cadet/jobs/autograder/grading_job.ex | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/cadet/jobs/autograder/grading_job.ex b/lib/cadet/jobs/autograder/grading_job.ex index 9be441986..89afb69ba 100644 --- a/lib/cadet/jobs/autograder/grading_job.ex +++ b/lib/cadet/jobs/autograder/grading_job.ex @@ -9,12 +9,10 @@ defmodule Cadet.Autograder.GradingJob do require Logger - alias Cadet.Assessments.{Answer, Assessment, Query, Question, Submission, SubmissionVotes} + alias Cadet.Assessments.{Answer, Assessment, Question, Submission, SubmissionVotes} alias Cadet.Autograder.Utilities - alias Cadet.Courses.{Group, AssessmentConfig} alias Cadet.Jobs.Log - @spec close_and_make_empty_submission(Cadet.Assessments.Assessment.t()) :: list() def close_and_make_empty_submission(assessment = %Assessment{id: id}) do id |> Utilities.fetch_submissions(assessment.course_id) From 1f407cc83efe5b80b69184a02a8a9611d4142b1f Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 15:24:50 +0800 Subject: [PATCH 06/15] called decimal_to_integer rather than Decimal.to_integer --- lib/cadet/assessments/assessments.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 780723526..f591dba61 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1729,7 +1729,7 @@ defmodule Cadet.Assessments do }) |> Repo.one() - xp = Decimal.to_integer(total.total_xp) + xp = decimal_to_integer(total.total_xp) xp_bonus = if xp <= 0 do From ee9845d4c08d7343588592b11294054829aa4904 Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 19:30:50 +0800 Subject: [PATCH 07/15] updated documentation --- lib/cadet/assessments/assessments.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index f591dba61..d7551c09f 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -808,7 +808,7 @@ defmodule Cadet.Assessments do def finalise_submission(submission = %Submission{}) do with {:status, :attempted} <- {:status, submission.status}, {:ok, updated_submission} <- update_submission_status(submission) do - # Couple with update_submission_status_and_xp_bonus to ensure notification is sent + # Couple with update_submission_status and update_xp_bonus to ensure notification is sent Notifications.write_notification_when_student_submits(submission) # Send email notification to avenger %{notification_type: "assessment_submission", submission_id: updated_submission.id} From 4bb69dc91628ab153e594fd34a00d4730bb5f29e Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 19:31:03 +0800 Subject: [PATCH 08/15] fixed tests --- test/cadet_web/controllers/assessments_controller_test.exs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index f9251d09b..d49be9e0f 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -1084,7 +1084,8 @@ defmodule CadetWeb.AssessmentsControllerTest do :answer, submission: submission, question: question, - answer: %{code: "f => f(f);"} + answer: %{code: "f => f(f);"}, + xp_adjustment: 10 ) conn @@ -1138,7 +1139,8 @@ defmodule CadetWeb.AssessmentsControllerTest do :answer, submission: submission, question: question, - answer: %{code: "f => f(f);"} + answer: %{code: "f => f(f);"}, + xp_adjustment: 10 ) conn From e3e4be0a796eb30dd657e6e1a1a0e0f1c4458cd9 Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 20:48:27 +0800 Subject: [PATCH 09/15] bonus xp is awarded even if marker is the same as user --- lib/cadet/assessments/assessments.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index d7551c09f..4f929844f 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1664,8 +1664,7 @@ defmodule Cadet.Assessments do {:valid, changeset = %Ecto.Changeset{valid?: true}} <- {:valid, Answer.grading_changeset(answer, attrs)}, {:ok, _} <- Repo.update(changeset) do - if is_fully_graded?(answer) and not is_own_submission do - # Every answer in this submission has been graded manually + if is_fully_graded?(answer) do submission = Submission |> join(:inner, [s], a in assoc(s, :assessment)) @@ -1673,6 +1672,9 @@ defmodule Cadet.Assessments do |> Repo.get(submission_id) update_xp_bonus(submission) + end + if is_fully_graded?(answer) and not is_own_submission do + # Every answer in this submission has been graded manually Notifications.write_notification_when_graded(submission_id, :graded) else {:ok, nil} From 48ac19344a34e71c10d86e85d0cf8106ac88fb8b Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 20:48:45 +0800 Subject: [PATCH 10/15] added new tests --- .../assessments_controller_test.exs | 194 +++++++++++++++++- 1 file changed, 191 insertions(+), 3 deletions(-) diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index d49be9e0f..809dee658 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -1046,6 +1046,193 @@ defmodule CadetWeb.AssessmentsControllerTest do end end + test "submission of answer with no effort grants 0 XP bonus", %{ + conn: conn, + courses: %{course1: course1}, + role_crs: role_crs + } do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + assessment_config = + insert( + :assessment_config, + early_submission_xp: 100, + hours_before_early_xp_decay: 48, + course: course1 + ) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -40), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + config: assessment_config, + course: course1 + ) + + question = insert(:programming_question, assessment: assessment) + + group = insert(:group, leader: role_crs.staff) + + course_reg = + insert(:course_registration, %{role: :student, group: group, course: course1}) + + submission = + insert(:submission, assessment: assessment, student: course_reg, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn + |> sign_in(course_reg.user) + |> post(build_url_submit(course1.id, assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == 0 + end + end + + test "submission of answer grants XP bonus only after being marked by an avenger", %{ + conn: conn, + courses: %{course1: course1}, + role_crs: role_crs + } do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + assessment_config = + insert( + :assessment_config, + early_submission_xp: 100, + hours_before_early_xp_decay: 48, + course: course1 + ) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -40), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + config: assessment_config, + course: course1 + ) + + question = insert(:programming_question, assessment: assessment) + + group = insert(:group, leader: role_crs.staff) + + course_reg = + insert(:course_registration, %{role: :student, group: group, course: course1}) + + submission = + insert(:submission, assessment: assessment, student: course_reg, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"}, + xp_adjustment: 0 + ) + + conn + |> sign_in(course_reg.user) + |> post(build_url_submit(course1.id, assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == 0 + + grade_question = fn question -> + Assessments.update_grading_info( + %{submission_id: submission.id, question_id: question.id}, + %{"xp_adjustment" => 10}, + role_crs.staff + ) + end + + grade_question.(question) + + submission_db = Repo.get(Submission, submission.id) + assert submission_db.xp_bonus == 100 + end + end + + test "submission of answer grants 0 XP bonus if an avenger gives 0 as well", %{ + conn: conn, + courses: %{course1: course1}, + role_crs: role_crs + } do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + assessment_config = + insert( + :assessment_config, + early_submission_xp: 100, + hours_before_early_xp_decay: 48, + course: course1 + ) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -40), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + config: assessment_config, + course: course1 + ) + + question = insert(:programming_question, assessment: assessment) + + group = insert(:group, leader: role_crs.staff) + + course_reg = + insert(:course_registration, %{role: :student, group: group, course: course1}) + + submission = + insert(:submission, assessment: assessment, student: course_reg, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"}, + xp_adjustment: 0 + ) + + conn + |> sign_in(course_reg.user) + |> post(build_url_submit(course1.id, assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == 0 + + grade_question = fn question -> + Assessments.update_grading_info( + %{submission_id: submission.id, question_id: question.id}, + %{"xp_adjustment" => 0}, + role_crs.staff + ) + end + + grade_question.(question) + + submission_db = Repo.get(Submission, submission.id) + assert submission_db.xp_bonus == 0 + end + end + test "submission of answer within early hours(seeded 48) of opening grants full XP bonus", %{ conn: conn, courses: %{course1: course1}, @@ -1085,7 +1272,7 @@ defmodule CadetWeb.AssessmentsControllerTest do submission: submission, question: question, answer: %{code: "f => f(f);"}, - xp_adjustment: 10 + xp: 10 ) conn @@ -1140,7 +1327,7 @@ defmodule CadetWeb.AssessmentsControllerTest do submission: submission, question: question, answer: %{code: "f => f(f);"}, - xp_adjustment: 10 + xp: 10 ) conn @@ -1198,7 +1385,8 @@ defmodule CadetWeb.AssessmentsControllerTest do :answer, submission: submission, question: question, - answer: %{code: "f => f(f);"} + answer: %{code: "f => f(f);"}, + xp: 10 ) conn From 126d18f0d096a0e43589d5ac6e678ef969c4c8ad Mon Sep 17 00:00:00 2001 From: thortol Date: Sat, 23 Mar 2024 21:37:26 +0800 Subject: [PATCH 11/15] fixed formatting --- lib/cadet/assessments/assessments.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 4f929844f..cc8a1123c 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1673,6 +1673,7 @@ defmodule Cadet.Assessments do update_xp_bonus(submission) end + if is_fully_graded?(answer) and not is_own_submission do # Every answer in this submission has been graded manually Notifications.write_notification_when_graded(submission_id, :graded) From ef76f3d8ebcc5d78415020ccf0c098b3e2800946 Mon Sep 17 00:00:00 2001 From: thortol Date: Sun, 24 Mar 2024 23:28:35 +0800 Subject: [PATCH 12/15] db migration added submitted_at column to submissions --- ...0240324214100_submissions_add_submitted_at_column.exs | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs diff --git a/priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs b/priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs new file mode 100644 index 000000000..912e3a09f --- /dev/null +++ b/priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs @@ -0,0 +1,9 @@ +defmodule Cadet.Repo.Migrations.SubmissionsAddSubmittedAtColumn do + use Ecto.Migration + + def change do + alter table(:submissions) do + add(:submitted_at, :timestamp, null: true) + end + end +end From dd8b7b8bdf57ec78790b33c39c362862a30779e5 Mon Sep 17 00:00:00 2001 From: thortol Date: Sun, 24 Mar 2024 23:29:33 +0800 Subject: [PATCH 13/15] bonus xp is now calculated from the new submitted_at column --- lib/cadet/assessments/assessments.ex | 92 +++++++++++++++------------- lib/cadet/assessments/submission.ex | 3 +- 2 files changed, 53 insertions(+), 42 deletions(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index cc8a1123c..0c25a6d21 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -925,7 +925,7 @@ defmodule Cadet.Assessments do {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} defp update_submission_status(submission = %Submission{}) do submission - |> Submission.changeset(%{status: :submitted}) + |> Submission.changeset(%{status: :submitted, submitted_at: Timex.now()}) |> Repo.update() end @@ -1707,54 +1707,64 @@ defmodule Cadet.Assessments do {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} defp update_xp_bonus(submission = %Submission{id: submission_id}) do - assessment = submission.assessment - assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) + # to ensure backwards compatibility + if submission.xp_bonus == 0 do + assessment = submission.assessment + assessment_conifg = Repo.get_by(AssessmentConfig, id: assessment.config_id) - max_bonus_xp = assessment_conifg.early_submission_xp - early_hours = assessment_conifg.hours_before_early_xp_decay + max_bonus_xp = assessment_conifg.early_submission_xp + early_hours = assessment_conifg.hours_before_early_xp_decay - ans_xp = - Answer - |> where(submission_id: ^submission_id) - |> order_by(:question_id) - |> group_by([a], a.id) - |> select([a], %{ - # 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) - }) + ans_xp = + Answer + |> where(submission_id: ^submission_id) + |> order_by(:question_id) + |> group_by([a], a.id) + |> select([a], %{ + # 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) + }) - total = - ans_xp - |> subquery - |> select([a], %{ - total_xp: sum(a.total_xp) - }) - |> Repo.one() + total = + ans_xp + |> subquery + |> select([a], %{ + total_xp: sum(a.total_xp) + }) + |> Repo.one() - xp = decimal_to_integer(total.total_xp) + xp = decimal_to_integer(total.total_xp) - xp_bonus = - if xp <= 0 do - 0 - else - if Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: early_hours)) do - max_bonus_xp + cur_time = + if submission.submitted_at == nil do + Timex.now() else - # This logic interpolates from max bonus at early hour to 0 bonus at close time - decaying_hours = - Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours - - remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, Timex.now(), :hours)]) - proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) - bonus_xp = round(max_bonus_xp * proportion) - Enum.max([0, bonus_xp]) + submission.submitted_at end - end - submission - |> Submission.changeset(%{xp_bonus: xp_bonus}) - |> Repo.update() + xp_bonus = + if xp <= 0 do + 0 + else + if Timex.before?(cur_time, Timex.shift(assessment.open_at, hours: early_hours)) do + max_bonus_xp + else + # This logic interpolates from max bonus at early hour to 0 bonus at close time + decaying_hours = + Timex.diff(assessment.close_at, assessment.open_at, :hours) - early_hours + + remaining_hours = Enum.max([0, Timex.diff(assessment.close_at, cur_time, :hours)]) + proportion = if(decaying_hours > 0, do: remaining_hours / decaying_hours, else: 1) + bonus_xp = round(max_bonus_xp * proportion) + Enum.max([0, bonus_xp]) + end + end + + submission + |> Submission.changeset(%{xp_bonus: xp_bonus}) + |> Repo.update() + end end @spec force_regrade_submission(integer() | String.t(), CourseRegistration.t()) :: diff --git a/lib/cadet/assessments/submission.ex b/lib/cadet/assessments/submission.ex index eb8164065..4de531e6b 100644 --- a/lib/cadet/assessments/submission.ex +++ b/lib/cadet/assessments/submission.ex @@ -17,6 +17,7 @@ defmodule Cadet.Assessments.Submission do field(:graded_count, :integer, virtual: true, default: 0) field(:grading_status, :string, virtual: true) field(:unsubmitted_at, :utc_datetime_usec) + field(:submitted_at, :utc_datetime_usec) belongs_to(:assessment, Assessment) belongs_to(:student, CourseRegistration) @@ -27,7 +28,7 @@ defmodule Cadet.Assessments.Submission do end @required_fields ~w(student_id assessment_id status)a - @optional_fields ~w(xp_bonus unsubmitted_by_id unsubmitted_at)a + @optional_fields ~w(xp_bonus unsubmitted_by_id unsubmitted_at submitted_at)a def changeset(submission, params) do submission From fbe2e3445c2488be02a10c09a868c3de54405107 Mon Sep 17 00:00:00 2001 From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com> Date: Sun, 11 Aug 2024 17:58:40 +0800 Subject: [PATCH 14/15] Redate migration file to maintain total ordering --- ...exs => 20240805214100_submissions_add_submitted_at_column.exs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename priv/repo/migrations/{20240324214100_submissions_add_submitted_at_column.exs => 20240805214100_submissions_add_submitted_at_column.exs} (100%) diff --git a/priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs b/priv/repo/migrations/20240805214100_submissions_add_submitted_at_column.exs similarity index 100% rename from priv/repo/migrations/20240324214100_submissions_add_submitted_at_column.exs rename to priv/repo/migrations/20240805214100_submissions_add_submitted_at_column.exs From 7e2b1547fe4fb9fced37e77dd28ec594f7c8ab92 Mon Sep 17 00:00:00 2001 From: Richard Dominick <34370238+RichDom2185@users.noreply.github.com> Date: Sun, 11 Aug 2024 18:02:17 +0800 Subject: [PATCH 15/15] Remove extra newline --- lib/cadet/assessments/assessments.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 1debb3a08..3c6eb0548 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -1424,7 +1424,6 @@ defmodule Cadet.Assessments do @spec update_xp_bonus(Submission.t()) :: {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} - defp update_xp_bonus(submission = %Submission{id: submission_id}) do # to ensure backwards compatibility if submission.xp_bonus == 0 do