diff --git a/config/config.exs b/config/config.exs index 244fe1e2e..c998adfe2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -23,7 +23,9 @@ config :cadet, Cadet.Jobs.Scheduler, # Compute contest leaderboard that close in the previous day at 00:01 {"1 0 * * *", {Cadet.Assessments, :update_final_contest_leaderboards, []}}, # Compute rolling leaderboard every 2 hours - {"0 */2 * * *", {Cadet.Assessments, :update_rolling_contest_leaderboards, []}} + {"0 */2 * * *", {Cadet.Assessments, :update_rolling_contest_leaderboards, []}}, + # Collate contest entries that close in the previous day at 00:01 + {"1 0 * * *", {Cadet.Assessments, :update_final_contest_entries, []}} ] # Configures the endpoint diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 1fa79ebc2..57d622ef3 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -23,6 +23,7 @@ defmodule Cadet.Assessments do alias Cadet.ProgramAnalysis.Lexer alias Ecto.Multi alias Cadet.Incentives.Achievements + alias Timex.Duration require Decimal @@ -500,6 +501,35 @@ defmodule Cadet.Assessments do Question.changeset(%Question{}, params_with_assessment_id) end + def update_final_contest_entries do + # 1435 = 1 day - 5 minutes + if Log.log_execution("update_final_contest_entries", Duration.from_minutes(1435)) do + Logger.info("Started update of contest entry pools") + questions = fetch_unassigned_voting_questions() + + for q <- questions do + insert_voting(q.course_id, q.question["contest_number"], q.question_id) + end + + Logger.info("Successfully update contest entry pools") + end + end + + # fetch voting questions where entries have not been assigned + def fetch_unassigned_voting_questions do + voting_assigned_question_ids = + SubmissionVotes + |> select([v], v.question_id) + |> Repo.all() + + Question + |> where(type: :voting) + |> where([q], q.id not in ^voting_assigned_question_ids) + |> join(:inner, [q], asst in assoc(q, :assessment)) + |> select([q, asst], %{course_id: asst.course_id, question: q.question, question_id: q.id}) + |> Repo.all() + end + @doc """ Generates and assigns contest entries for users with given usernames. """ @@ -522,102 +552,119 @@ defmodule Cadet.Assessments do {:error, error_changeset} else - # Returns contest submission ids with answers that contain "return" - contest_submission_ids = - Submission - |> join(:inner, [s], ans in assoc(s, :answers)) - |> join(:inner, [s, ans], cr in assoc(s, :student)) - |> where([s, ans, cr], cr.role == "student") - |> where([s, _], s.assessment_id == ^contest_assessment.id and s.status == "submitted") - |> where( - [_, ans, cr], - fragment( - "?->>'code' like ?", - ans.answer, - "%return%" - ) + if Timex.compare(contest_assessment.close_at, Timex.now()) < 0 do + compile_entries(course_id, contest_assessment, question_id) + else + # contest has not closed, do nothing + {:ok, nil} + end + end + end + + def compile_entries( + course_id, + contest_assessment, + question_id + ) do + # Returns contest submission ids with answers that contain "return" + contest_submission_ids = + Submission + |> join(:inner, [s], ans in assoc(s, :answers)) + |> join(:inner, [s, ans], cr in assoc(s, :student)) + |> where([s, ans, cr], cr.role == "student") + |> where([s, _], s.assessment_id == ^contest_assessment.id and s.status == "submitted") + |> where( + [_, ans, cr], + fragment( + "?->>'code' like ?", + ans.answer, + "%return%" ) - |> select([s, _ans], {s.student_id, s.id}) - |> Repo.all() - |> Enum.into(%{}) + ) + |> select([s, _ans], {s.student_id, s.id}) + |> Repo.all() + |> Enum.into(%{}) - contest_submission_ids_length = Enum.count(contest_submission_ids) + contest_submission_ids_length = Enum.count(contest_submission_ids) - voter_ids = - CourseRegistration - |> where(role: "student", course_id: ^course_id) - |> select([cr], cr.id) - |> Repo.all() + voter_ids = + CourseRegistration + |> where(role: "student", course_id: ^course_id) + |> select([cr], cr.id) + |> Repo.all() - votes_per_user = min(contest_submission_ids_length, 10) + votes_per_user = min(contest_submission_ids_length, 10) - votes_per_submission = - if Enum.empty?(contest_submission_ids) do - 0 - else - trunc(Float.ceil(votes_per_user * length(voter_ids) / contest_submission_ids_length)) - end + votes_per_submission = + if Enum.empty?(contest_submission_ids) do + 0 + else + trunc(Float.ceil(votes_per_user * length(voter_ids) / contest_submission_ids_length)) + end - submission_id_list = - contest_submission_ids - |> Enum.map(fn {_, s_id} -> s_id end) - |> Enum.shuffle() - |> List.duplicate(votes_per_submission) - |> List.flatten() - - {_submission_map, submission_votes_changesets} = - voter_ids - |> Enum.reduce({submission_id_list, []}, fn voter_id, acc -> - {submission_list, submission_votes} = acc - - user_contest_submission_id = Map.get(contest_submission_ids, voter_id) - - {votes, rest} = - submission_list - |> Enum.reduce_while({MapSet.new(), submission_list}, fn s_id, acc -> - {user_votes, submissions} = acc - - max_votes = - if votes_per_user == contest_submission_ids_length and - not is_nil(user_contest_submission_id) do - # no. of submssions is less than 10. Unable to find - votes_per_user - 1 - else - votes_per_user - end - - if MapSet.size(user_votes) < max_votes do - if s_id != user_contest_submission_id and not MapSet.member?(user_votes, s_id) do - new_user_votes = MapSet.put(user_votes, s_id) - new_submissions = List.delete(submissions, s_id) - {:cont, {new_user_votes, new_submissions}} - else - {:cont, {user_votes, submissions}} - end + submission_id_list = + contest_submission_ids + |> Enum.map(fn {_, s_id} -> s_id end) + |> Enum.shuffle() + |> List.duplicate(votes_per_submission) + |> List.flatten() + + {_submission_map, submission_votes_changesets} = + voter_ids + |> Enum.reduce({submission_id_list, []}, fn voter_id, acc -> + {submission_list, submission_votes} = acc + + user_contest_submission_id = Map.get(contest_submission_ids, voter_id) + + {votes, rest} = + submission_list + |> Enum.reduce_while({MapSet.new(), submission_list}, fn s_id, acc -> + {user_votes, submissions} = acc + + max_votes = + if votes_per_user == contest_submission_ids_length and + not is_nil(user_contest_submission_id) do + # no. of submssions is less than 10. Unable to find + votes_per_user - 1 else - {:halt, acc} + votes_per_user end - end) - votes = MapSet.to_list(votes) - - new_submission_votes = - votes - |> Enum.map(fn s_id -> - %SubmissionVotes{voter_id: voter_id, submission_id: s_id, question_id: question_id} - end) - |> Enum.concat(submission_votes) - - {rest, new_submission_votes} - end) - - submission_votes_changesets - |> Enum.with_index() - |> Enum.reduce(Multi.new(), fn {changeset, index}, multi -> - Multi.insert(multi, Integer.to_string(index), changeset) + if MapSet.size(user_votes) < max_votes do + if s_id != user_contest_submission_id and not MapSet.member?(user_votes, s_id) do + new_user_votes = MapSet.put(user_votes, s_id) + new_submissions = List.delete(submissions, s_id) + {:cont, {new_user_votes, new_submissions}} + else + {:cont, {user_votes, submissions}} + end + else + {:halt, acc} + end + end) + + votes = MapSet.to_list(votes) + + new_submission_votes = + votes + |> Enum.map(fn s_id -> + %SubmissionVotes{ + voter_id: voter_id, + submission_id: s_id, + question_id: question_id + } + end) + |> Enum.concat(submission_votes) + + {rest, new_submission_votes} end) - |> Repo.transaction() - end + + submission_votes_changesets + |> Enum.with_index() + |> Enum.reduce(Multi.new(), fn {changeset, index}, multi -> + Multi.insert(multi, Integer.to_string(index), changeset) + end) + |> Repo.transaction() end def update_assessment(id, params) when is_ecto_id(id) do @@ -1026,7 +1073,7 @@ defmodule Cadet.Assessments do """ def update_rolling_contest_leaderboards do # 115 = 2 hours - 5 minutes is default. - if Log.log_execution("update_rolling_contest_leaderboards", Timex.Duration.from_minutes(115)) do + if Log.log_execution("update_rolling_contest_leaderboards", Duration.from_minutes(115)) do Logger.info("Started update_rolling_contest_leaderboards") voting_questions_to_update = fetch_active_voting_questions() @@ -1053,7 +1100,7 @@ defmodule Cadet.Assessments do """ def update_final_contest_leaderboards do # 1435 = 24 hours - 5 minutes - if Log.log_execution("update_final_contest_leaderboards", Timex.Duration.from_minutes(1435)) do + if Log.log_execution("update_final_contest_leaderboards", Duration.from_minutes(1435)) do Logger.info("Started update_final_contest_leaderboards") voting_questions_to_update = fetch_voting_questions_due_yesterday() diff --git a/test/cadet/assessments/assessments_test.exs b/test/cadet/assessments/assessments_test.exs index 319f3aba4..70fe132cd 100644 --- a/test/cadet/assessments/assessments_test.exs +++ b/test/cadet/assessments/assessments_test.exs @@ -147,16 +147,27 @@ defmodule Cadet.AssessmentsTest do end describe "contest voting" do - test "inserts votes into submission_votes table" do - contest_question = insert(:programming_question) - contest_assessment = contest_question.assessment - course = contest_question.assessment.course + test "inserts votes into submission_votes table if contest has closed" do + course = insert(:course) + config = insert(:assessment_config) + # contest assessment that has closed + closed_contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -1), + course: course, + config: config + ) + + contest_question = insert(:programming_question, assessment: closed_contest_assessment) voting_assessment = insert(:assessment, %{course: course}) question = insert(:voting_question, %{ assessment: voting_assessment, - question: build(:voting_question_content, contest_number: contest_assessment.number) + question: + build(:voting_question_content, contest_number: closed_contest_assessment.number) }) students = @@ -202,7 +213,226 @@ defmodule Cadet.AssessmentsTest do # students with own contest submissions will vote for 5 entries # students without own contest submissin will vote for 6 entries - assert length(Repo.all(SubmissionVotes, question_id: question.id)) == 6 * 5 + 6 + assert SubmissionVotes |> where(question_id: ^question.id) |> Repo.all() |> length() == + 6 * 5 + 6 + end + + test "does not insert entries for voting if contest is still open" do + course = insert(:course) + config = insert(:assessment_config) + # contest assessment that is still open + open_contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: 1), + course: course, + config: config + ) + + contest_question = insert(:programming_question, assessment: open_contest_assessment) + voting_assessment = insert(:assessment, %{course: course}) + + question = + insert(:voting_question, %{ + assessment: voting_assessment, + question: + build(:voting_question_content, contest_number: open_contest_assessment.number) + }) + + students = + insert_list(6, :course_registration, %{ + role: :student, + course: course + }) + + Enum.map(students, fn student -> + submission = + insert(:submission, + student: student, + assessment: contest_question.assessment, + status: "submitted" + ) + + insert(:answer, + answer: %{code: "return 2;"}, + submission: submission, + question: contest_question + ) + end) + + Assessments.insert_voting(course.id, contest_question.assessment.number, question.id) + + # No entries should be released for students to vote on while the contest is still open + assert SubmissionVotes |> where(question_id: ^question.id) |> Repo.all() |> length() == 0 + end + + test "function that checks for closed contests and releases entries into voting pool" do + course = insert(:course) + config = insert(:assessment_config) + # contest assessment that has closed + closed_contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -1), + course: course, + config: config + ) + + # contest assessment that is still open + open_contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: 1), + course: course, + config: config + ) + + # contest assessment that is closed but insert_voting has already been done + compiled_contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -1), + course: course, + config: config + ) + + closed_contest_question = + insert(:programming_question, assessment: closed_contest_assessment) + + open_contest_question = insert(:programming_question, assessment: open_contest_assessment) + + compiled_contest_question = + insert(:programming_question, assessment: compiled_contest_assessment) + + closed_voting_assessment = insert(:assessment, %{course: course}) + open_voting_assessment = insert(:assessment, %{course: course}) + compiled_voting_assessment = insert(:assessment, %{course: course}) + + closed_question = + insert(:voting_question, %{ + assessment: closed_voting_assessment, + question: + build(:voting_question_content, contest_number: closed_contest_assessment.number) + }) + + open_question = + insert(:voting_question, %{ + assessment: open_voting_assessment, + question: + build(:voting_question_content, contest_number: open_contest_assessment.number) + }) + + compiled_question = + insert(:voting_question, %{ + assessment: compiled_voting_assessment, + question: + build(:voting_question_content, contest_number: compiled_contest_assessment.number) + }) + + students = + insert_list(10, :course_registration, %{ + role: :student, + course: course + }) + + first_four = Enum.slice(students, 0..3) + last_six = Enum.slice(students, 4..9) + + Enum.map(first_four, fn student -> + submission = + insert(:submission, + student: student, + assessment: compiled_contest_question.assessment, + status: "submitted" + ) + + insert(:answer, + answer: %{code: "return 2;"}, + submission: submission, + question: compiled_contest_question + ) + end) + + # Only the compiled_assessment has already released entries into voting pool + Assessments.insert_voting( + course.id, + compiled_contest_question.assessment.number, + compiled_question.id + ) + + assert SubmissionVotes |> where(question_id: ^closed_question.id) |> Repo.all() |> length() == + 0 + + assert SubmissionVotes |> where(question_id: ^open_question.id) |> Repo.all() |> length() == + 0 + + assert SubmissionVotes + |> where(question_id: ^compiled_question.id) + |> Repo.all() + |> length() == 4 * 3 + 6 * 4 + + Enum.map(students, fn student -> + submission = + insert(:submission, + student: student, + assessment: closed_contest_question.assessment, + status: "submitted" + ) + + insert(:answer, + answer: %{code: "return 2;"}, + submission: submission, + question: closed_contest_question + ) + end) + + Enum.map(students, fn student -> + submission = + insert(:submission, + student: student, + assessment: open_contest_question.assessment, + status: "submitted" + ) + + insert(:answer, + answer: %{code: "return 2;"}, + submission: submission, + question: open_contest_question + ) + end) + + Enum.map(last_six, fn student -> + submission = + insert(:submission, + student: student, + assessment: compiled_contest_question.assessment, + status: "submitted" + ) + + insert(:answer, + answer: %{code: "return 2;"}, + submission: submission, + question: compiled_contest_question + ) + end) + + Assessments.update_final_contest_entries() + + # only the closed_contest should have been updated + assert SubmissionVotes |> where(question_id: ^closed_question.id) |> Repo.all() |> length() == + 10 * 9 + + assert SubmissionVotes |> where(question_id: ^open_question.id) |> Repo.all() |> length() == + 0 + + assert SubmissionVotes + |> where(question_id: ^compiled_question.id) + |> Repo.all() + |> length() == 4 * 3 + 6 * 4 end test "create voting parameters with invalid contest number" do @@ -225,9 +455,19 @@ defmodule Cadet.AssessmentsTest do end test "deletes submission_votes when assessment is deleted" do - contest_question = insert(:programming_question) - course = contest_question.assessment.course - config = contest_question.assessment.config + course = insert(:course) + config = insert(:assessment_config) + # contest assessment that has closed + contest_assessment = + insert(:assessment, + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -1), + course: course, + config: config + ) + + contest_question = insert(:programming_question, assessment: contest_assessment) voting_assessment = insert(:assessment, %{course: course, config: config}) question = insert(:voting_question, assessment: voting_assessment) students = insert_list(5, :course_registration, %{role: :student, course: course})