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

Prevent deletion of contests when contest voting file is present #1002

Merged
merged 28 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 45 additions & 16 deletions lib/cadet/assessments/assessments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,33 @@ defmodule Cadet.Assessments do
def delete_assessment(id) do
assessment = Repo.get(Assessment, id)

Submission
|> where(assessment_id: ^id)
|> delete_submission_assocation(id)
is_voted_on =
Question
|> where(type: :voting)
|> join(:inner, [q], asst in assoc(q, :assessment))
|> where(
[q, asst],
q.question["contest_number"] == ^assessment.number and
asst.course_id == ^assessment.course_id
)
|> Repo.exists?()

Question
|> where(assessment_id: ^id)
|> Repo.all()
|> Enum.each(fn q ->
delete_submission_votes_association(q)
end)
if is_voted_on do
{:error, {:bad_request, "Contest voting for this contest is still up"}}
else
Submission
|> where(assessment_id: ^id)
|> delete_submission_assocation(id)

Repo.delete(assessment)
Question
|> where(assessment_id: ^id)
|> Repo.all()
|> Enum.each(fn q ->
delete_submission_votes_association(q)
end)

Repo.delete(assessment)
end
end

defp delete_submission_votes_association(question) do
Expand Down Expand Up @@ -522,12 +537,26 @@ defmodule Cadet.Assessments do
|> 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()
valid_assessments =
Assessment
|> select([a], %{number: a.number, course_id: a.course_id})
|> Repo.all()

valid_questions =
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()

# fetch only voting where there is a corresponding contest
Enum.filter(valid_questions, fn q ->
Enum.any?(
valid_assessments,
fn a -> a.number == q.question["contest_number"] and a.course_id == q.course_id end
)
end)
end

@doc """
Expand Down
82 changes: 82 additions & 0 deletions test/cadet/assessments/assessments_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ defmodule Cadet.AssessmentsTest do
closed_voting_assessment = insert(:assessment, %{course: course})
open_voting_assessment = insert(:assessment, %{course: course})
compiled_voting_assessment = insert(:assessment, %{course: course})
# voting assessment that references an invalid contest number
invalid_voting_assessment = insert(:assessment, %{course: course})

closed_question =
insert(:voting_question, %{
Expand All @@ -333,6 +335,12 @@ defmodule Cadet.AssessmentsTest do
build(:voting_question_content, contest_number: compiled_contest_assessment.number)
})

invalid_question =
insert(:voting_question, %{
assessment: invalid_voting_assessment,
question: build(:voting_question_content, contest_number: "test_invalid")
})

students =
insert_list(10, :course_registration, %{
role: :student,
Expand Down Expand Up @@ -375,6 +383,9 @@ defmodule Cadet.AssessmentsTest do
|> Repo.all()
|> length() == 4 * 3 + 6 * 4

assert SubmissionVotes |> where(question_id: ^invalid_question.id) |> Repo.all() |> length() ==
0

Enum.map(students, fn student ->
submission =
insert(:submission,
Expand Down Expand Up @@ -420,6 +431,15 @@ defmodule Cadet.AssessmentsTest do
)
end)

# fetching all unassigned voting questions should only yield open and closed questions
unassigned_voting_questions = Assessments.fetch_unassigned_voting_questions()
assert Enum.count(unassigned_voting_questions) == 2

assert Enum.map(unassigned_voting_questions, fn q -> q.question_id end) == [
closed_question.id,
open_question.id
]

Assessments.update_final_contest_entries()

# only the closed_contest should have been updated
Expand All @@ -433,6 +453,9 @@ defmodule Cadet.AssessmentsTest do
|> where(question_id: ^compiled_question.id)
|> Repo.all()
|> length() == 4 * 3 + 6 * 4

assert SubmissionVotes |> where(question_id: ^invalid_question.id) |> Repo.all() |> length() ==
0
end

test "create voting parameters with invalid contest number" do
Expand Down Expand Up @@ -493,6 +516,65 @@ defmodule Cadet.AssessmentsTest do
Assessments.delete_assessment(voting_assessment.id)
refute Repo.exists?(SubmissionVotes, question_id: question.id)
end

test "does not delete contest assessment if referencing voting assessment is present" do
course = insert(:course)
config = insert(:assessment_config)

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,
number: "test"
)

voting_assessment = insert(:assessment, %{course: course, config: config})

# insert voting question that references the contest assessment
_voting_question =
insert(:voting_question, %{
assessment: voting_assessment,
question: build(:voting_question_content, contest_number: contest_assessment.number)
})

error_message = {:bad_request, "Contest voting for this contest is still up"}

assert {:error, ^error_message} = Assessments.delete_assessment(contest_assessment.id)
# deletion should fail
assert Assessment |> where(id: ^contest_assessment.id) |> Repo.exists?()
end

test "deletes contest assessment if voting assessment references same number but different course" do
course_1 = insert(:course)
course_2 = insert(:course)
config = insert(:assessment_config)

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_1,
config: config,
number: "test"
)

voting_assessment = insert(:assessment, %{course: course_2, config: config})

# insert voting question from a different course that references the same contest number
_voting_question =
insert(:voting_question, %{
assessment: voting_assessment,
question: build(:voting_question_content, contest_number: contest_assessment.number)
})

assert {:ok, _} = Assessments.delete_assessment(contest_assessment.id)
# deletion should succeed
refute Assessment |> where(id: ^contest_assessment.id) |> Repo.exists?()
end
end

describe "contest voting leaderboard utility functions" do
Expand Down
Loading