From 3cf8d7d4832e7e887a48529ddcb4b2f332c02d3e Mon Sep 17 00:00:00 2001 From: Marcus <88984742+emptygx@users.noreply.github.com> Date: Sat, 13 Apr 2024 20:03:30 +0800 Subject: [PATCH] Add Publish Grading Feature (#965) * Add migrations to create is_grading_published column for submissions Current submissions will have the is_grading_published field populated as `true` if all answers have been graded i.e. have a grader_id, while the rest will default to `false`. Future submissions will have the field with default value `false` upon creation. * Add and update endpoints for new publish grading feature - Update existing endpoints and queries related to assessment retrieval - Add new routes and endpoints to handle new publish grading feature - Add notification for new feature * Fix missing return tuple values in assessments.ex * Fix notification and formatting * Fix indentation and minor error * Fix formatting * fix formatting * fix credo long line * fix credo nesting alias * Fix formatting and code quality * Fix tests * feat: Implement format helper function Changing xp in the assessment_with_questions_and_answers is the wrong direction, I feel that the function should not be tied to a specific format. Instead, I will call a helper function in the controller to format the xp. * feat: Add formatting function for getting all assessments * fix: Fix bug of using virtual variable instead * refactor: Remove default value for virtual variable is_grading_published * feat: Implement tests for helper functions * feat: Add isGradingPublished to response for fetching all assessments * refactor: Format * chore: Remove unused match * feat: Implement tests for unpublish route * feat: Implement tests for publish route * refactor: Move repeated code into setup for publish test * refactor: Move repeated code into setup for unpublish test * refactor: Format code * feat: Add a guard to prevent unsubmit when grade is still published * feat: Implement filter by notPublished * refactor: Format * fix: Fix incorrect guard check for is_grading_published * refactor: Edit tests to accommodate new guard * feat: Implement test for new guard (check is_grading_published) * feat: Update seed to include is_grading_published * refactor: Update old tests to accommodate for is_grading_published * refactor: Improve filter tests to be more flexible Remove hard coded numbers and count expected number with students_with_assessment_info * feat: Implement test for filter by not published * chore: Format * refactor: Make is_grading_published default to false in factory function * feat: Add is_grading_auto_published column to assessment config * feat: Add guard clause to ensure submission is fully graded before publishing * feat: Implement publish_grading and is_fully_autograded? - Adds another publish_grading function which bypasses all checks - Adds a new function is_fully_autograded? which checks if all answers are successfully auto graded * chore: Clean up code * feat: Add is_grading_auto_published and is_manually_graded to seed * feat: Implement tests for is_fully_autograded?/1 * feat: Implement publish_all_grades route * feat: Add publish_all_grades in controller * feat: Implement publish_all_graded function * feat: Implement tests for publish_all_graded/2 * refactor: Rename param names and allow filter by true or false * refactor: Add tests for change in param and refactor code * refactor: Change response for publish all grades * refactor: Use update_all instead of recursively updating individual submissions * feat: Implement unpublish all grades route * feat: Implement unpublish_all * feat: Implement unpublish_all tests * chore: Format * feat: Implement auto publish for mcq/voting questions * feat: Implement auto publish for auto graded programming questions * chore: Fix consistency issue * feat: Implement published and unpublished notifications and remove deprecated ones * feat: Include isGradingAutoPublished in response for assessment configs * fix: Include sending of notifications when publishing/unpublishing all * docs: Add docs for functions implemented * chore: Update wording for tests * chore: Remove unused variables * feat: Implement test for unpublish and publish all routes * feat: Implement guard for publish/unpublish grades This commit prevents admin/staff from individually publishing/unpublishing grades for students. * refactor: Change notification types in swagger_schema * chore: Add comment in seed * refactor: remove redundant lines * Redate migrations Ensures total ordering of migration files are preserved. * Revert unnecessary changes * Revert more unnecessary changes * refactor: Move duplicate code into helper function * chore: Fix typo * refactor: Change control flow structure --------- Co-authored-by: YaleChen299 Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com> Co-authored-by: GabrielCWT <77312579+GabrielCWT@users.noreply.github.com> --- lib/cadet/accounts/notification_type.ex | 10 +- lib/cadet/accounts/notifications.ex | 33 +- lib/cadet/assessments/assessment.ex | 1 + lib/cadet/assessments/assessments.ex | 394 ++++++++++++++++- lib/cadet/assessments/submission.ex | 3 +- lib/cadet/courses/assessment_config.ex | 3 +- lib/cadet/jobs/autograder/grading_job.ex | 12 +- .../jobs/autograder/result_store_worker.ex | 29 +- lib/cadet/workers/NotificationWorker.ex | 2 +- .../admin_grading_controller.ex | 60 +++ .../admin_views/admin_courses_view.ex | 3 +- .../admin_views/admin_grading_view.ex | 1 + .../controllers/assessments_controller.ex | 10 +- .../controllers/notifications_controller.ex | 10 +- lib/cadet_web/router.ex | 11 + lib/cadet_web/views/assessments_view.ex | 1 + ...0240322122108_add_is_grading_published.exs | 9 + ...0322184853_update_is_grading_published.exs | 24 ++ ...31222300_add_is_grading_auto_published.exs | 15 + priv/repo/seeds.exs | 20 +- test/cadet/accounts/notification_test.exs | 34 +- test/cadet/assessments/assessments_test.exs | 408 ++++++++++++++++-- .../jobs/autograder/grading_job_test.exs | 293 ++++++++++++- .../autograder/result_store_worker_test.exs | 27 +- .../notification_worker_test.exs | 2 +- .../admin_courses_controller_test.exs | 9 +- .../admin_grading_controller_test.exs | 370 +++++++++++++++- .../admin_user_controller_test.exs | 3 +- .../assessments_controller_test.exs | 21 +- .../controllers/user_controller_test.exs | 9 +- .../accounts/notification_factory.ex | 2 +- .../assessments/submission_factory.ex | 3 +- test/support/seeds.ex | 38 +- 33 files changed, 1715 insertions(+), 155 deletions(-) create mode 100644 priv/repo/migrations/20240322122108_add_is_grading_published.exs create mode 100644 priv/repo/migrations/20240322184853_update_is_grading_published.exs create mode 100644 priv/repo/migrations/20240331222300_add_is_grading_auto_published.exs diff --git a/lib/cadet/accounts/notification_type.ex b/lib/cadet/accounts/notification_type.ex index 3ce69f709..a5c62397e 100644 --- a/lib/cadet/accounts/notification_type.ex +++ b/lib/cadet/accounts/notification_type.ex @@ -3,12 +3,12 @@ import EctoEnum defenum(Cadet.Accounts.NotificationType, :notification_type, [ # Notifications for new assessments :new, - # Notifications for autograded assessments - :autograded, - # Notifications for manually graded assessments - :graded, # Notifications for unsubmitted submissions :unsubmitted, # Notifications for submitted assessments - :submitted + :submitted, + # Notifications for published grades + :published_grading, + # Notifications for unpublished grades + :unpublished_grading ]) diff --git a/lib/cadet/accounts/notifications.ex b/lib/cadet/accounts/notifications.ex index b6bd14d64..0dd5d8cc5 100644 --- a/lib/cadet/accounts/notifications.ex +++ b/lib/cadet/accounts/notifications.ex @@ -145,18 +145,41 @@ defmodule Cadet.Accounts.Notifications do {:ok, Ecto.Schema.t()} | {:error, Ecto.Changeset.t()} def handle_unsubmit_notifications(assessment_id, student = %CourseRegistration{}) when is_ecto_id(assessment_id) do - # Fetch and delete all notifications of :autograded and :graded + # Fetch and delete all notifications of :unpublished_grading # Add new notification :unsubmitted Notification |> where(course_reg_id: ^student.id) |> where(assessment_id: ^assessment_id) - |> where([n], n.type in ^[:autograded, :graded]) + |> where([n], n.type in ^[:unpublished_grading]) |> Repo.delete_all() write(%{ type: :unsubmitted, - role: student.role, + role: :student, + course_reg_id: student.id, + assessment_id: assessment_id + }) + end + + @doc """ + Function that handles notifications when a submission grade is unpublished. + Deletes all :published notifications and adds a new :unpublished_grading notification. + """ + @spec handle_unpublish_grades_notifications(integer(), CourseRegistration.t()) :: + {:ok, Ecto.Schema.t()} | {:error, Ecto.Changeset.t()} + def handle_unpublish_grades_notifications(assessment_id, student = %CourseRegistration{}) + when is_ecto_id(assessment_id) do + Notification + |> where(course_reg_id: ^student.id) + |> where(assessment_id: ^assessment_id) + |> where([n], n.type in ^[:published_grading]) + |> Repo.delete_all() + + write(%{ + type: :unpublished_grading, + read: false, + role: :student, course_reg_id: student.id, assessment_id: assessment_id }) @@ -166,9 +189,9 @@ defmodule Cadet.Accounts.Notifications do Writes a notification that a student's submission has been graded successfully. (for the student) """ - @spec write_notification_when_graded(integer(), any()) :: + @spec write_notification_when_published(integer(), any()) :: {:ok, Ecto.Schema.t()} | {:error, Ecto.Changeset.t()} - def write_notification_when_graded(submission_id, type) when type in [:graded, :autograded] do + def write_notification_when_published(submission_id, type) when type in [:published_grading] do case Repo.get(Submission, submission_id) do nil -> {:error, %Ecto.Changeset{}} diff --git a/lib/cadet/assessments/assessment.ex b/lib/cadet/assessments/assessment.ex index de6d750af..db9cc7514 100644 --- a/lib/cadet/assessments/assessment.ex +++ b/lib/cadet/assessments/assessment.ex @@ -20,6 +20,7 @@ defmodule Cadet.Assessments.Assessment do field(:grading_status, :string, virtual: true) field(:question_count, :integer, virtual: true) field(:graded_count, :integer, virtual: true) + field(:is_grading_published, :boolean, virtual: true) field(:title, :string) field(:is_published, :boolean, default: false) field(:summary_short, :string) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 28602b298..acffe823d 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -115,6 +115,7 @@ defmodule Cadet.Assessments do [s], s.id in ^submission_ids ) + |> where(is_grading_published: true) |> join(:inner, [s], a in Answer, on: s.id == a.submission_id) |> group_by([s], s.id) |> select([s, a], %{ @@ -301,7 +302,18 @@ defmodule Cadet.Assessments do end) |> load_contest_voting_entries(course_reg, assessment) - assessment = assessment |> Map.put(:questions, questions) + is_grading_published = + Submission + |> where(assessment_id: ^id) + |> where(student_id: ^course_reg.id) + |> select([s], s.is_grading_published) + |> Repo.one() + + assessment = + assessment + |> Map.put(:questions, questions) + |> Map.put(:is_grading_published, is_grading_published) + {:ok, assessment} else {:error, {:forbidden, "Assessment not open"}} @@ -341,7 +353,7 @@ defmodule Cadet.Assessments do [s], s.id in ^submission_ids ) - |> select([s], [:assessment_id, :status]) + |> select([s], [:assessment_id, :status, :is_grading_published]) assessments = cr.course_id @@ -358,7 +370,8 @@ defmodule Cadet.Assessments do a | xp: sa.xp, graded_count: sa.graded_count, - user_status: s.status + user_status: s.status, + is_grading_published: s.is_grading_published }) |> filter_published_assessments(cr) |> order_by(:open_at) @@ -378,6 +391,52 @@ defmodule Cadet.Assessments do Repo.all(query) end + @doc """ + A helper function which removes grading information from all assessments + if it's grading is not published. + """ + def format_all_assessments(assessments) do + Enum.map(assessments, fn a -> + if a.is_grading_published do + a + else + a + |> Map.put(:xp, 0) + |> Map.put(:graded_count, 0) + end + end) + end + + @doc """ + A helper function which removes grading information from the assessment + if it's grading is not published. + """ + def format_assessment_with_questions_and_answers(assessment) do + if assessment.is_grading_published do + assessment + else + %{ + assessment + | questions: + Enum.map(assessment.questions, fn q -> + %{ + q + | answer: %{ + q.answer + | xp: 0, + xp_adjustment: 0, + autograding_status: :none, + autograding_results: [], + grader: nil, + grader_id: nil, + comments: nil + } + } + end) + } + end + end + def filter_published_assessments(assessments, cr) do role = cr.role @@ -1004,7 +1063,9 @@ defmodule Cadet.Assessments do {:allowed_to_unsubmit?, true} <- {:allowed_to_unsubmit?, role == :admin or bypass or is_nil(submission.student_id) or - Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)} do + Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)}, + {:is_grading_published?, false} <- + {:is_grading_published?, submission.is_grading_published} do Multi.new() |> Multi.run( :rollback_submission, @@ -1064,14 +1125,14 @@ defmodule Cadet.Assessments do team_members = Repo.all(query) Enum.each(team_members, fn tm_id -> - Cadet.Accounts.Notifications.handle_unsubmit_notifications( + Notifications.handle_unsubmit_notifications( submission.assessment.id, Repo.get(CourseRegistration, tm_id) ) end) student_id -> - Cadet.Accounts.Notifications.handle_unsubmit_notifications( + Notifications.handle_unsubmit_notifications( submission.assessment.id, Repo.get(CourseRegistration, student_id) ) @@ -1094,11 +1155,264 @@ defmodule Cadet.Assessments do {:allowed_to_unsubmit?, false} -> {:error, {:forbidden, "Only Avenger of student or Admin is permitted to unsubmit"}} + {:is_grading_published?, true} -> + {:error, {:forbidden, "Grading has not been unpublished"}} + + _ -> + {:error, {:internal_server_error, "Please try again later."}} + end + end + + defp can_publish?(submission_id, cr = %CourseRegistration{id: course_reg_id, role: role}) do + submission = + Submission + |> join(:inner, [s], a in assoc(s, :assessment)) + |> join(:inner, [_, a], c in assoc(a, :config)) + |> preload([_, a, c], assessment: {a, config: c}) + |> Repo.get(submission_id) + + # allows staff to unpublish own assessment + bypass = role in @bypass_closed_roles and submission.student_id == course_reg_id + + with {:submission_found?, true} <- {:submission_found?, is_map(submission)}, + {:status, :submitted} <- {:status, submission.status}, + {:is_manually_graded?, true} <- + {:is_manually_graded?, submission.assessment.config.is_manually_graded}, + {:fully_graded?, true} <- {:fully_graded?, is_fully_graded?(submission_id)}, + {:allowed_to_publish?, true} <- + {:allowed_to_publish?, + role == :admin or bypass or + Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)} do + {:ok, submission} + end + end + + @doc """ + Unpublishes grading for a submission and send notification to student. + Requires admin or staff who is group leader of student. + + Only manually graded assessments can be individually unpublished. We can only + unpublish all submissions for auto-graded assessments. + + Returns `{:ok, nil}` on success, otherwise `{:error, {status, message}}`. + """ + def unpublish_grading(submission_id, cr = %CourseRegistration{}) + when is_ecto_id(submission_id) do + case can_publish?(submission_id, cr) do + {:ok, submission} -> + submission + |> Submission.changeset(%{is_grading_published: false}) + |> Repo.update() + + Notifications.handle_unpublish_grades_notifications( + submission.assessment.id, + Repo.get(CourseRegistration, submission.student_id) + ) + + {:ok, nil} + + {:submission_found?, false} -> + {:error, {:not_found, "Submission not found"}} + + {:allowed_to_publish?, false} -> + {:error, + {:forbidden, "Only Avenger of student or Admin is permitted to unpublish grading"}} + + {:is_manually_graded?, false} -> + {:error, + {:bad_request, "Only manually graded assessments can be individually unpublished"}} + + _ -> + {:error, {:internal_server_error, "Please try again later."}} + end + end + + @doc """ + Publishes grading for a submission and send notification to student. + Requires admin or staff who is group leader of student and all answers to be graded. + + Only manually graded assessments can be individually published. We can only + publish all submissions for auto-graded assessments. + + Returns `{:ok, nil}` on success, otherwise `{:error, {status, message}}`. + """ + def publish_grading(submission_id, cr = %CourseRegistration{}) + when is_ecto_id(submission_id) do + case can_publish?(submission_id, cr) do + {:ok, submission} -> + submission + |> Submission.changeset(%{is_grading_published: true}) + |> Repo.update() + + Notifications.write_notification_when_published( + submission.id, + :published_grading + ) + + {:ok, nil} + + {:submission_found?, false} -> + {:error, {:not_found, "Submission not found"}} + + {:status, :attempting} -> + {:error, {:bad_request, "Some questions have not been attempted"}} + + {:status, :attempted} -> + {:error, {:bad_request, "Assessment has not been submitted"}} + + {:allowed_to_publish?, false} -> + {:error, {:forbidden, "Only Avenger of student or Admin is permitted to publish grading"}} + + {:is_manually_graded?, false} -> + {:error, {:bad_request, "Only manually graded assessments can be individually published"}} + + {:fully_graded?, false} -> + {:error, {:bad_request, "Some answers are not graded"}} + + _ -> + {:error, {:internal_server_error, "Please try again later."}} + end + end + + @doc """ + Publishes grading for a submission and send notification to student. + This function is used by the auto-grading system to publish grading. Bypasses Course Reg checks. + + Returns `{:ok, nil}` on success, otherwise `{:error, {status, message}}`. + """ + def publish_grading(submission_id) + when is_ecto_id(submission_id) do + submission = + Submission + |> join(:inner, [s], a in assoc(s, :assessment)) + |> preload([_, a], assessment: a) + |> Repo.get(submission_id) + + with {:submission_found?, true} <- {:submission_found?, is_map(submission)}, + {:status, :submitted} <- {:status, submission.status} do + submission + |> Submission.changeset(%{is_grading_published: true}) + |> Repo.update() + + Notifications.write_notification_when_published( + submission.id, + :published_grading + ) + + {:ok, nil} + else + {:submission_found?, false} -> + {:error, {:not_found, "Submission not found"}} + + {:status, :attempting} -> + {:error, {:bad_request, "Some questions have not been attempted"}} + + {:status, :attempted} -> + {:error, {:bad_request, "Assessment has not been submitted"}} + _ -> {:error, {:internal_server_error, "Please try again later."}} end end + @doc """ + Publishes grading for all graded submissions for an assessment and sends notifications to students. + Requires admin. + + Returns `{:ok, nil}` on success, otherwise `{:error, {status, message}}`. + """ + def publish_all_graded(publisher = %CourseRegistration{}, assessment_id) do + if publisher.role == :admin do + answers_query = + Answer + |> group_by([ans], ans.submission_id) + |> select([ans], %{ + submission_id: ans.submission_id, + graded_count: filter(count(ans.id), not is_nil(ans.grader_id)), + autograded_count: filter(count(ans.id), ans.autograding_status == :success) + }) + + question_query = + Question + |> group_by([q], q.assessment_id) + |> join(:inner, [q], a in Assessment, on: q.assessment_id == a.id) + |> select([q, a], %{ + assessment_id: q.assessment_id, + question_count: count(q.id) + }) + + submission_query = + Submission + |> join(:inner, [s], ans in subquery(answers_query), on: ans.submission_id == s.id) + |> join(:inner, [s, ans], asst in subquery(question_query), + on: s.assessment_id == asst.assessment_id + ) + |> join(:inner, [s, ans, asst], cr in CourseRegistration, on: s.student_id == cr.id) + |> where([s, ans, asst, cr], cr.course_id == ^publisher.course_id) + |> where( + [s, ans, asst, cr], + asst.question_count == ans.graded_count or asst.question_count == ans.autograded_count + ) + |> where([s, ans, asst, cr], s.is_grading_published == false) + |> where([s, ans, asst, cr], s.assessment_id == ^assessment_id) + |> select([s, ans, asst, cr], %{ + id: s.id + }) + + submissions = Repo.all(submission_query) + + Repo.update_all(submission_query, set: [is_grading_published: true]) + + Enum.each(submissions, fn submission -> + Notifications.write_notification_when_published( + submission.id, + :published_grading + ) + end) + + {:ok, nil} + else + {:error, {:forbidden, "Only Admin is permitted to publish all grades"}} + end + end + + @doc """ + Unpublishes grading for all submissions with grades published for an assessment and sends notifications to students. + Requires admin role. + + Returns `{:ok, nil}` on success, otherwise `{:error, {status, message}}`. + """ + + def unpublish_all(publisher = %CourseRegistration{}, assessment_id) do + if publisher.role == :admin do + submission_query = + Submission + |> join(:inner, [s], cr in CourseRegistration, on: s.student_id == cr.id) + |> where([s, cr], cr.course_id == ^publisher.course_id) + |> where([s, cr], s.is_grading_published == true) + |> where([s, cr], s.assessment_id == ^assessment_id) + |> select([s, cr], %{ + id: s.id, + student_id: cr.id + }) + + submissions = Repo.all(submission_query) + + Repo.update_all(submission_query, set: [is_grading_published: false]) + + Enum.each(submissions, fn submission -> + Notifications.handle_unpublish_grades_notifications( + assessment_id, + Repo.get(CourseRegistration, submission.student_id) + ) + end) + + {:ok, nil} + else + {:error, {:forbidden, "Only Admin is permitted to unpublish all grades"}} + end + end + @spec update_submission_status_and_xp_bonus(Submission.t()) :: {:ok, Submission.t()} | {:error, Ecto.Changeset.t()} defp update_submission_status_and_xp_bonus(submission = %Submission{}) do @@ -1508,13 +1822,24 @@ defmodule Cadet.Assessments do The input parameters are the user and query parameters. Query parameters are used to filter the submissions. - The group parameter is used to check whether only the groups under the grader - should be returned. If pageSize and offset are not provided, the default - values are 10 and 0 respectively. - - The return value is - {:ok, %{"count": count, "data": submissions}} if no errors, - else it is {:error, {:forbidden, "Forbidden."}} + The return value is `{:ok, %{"count": count, "data": submissions}}` + + # Parameters + - `pageSize`: Integer. The number of submissions to return. Default is 10. + - `offset`: Integer. The number of submissions to skip. Default is 0. + - `title`: String. Assessment title. + - `status`: String. Submission status. + - `isFullyGraded`: Boolean. Whether the submission is fully graded. + - `isGradingPublished`: Boolean. Whether the grading is published. + - `group`: Boolean. Only the groups under the grader should be returned. + - `groupName`: String. Group name. + - `name`: String. User name. + - `username`: String. User username. + - `type`: String. Assessment Config type. + - `isManuallyGraded`: Boolean. Whether the assessment is manually graded. + + # Implementation + Uses helper functions to build the filter query. Helper functions are separated by tables in the database. """ @spec submissions_by_grader_for_index(CourseRegistration.t()) :: @@ -1533,7 +1858,7 @@ defmodule Cadet.Assessments do grader = %CourseRegistration{course_id: course_id}, params \\ %{ "group" => "false", - "notFullyGraded" => "true", + "isFullyGraded" => "false", "pageSize" => "10", "offset" => "0" } @@ -1585,6 +1910,7 @@ defmodule Cadet.Assessments do student_id: s.student_id, team_id: s.team_id, assessment_id: s.assessment_id, + is_grading_published: s.is_grading_published, xp: ans.xp, xp_adjustment: ans.xp_adjustment, graded_count: ans.graded_count, @@ -1637,8 +1963,14 @@ defmodule Cadet.Assessments do {"status", value}, dynamic -> dynamic([submission], ^dynamic and submission.status == ^value) - {"notFullyGraded", "true"}, dynamic -> - dynamic([ans: ans, asst: asst], ^dynamic and asst.question_count > ans.graded_count) + {"isFullyGraded", value}, dynamic -> + dynamic( + [ans: ans, asst: asst], + ^dynamic and asst.question_count == ans.graded_count == ^value + ) + + {"isGradingPublished", value}, dynamic -> + dynamic([submission], ^dynamic and submission.is_grading_published == ^value) {_, _}, dynamic -> dynamic @@ -1828,7 +2160,7 @@ defmodule Cadet.Assessments do end end - defp is_fully_graded?(%Answer{submission_id: submission_id}) do + defp is_fully_graded?(submission_id) do submission = Submission |> Repo.get_by(id: submission_id) @@ -1849,6 +2181,27 @@ defmodule Cadet.Assessments do question_count == graded_count end + def is_fully_autograded?(submission_id) do + submission = + Submission + |> Repo.get_by(id: submission_id) + + question_count = + Question + |> where(assessment_id: ^submission.assessment_id) + |> select([q], count(q.id)) + |> Repo.one() + + graded_count = + Answer + |> where([a], submission_id: ^submission_id) + |> where([a], a.autograding_status == :success) + |> select([a], count(a.id)) + |> Repo.one() + + question_count == graded_count + end + @spec update_grading_info( %{submission_id: integer() | String.t(), question_id: integer() | String.t()}, %{}, @@ -1884,12 +2237,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 - Notifications.write_notification_when_graded(submission_id, :graded) - else - {:ok, nil} - end + {:ok, nil} else {:answer_found?, false} -> {:error, {:bad_request, "Answer not found or user not permitted to grade."}} diff --git a/lib/cadet/assessments/submission.ex b/lib/cadet/assessments/submission.ex index db3a2b296..556e81301 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(:is_grading_published, :boolean, default: false) belongs_to(:assessment, Assessment) belongs_to(:student, CourseRegistration) @@ -27,7 +28,7 @@ defmodule Cadet.Assessments.Submission do timestamps() end - @required_fields ~w(assessment_id status)a + @required_fields ~w(assessment_id status is_grading_published)a @optional_fields ~w(xp_bonus unsubmitted_by_id unsubmitted_at student_id team_id)a def changeset(submission, params) do diff --git a/lib/cadet/courses/assessment_config.ex b/lib/cadet/courses/assessment_config.ex index a9ea941ed..ca7f6841e 100644 --- a/lib/cadet/courses/assessment_config.ex +++ b/lib/cadet/courses/assessment_config.ex @@ -17,6 +17,7 @@ defmodule Cadet.Courses.AssessmentConfig do # used by frontend to determine display styles field(:early_submission_xp, :integer, default: 0) field(:hours_before_early_xp_decay, :integer, default: 0) + field(:is_grading_auto_published, :boolean, default: false) belongs_to(:course, Course) @@ -25,7 +26,7 @@ defmodule Cadet.Courses.AssessmentConfig do @required_fields ~w(course_id)a @optional_fields ~w(order type early_submission_xp - hours_before_early_xp_decay show_grading_summary is_manually_graded has_voting_features has_token_counter)a + hours_before_early_xp_decay show_grading_summary is_manually_graded has_voting_features has_token_counter is_grading_auto_published)a def changeset(assessment_config, params) do assessment_config diff --git a/lib/cadet/jobs/autograder/grading_job.ex b/lib/cadet/jobs/autograder/grading_job.ex index 89afb69ba..78e41bf2f 100644 --- a/lib/cadet/jobs/autograder/grading_job.ex +++ b/lib/cadet/jobs/autograder/grading_job.ex @@ -9,8 +9,10 @@ defmodule Cadet.Autograder.GradingJob do require Logger + alias Cadet.Assessments alias Cadet.Assessments.{Answer, Assessment, Question, Submission, SubmissionVotes} alias Cadet.Autograder.Utilities + alias Cadet.Courses.AssessmentConfig alias Cadet.Jobs.Log def close_and_make_empty_submission(assessment = %Assessment{id: id}) do @@ -258,6 +260,14 @@ defmodule Cadet.Autograder.GradingJob do ) end - defp grade_submission_question_answer_lists(_, [], [], _, _) do + defp grade_submission_question_answer_lists(submission_id, [], [], _, _) do + submission = Repo.get(Submission, submission_id) + assessment = Repo.get(Assessment, submission.assessment_id) + assessment_config = Repo.get_by(AssessmentConfig, id: assessment.config_id) + is_grading_auto_published = assessment_config.is_grading_auto_published + + if Assessments.is_fully_autograded?(submission_id) and is_grading_auto_published do + Assessments.publish_grading(submission_id) + end end end diff --git a/lib/cadet/jobs/autograder/result_store_worker.ex b/lib/cadet/jobs/autograder/result_store_worker.ex index 0851de8de..852253488 100644 --- a/lib/cadet/jobs/autograder/result_store_worker.ex +++ b/lib/cadet/jobs/autograder/result_store_worker.ex @@ -14,8 +14,9 @@ defmodule Cadet.Autograder.ResultStoreWorker do alias Ecto.Multi - alias Cadet.Repo - alias Cadet.Assessments.Answer + alias Cadet.{Assessments, Repo} + alias Cadet.Assessments.{Answer, Assessment, Submission} + alias Cadet.Courses.AssessmentConfig def perform(params = %{answer_id: answer_id, result: result}) when is_ecto_id(answer_id) do @@ -53,7 +54,11 @@ defmodule Cadet.Autograder.ResultStoreWorker do end end - defp update_answer(answer = %Answer{}, result = %{status: status}, overwrite) do + defp update_answer( + answer = %Answer{submission_id: submission_id}, + result = %{status: status}, + overwrite + ) do xp = cond do result.max_score == 0 and length(result.result) > 0 -> @@ -79,8 +84,20 @@ defmodule Cadet.Autograder.ResultStoreWorker do changes = if(overwrite, do: Map.put(changes, :xp_adjustment, 0), else: changes) - answer - |> Answer.autograding_changeset(changes) - |> Repo.update() + res = + answer + |> Answer.autograding_changeset(changes) + |> Repo.update() + + submission = Repo.get(Submission, submission_id) + assessment = Repo.get(Assessment, submission.assessment_id) + assessment_config = Repo.get_by(AssessmentConfig, id: assessment.config_id) + is_grading_auto_published = assessment_config.is_grading_auto_published + + if Assessments.is_fully_autograded?(submission_id) and is_grading_auto_published do + Assessments.publish_grading(submission_id) + end + + res end end diff --git a/lib/cadet/workers/NotificationWorker.ex b/lib/cadet/workers/NotificationWorker.ex index 67913a033..ad402a8fb 100644 --- a/lib/cadet/workers/NotificationWorker.ex +++ b/lib/cadet/workers/NotificationWorker.ex @@ -76,7 +76,7 @@ defmodule Cadet.Workers.NotificationWorker do {:ok, %{data: %{submissions: ungraded_submissions}}} = Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ "group" => "true", - "notFullyGraded" => "true" + "isFullyGraded" => "false" }) if Enum.count(ungraded_submissions) < ungraded_threshold do diff --git a/lib/cadet_web/admin_controllers/admin_grading_controller.ex b/lib/cadet_web/admin_controllers/admin_grading_controller.ex index b69c743f9..d95431362 100644 --- a/lib/cadet_web/admin_controllers/admin_grading_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_grading_controller.ex @@ -81,6 +81,64 @@ defmodule CadetWeb.AdminGradingController do end end + def unpublish_grades(conn, %{"submissionid" => submission_id}) when is_ecto_id(submission_id) do + course_reg = conn.assigns[:course_reg] + + case Assessments.unpublish_grading(submission_id, course_reg) do + {:ok, nil} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + + def publish_grades(conn, %{"submissionid" => submission_id}) when is_ecto_id(submission_id) do + course_reg = conn.assigns[:course_reg] + + case Assessments.publish_grading(submission_id, course_reg) do + {:ok, nil} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + + def publish_all_grades(conn, %{"assessmentid" => assessment_id}) + when is_ecto_id(assessment_id) do + course_reg = conn.assigns[:course_reg] + + case Assessments.publish_all_graded(course_reg, assessment_id) do + {:ok, nil} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + + def unpublish_all_grades(conn, %{"assessmentid" => assessment_id}) + when is_ecto_id(assessment_id) do + course_reg = conn.assigns[:course_reg] + + case Assessments.unpublish_all(course_reg, assessment_id) do + {:ok, nil} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + def autograde_submission(conn, %{"submissionid" => submission_id}) do course_reg = conn.assigns[:course_reg] @@ -276,6 +334,8 @@ defmodule CadetWeb.AdminGradingController do unsubmittedBy(Schema.ref(:GraderInfo)) unsubmittedAt(:string, "Last unsubmitted at", format: "date-time", required: false) + + isGradingPublished(:boolean, "Whether the grading is published", required: true) end end, AssessmentInfo: diff --git a/lib/cadet_web/admin_views/admin_courses_view.ex b/lib/cadet_web/admin_views/admin_courses_view.ex index 2804eb664..ba5ec6820 100644 --- a/lib/cadet_web/admin_views/admin_courses_view.ex +++ b/lib/cadet_web/admin_views/admin_courses_view.ex @@ -14,7 +14,8 @@ defmodule CadetWeb.AdminCoursesView do earlySubmissionXp: :early_submission_xp, hasVotingFeatures: :has_voting_features, hasTokenCounter: :has_token_counter, - hoursBeforeEarlyXpDecay: :hours_before_early_xp_decay + hoursBeforeEarlyXpDecay: :hours_before_early_xp_decay, + isGradingAutoPublished: :is_grading_auto_published }) end end diff --git a/lib/cadet_web/admin_views/admin_grading_view.ex b/lib/cadet_web/admin_views/admin_grading_view.ex index 3f26e41d1..313a27f05 100644 --- a/lib/cadet_web/admin_views/admin_grading_view.ex +++ b/lib/cadet_web/admin_views/admin_grading_view.ex @@ -85,6 +85,7 @@ defmodule CadetWeb.AdminGradingView do xp: :xp, xpAdjustment: :xp_adjustment, xpBonus: :xp_bonus, + isGradingPublished: :is_grading_published, gradedCount: &case &1.graded_count do nil -> 0 diff --git a/lib/cadet_web/controllers/assessments_controller.ex b/lib/cadet_web/controllers/assessments_controller.ex index ec64f68c9..bb9a563f2 100644 --- a/lib/cadet_web/controllers/assessments_controller.ex +++ b/lib/cadet_web/controllers/assessments_controller.ex @@ -40,7 +40,7 @@ defmodule CadetWeb.AssessmentsController do def index(conn, _) do cr = conn.assigns.course_reg {:ok, assessments} = Assessments.all_assessments(cr) - + assessments = Assessments.format_all_assessments(assessments) render(conn, "index.json", assessments: assessments) end @@ -48,8 +48,12 @@ defmodule CadetWeb.AssessmentsController do cr = conn.assigns.course_reg case Assessments.assessment_with_questions_and_answers(assessment_id, cr) do - {:ok, assessment} -> render(conn, "show.json", assessment: assessment) - {:error, {status, message}} -> send_resp(conn, status, message) + {:ok, assessment} -> + assessment = Assessments.format_assessment_with_questions_and_answers(assessment) + render(conn, "show.json", assessment: assessment) + + {:error, {status, message}} -> + send_resp(conn, status, message) end end diff --git a/lib/cadet_web/controllers/notifications_controller.ex b/lib/cadet_web/controllers/notifications_controller.ex index bdeb5c6e8..19c436cd9 100644 --- a/lib/cadet_web/controllers/notifications_controller.ex +++ b/lib/cadet_web/controllers/notifications_controller.ex @@ -100,7 +100,15 @@ defmodule CadetWeb.NotificationsController do NotificationType: swagger_schema do type(:string) - enum([:new, :deadline, :autograded, :graded, :submitted, :unsubmitted, :new_message]) + + enum([ + :new, + :submitted, + :unsubmitted, + :unpublished_grading, + :published_grading, + :new_message + ]) end } end diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index 896410583..b54bb9f69 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -132,8 +132,19 @@ defmodule CadetWeb.Router do get("/grading", AdminGradingController, :index) get("/grading/summary", AdminGradingController, :grading_summary) + + post("/grading/:assessmentid/publish_all_grades", AdminGradingController, :publish_all_grades) + + post( + "/grading/:assessmentid/unpublish_all_grades", + AdminGradingController, + :unpublish_all_grades + ) + get("/grading/:submissionid", AdminGradingController, :show) post("/grading/:submissionid/unsubmit", AdminGradingController, :unsubmit) + post("/grading/:submissionid/unpublish_grades", AdminGradingController, :unpublish_grades) + post("/grading/:submissionid/publish_grades", AdminGradingController, :publish_grades) post("/grading/:submissionid/autograde", AdminGradingController, :autograde_submission) post("/grading/:submissionid/:questionid", AdminGradingController, :update) diff --git a/lib/cadet_web/views/assessments_view.ex b/lib/cadet_web/views/assessments_view.ex index d7da3ad5d..0b967397b 100644 --- a/lib/cadet_web/views/assessments_view.ex +++ b/lib/cadet_web/views/assessments_view.ex @@ -32,6 +32,7 @@ defmodule CadetWeb.AssessmentsView do isPublished: :is_published, questionCount: :question_count, gradedCount: &(&1.graded_count || 0), + isGradingPublished: :is_grading_published, earlySubmissionXp: & &1.config.early_submission_xp, maxTeamSize: :max_team_size, hasVotingFeatures: :has_voting_features, diff --git a/priv/repo/migrations/20240322122108_add_is_grading_published.exs b/priv/repo/migrations/20240322122108_add_is_grading_published.exs new file mode 100644 index 000000000..473a71f37 --- /dev/null +++ b/priv/repo/migrations/20240322122108_add_is_grading_published.exs @@ -0,0 +1,9 @@ +defmodule Cadet.Repo.Migrations.AddIsGradingPublished do + use Ecto.Migration + + def change do + alter table(:submissions) do + add(:is_grading_published, :boolean, null: false, default: false) + end + end +end diff --git a/priv/repo/migrations/20240322184853_update_is_grading_published.exs b/priv/repo/migrations/20240322184853_update_is_grading_published.exs new file mode 100644 index 000000000..28c43392a --- /dev/null +++ b/priv/repo/migrations/20240322184853_update_is_grading_published.exs @@ -0,0 +1,24 @@ +defmodule MyApp.Repo.Migrations.UpdateIsGradingPublished do + use Ecto.Migration + + def up do + execute(""" + UPDATE submissions + SET is_grading_published = true + WHERE id IN ( + SELECT s.id + FROM submissions AS s + JOIN answers AS a ON a.submission_id = s.id + GROUP BY s.id + HAVING COUNT(a.id) = COUNT(a.grader_id) + ) + """) + end + + def down do + execute(""" + UPDATE submissions + SET is_grading_published = false + """) + end +end diff --git a/priv/repo/migrations/20240331222300_add_is_grading_auto_published.exs b/priv/repo/migrations/20240331222300_add_is_grading_auto_published.exs new file mode 100644 index 000000000..4c44c2101 --- /dev/null +++ b/priv/repo/migrations/20240331222300_add_is_grading_auto_published.exs @@ -0,0 +1,15 @@ +defmodule Cadet.Repo.Migrations.AddIsGradingAutoPublished do + use Ecto.Migration + + def up do + alter table(:assessment_configs) do + add(:is_grading_auto_published, :boolean, null: false, default: false) + end + end + + def down do + alter table(:assessment_configs) do + remove(:is_grading_auto_published) + end + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index c5661ee65..37be60527 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -80,8 +80,8 @@ if Cadet.Env.env() == :dev do avenger_group = insert(:group, %{name: "MockAvengerGroup", leader: avenger_cr}) groups_and_courses = [{admin_group, admin_course}, {avenger_group, avenger_course}] - # Users + # Users Enum.each(groups_and_courses, fn {group, course} -> students = for i <- 1..number_of_students do @@ -99,11 +99,23 @@ if Cadet.Env.env() == :dev do end # Assessments and Submissions - valid_assessment_types = [{1, "Missions"}, {2, "Paths"}, {3, "Quests"}] + # {order, type, is_grading_auto_published, is_manually_graded} + valid_assessment_types = [ + {1, "Missions", false, true}, + {2, "Paths", true, false}, + {3, "Quests", false, true} + ] assessment_configs = - Enum.map(valid_assessment_types, fn {order, type} -> - insert(:assessment_config, %{type: type, order: order, course: course}) + Enum.map(valid_assessment_types, fn {order, type, is_grading_auto_published, + is_manually_graded} -> + insert(:assessment_config, %{ + type: type, + order: order, + course: course, + is_grading_auto_published: is_grading_auto_published, + is_manually_graded: is_manually_graded + }) end) for i <- 1..number_of_assessments do diff --git a/test/cadet/accounts/notification_test.exs b/test/cadet/accounts/notification_test.exs index 7d0ece257..ec206e0e1 100644 --- a/test/cadet/accounts/notification_test.exs +++ b/test/cadet/accounts/notification_test.exs @@ -320,28 +320,31 @@ defmodule Cadet.Accounts.NotificationTest do student: student, individual_submission: individual_submission } do - Notifications.write_notification_when_graded(individual_submission.id, :autograded) + Notifications.write_notification_when_published( + individual_submission.id, + :published_grading + ) notification = Repo.get_by(Notification, course_reg_id: student.id, - type: :autograded, + type: :published_grading, assessment_id: assessment.id ) - assert %{type: :autograded} = notification + assert %{type: :published_grading} = notification end test "no notification when no submission", %{ assessment: assessment, student: student } do - Notifications.write_notification_when_graded(-1, :autograded) + Notifications.write_notification_when_published(-1, :published_grading) notification = Repo.get_by(Notification, course_reg_id: student.id, - type: :autograded, + type: :published_grading, assessment_id: assessment.id ) @@ -353,7 +356,7 @@ defmodule Cadet.Accounts.NotificationTest do team: team, team_submission: team_submission } do - Notifications.write_notification_when_graded(team_submission.id, :autograded) + Notifications.write_notification_when_published(team_submission.id, :published_grading) team_members = Repo.all(from(tm in TeamMember, where: tm.team_id == ^team.id, preload: :student)) @@ -364,11 +367,11 @@ defmodule Cadet.Accounts.NotificationTest do notification = Repo.get_by(Notification, course_reg_id: student.id, - type: :autograded, + type: :published_grading, assessment_id: assessment.id ) - assert %{type: :autograded} = notification + assert %{type: :published_grading} = notification end) end @@ -377,16 +380,19 @@ defmodule Cadet.Accounts.NotificationTest do student: student, individual_submission: individual_submission } do - Notifications.write_notification_when_graded(individual_submission.id, :graded) + Notifications.write_notification_when_published( + individual_submission.id, + :published_grading + ) notification = Repo.get_by(Notification, course_reg_id: student.id, - type: :graded, + type: :published_grading, assessment_id: assessment.id ) - assert %{type: :graded} = notification + assert %{type: :published_grading} = notification end test "receives notification when maunally graded [team submission]", %{ @@ -394,7 +400,7 @@ defmodule Cadet.Accounts.NotificationTest do team: team, team_submission: team_submission } do - Notifications.write_notification_when_graded(team_submission.id, :graded) + Notifications.write_notification_when_published(team_submission.id, :published_grading) team_members = Repo.all(from(tm in TeamMember, where: tm.team_id == ^team.id, preload: :student)) @@ -405,11 +411,11 @@ defmodule Cadet.Accounts.NotificationTest do notification = Repo.get_by(Notification, course_reg_id: student.id, - type: :graded, + type: :published_grading, assessment_id: assessment.id ) - assert %{type: :graded} = notification + assert %{type: :published_grading} = notification end) end diff --git a/test/cadet/assessments/assessments_test.exs b/test/cadet/assessments/assessments_test.exs index 96a98cbc1..a95b7382a 100644 --- a/test/cadet/assessments/assessments_test.exs +++ b/test/cadet/assessments/assessments_test.exs @@ -1305,7 +1305,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1373,7 +1374,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1442,7 +1444,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1529,7 +1532,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1640,7 +1644,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1676,7 +1681,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1724,7 +1730,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1785,7 +1792,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1853,7 +1861,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -1921,7 +1930,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -2008,7 +2018,8 @@ defmodule Cadet.AssessmentsTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -2061,8 +2072,131 @@ defmodule Cadet.AssessmentsTest do end end - # Tests assume each config has only 1 assessment - describe "get submission function" do + describe "grading published feature" do + setup do + course = insert(:course) + config = insert(:assessment_config, %{type: "Test", course: course}) + student = insert(:course_registration, course: course, role: :student) + student_2 = insert(:course_registration, course: course, role: :student) + avenger = insert(:course_registration, course: course, role: :staff) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -1), + close_at: Timex.shift(Timex.now(), hours: 500), + is_published: true, + config: config, + course: course + ) + + question = insert(:mcq_question, assessment: assessment) + + submission = + insert(:submission, + assessment: assessment, + student: student, + status: :attempted, + is_grading_published: false + ) + + _answer = + insert( + :answer, + submission: submission, + question: question, + answer: %{:choice_id => 1}, + xp: 400, + xp_adjustment: 300, + comments: "Dummy Comment", + autograding_status: :failed, + autograding_results: [ + %{ + errors: [ + %{ + error_message: "DummyError", + error_type: "systemError" + } + ], + result_type: "error" + } + ], + grader: avenger + ) + + published_submission = + insert(:submission, + assessment: assessment, + student: student_2, + status: :submitted, + is_grading_published: true + ) + + _published_answer = + insert( + :answer, + submission: published_submission, + question: question, + answer: %{:choice_id => 1}, + xp: 400, + xp_adjustment: 300, + comments: "Dummy Comment", + autograding_status: :failed, + autograding_results: [ + %{ + errors: [ + %{ + error_message: "DummyError", + error_type: "systemError" + } + ], + result_type: "error" + } + ], + grader: avenger + ) + + %{assessment: assessment, student: student, student_2: student_2} + end + + test "unpublished grades are hidden", %{assessment: assessment, student: student} do + {_, assessment_with_q_and_a} = + Assessments.assessment_with_questions_and_answers(assessment, student) + + formatted_assessment = + Assessments.format_assessment_with_questions_and_answers(assessment_with_q_and_a) + + formatted_answer = hd(formatted_assessment.questions).answer + + assert formatted_answer.xp == 0 + assert formatted_answer.xp_adjustment == 0 + assert formatted_answer.autograding_status == :none + assert formatted_answer.autograding_results == [] + assert formatted_answer.grader == nil + assert formatted_answer.grader_id == nil + assert formatted_answer.comments == nil + end + + test "published grades are shown", %{assessment: assessment, student_2: student} do + {_, assessment_with_q_and_a} = + Assessments.assessment_with_questions_and_answers(assessment, student) + + formatted_assessment = + Assessments.format_assessment_with_questions_and_answers(assessment_with_q_and_a) + + formatted_answer = hd(formatted_assessment.questions).answer + + assert formatted_answer.xp != 0 + assert formatted_answer.xp_adjustment != 0 + assert formatted_answer.autograding_status != :none + assert formatted_answer.autograding_results != [] + assert formatted_answer.grader != nil + assert formatted_answer.grader_id != nil + assert formatted_answer.comments != nil + end + end + + describe "submissions_by_grader_for_index function" do setup do seed = Cadet.Test.Seeds.assessments() @@ -2172,17 +2306,44 @@ defmodule Cadet.AssessmentsTest do end) end + test "filter by submission fully graded", %{ + course_regs: %{avenger1_cr: avenger}, + assessments: assessments, + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info + } do + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, is_graded, _, _} -> is_graded end) + + {_, res} = + Assessments.submissions_by_grader_for_index(avenger, %{ + "isFullyGraded" => "true", + "pageSize" => total_submissions + }) + + submissions_from_res = res[:data][:submissions] + + assert length(submissions_from_res) == expected_length + + Enum.each(submissions_from_res, fn s -> + assert s.question_count == s.graded_count + end) + end + test "filter by submission not fully graded", %{ - course_regs: %{avenger1_cr: avenger, students: students}, + course_regs: %{avenger1_cr: avenger}, assessments: assessments, - total_submissions: total_submissions + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info } do - # All but one is fully graded - expected_length = length(Map.keys(assessments)) * (length(students) - 1) + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, is_graded, _, _} -> !is_graded end) {_, res} = Assessments.submissions_by_grader_for_index(avenger, %{ - "notFullyGraded" => "true", + "isFullyGraded" => "false", "pageSize" => total_submissions }) @@ -2195,13 +2356,71 @@ defmodule Cadet.AssessmentsTest do end) end + test "filter by submission published", %{ + course_regs: %{avenger1_cr: avenger}, + assessments: assessments, + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info + } do + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, _, is_published, _} -> + is_published + end) + + {_, res} = + Assessments.submissions_by_grader_for_index(avenger, %{ + "isGradingPublished" => "true", + "pageSize" => total_submissions + }) + + submissions_from_res = res[:data][:submissions] + + assert length(submissions_from_res) == expected_length + + Enum.each(submissions_from_res, fn s -> + assert s.is_grading_published == true + end) + end + + test "filter by submission not published", %{ + course_regs: %{avenger1_cr: avenger}, + assessments: assessments, + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info + } do + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, _, is_published, _} -> + !is_published + end) + + {_, res} = + Assessments.submissions_by_grader_for_index(avenger, %{ + "isGradingPublished" => "false", + "pageSize" => total_submissions + }) + + submissions_from_res = res[:data][:submissions] + + assert length(submissions_from_res) == expected_length + + Enum.each(submissions_from_res, fn s -> + assert s.is_grading_published == false + end) + end + test "filter by group avenger", %{ course_regs: %{avenger1_cr: avenger, group: group, students: students}, assessments: assessments, - total_submissions: total_submissions + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info } do - # All but one is in the same group - expected_length = length(Map.keys(assessments)) * (length(students) - 1) + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, _, _, avenger_cr} -> + avenger_cr.id == avenger.id + end) {_, res} = Assessments.submissions_by_grader_for_index(avenger, %{ @@ -2222,10 +2441,14 @@ defmodule Cadet.AssessmentsTest do test "filter by group avenger2", %{ course_regs: %{avenger2_cr: avenger2, group2: group2, students: students}, assessments: assessments, - total_submissions: total_submissions + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info } do - # One in the same group - expected_length = length(Map.keys(assessments)) + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {_, _, _, _, avenger_cr} -> + avenger_cr.id == avenger2.id + end) {_, res} = Assessments.submissions_by_grader_for_index(avenger2, %{ @@ -2247,10 +2470,15 @@ defmodule Cadet.AssessmentsTest do test "filter by group name group", %{ course_regs: %{avenger2_cr: avenger2, group: group, students: students}, assessments: assessments, - total_submissions: total_submissions + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info } do - # All but one is in group - expected_length = length(Map.keys(assessments)) * (length(students) - 1) + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {student, _, _, _, _} -> + student.group.id == group.id + end) + group_name = group.name {_, res} = @@ -2273,10 +2501,15 @@ defmodule Cadet.AssessmentsTest do test "filter by group name group2", %{ course_regs: %{avenger1_cr: avenger, group2: group2, students: students}, assessments: assessments, - total_submissions: total_submissions + total_submissions: total_submissions, + students_with_assessment_info: students_with_assessment_info } do - # One in the group - expected_length = length(Map.keys(assessments)) + expected_length = + length(Map.keys(assessments)) * + Enum.count(students_with_assessment_info, fn {student, _, _, _, _} -> + student.group.id == group2.id + end) + group_name = group2.name {_, res} = @@ -2441,7 +2674,6 @@ defmodule Cadet.AssessmentsTest do test "filter by assessment config 1", %{ course_regs: %{avenger1_cr: avenger, students: students}, - assessments: assessments, assessment_configs: assessment_configs, total_submissions: total_submissions } do @@ -2470,7 +2702,6 @@ defmodule Cadet.AssessmentsTest do test "filter by assessment config 2", %{ course_regs: %{avenger1_cr: avenger, students: students}, - assessments: assessments, assessment_configs: assessment_configs, total_submissions: total_submissions } do @@ -2500,7 +2731,6 @@ defmodule Cadet.AssessmentsTest do test "filter by assessment config 3", %{ course_regs: %{avenger1_cr: avenger, students: students}, - assessments: assessments, assessment_configs: assessment_configs, total_submissions: total_submissions } do @@ -2530,7 +2760,6 @@ defmodule Cadet.AssessmentsTest do test "filter by assessment config manually graded", %{ course_regs: %{avenger1_cr: avenger, students: students}, - assessments: assessments, assessment_configs: assessment_configs, total_submissions: total_submissions } do @@ -2560,7 +2789,6 @@ defmodule Cadet.AssessmentsTest do test "filter by assessment config not manually graded", %{ course_regs: %{avenger1_cr: avenger, students: students}, - assessments: assessments, assessment_configs: assessment_configs, total_submissions: total_submissions } do @@ -2589,6 +2817,116 @@ defmodule Cadet.AssessmentsTest do end end + describe "is_fully_autograded? function" do + setup do + assessment = insert(:assessment) + student = insert(:course_registration, role: :student) + question = insert(:mcq_question, assessment: assessment) + question2 = insert(:mcq_question, assessment: assessment) + + submission = + insert(:submission, assessment: assessment, student: student, status: :submitted) + + %{question: question, question2: question2, submission: submission} + end + + test "returns true when all answers are autograded successfully", %{ + question: question, + question2: question2, + submission: submission + } do + insert(:answer, submission: submission, question: question, autograding_status: :success) + insert(:answer, submission: submission, question: question2, autograding_status: :success) + + assert Assessments.is_fully_autograded?(submission.id) == true + end + + test "returns false when not all answers are autograded successfully", %{ + question: question, + question2: question2, + submission: submission + } do + insert(:answer, submission: submission, question: question, autograding_status: :success) + insert(:answer, submission: submission, question: question2, autograding_status: :failed) + + assert Assessments.is_fully_autograded?(submission.id) == false + end + + test "returns false when not all answers are autograded successfully 2", %{ + question: question, + question2: question2, + submission: submission + } do + insert(:answer, submission: submission, question: question, autograding_status: :success) + insert(:answer, submission: submission, question: question2, autograding_status: :none) + + assert Assessments.is_fully_autograded?(submission.id) == false + end + end + + describe "publish and unpublish all grading" do + setup do + Cadet.Test.Seeds.assessments() + end + + test "publish all graded submissions for an assessment", + %{ + role_crs: %{admin: admin}, + assessments: assessments, + students_with_assessment_info: students + } do + assessment_id = assessments["mission"][:assessment].id + + # 1 student has all assessments published + expected_length = + Enum.count(students, fn {_, _, is_graded, is_grading_published, _} -> + is_graded and not is_grading_published + end) + length(Map.keys(assessments)) + + Assessments.publish_all_graded(admin, assessment_id) + + published_submissions = + Submission + |> where([s], s.is_grading_published == true) + |> select([s], %{count: s.id |> count()}) + |> Repo.one() + + number_of_published_submissions = published_submissions.count + assert number_of_published_submissions == expected_length + end + + test "unpublish all submissions for an assessment", + %{ + role_crs: %{admin: admin}, + assessments: assessments, + students_with_assessment_info: students + } do + assessment_id = assessments["mission"][:assessment].id + + published_submissions_before = + Submission + |> where([s], s.is_grading_published == true) + |> select([s], %{count: s.id |> count()}) + |> Repo.one() + + expected_unpublished_length = + Enum.count(students, fn {_, _, _, is_grading_published, _} -> + is_grading_published + end) + + Assessments.unpublish_all(admin, assessment_id) + + published_submissions_after = + Submission + |> where([s], s.is_grading_published == true) + |> select([s], %{count: s.id |> count()}) + |> Repo.one() + + assert published_submissions_after.count + expected_unpublished_length == + published_submissions_before.count + end + end + defp get_answer_relative_scores(answers) do answers |> Enum.map(fn ans -> ans.relative_score end) end diff --git a/test/cadet/jobs/autograder/grading_job_test.exs b/test/cadet/jobs/autograder/grading_job_test.exs index eaac9903e..44b333553 100644 --- a/test/cadet/jobs/autograder/grading_job_test.exs +++ b/test/cadet/jobs/autograder/grading_job_test.exs @@ -411,10 +411,12 @@ defmodule Cadet.Autograder.GradingJobTest do end end - describe "#grade_all_due_yesterday, all mcq questions" do + describe "#grade_all_due_yesterday, all mcq questions, grading set to auto publish" do setup do course = insert(:course) - assessment_config = insert(:assessment_config, %{course: course}) + + assessment_config = + insert(:assessment_config, %{course: course, is_grading_auto_published: true}) assessments = insert_list(3, :assessment, %{ @@ -441,9 +443,10 @@ defmodule Cadet.Autograder.GradingJobTest do } do student = insert(:course_registration, %{course: course, role: :student}) - for {assessment, _} <- assessments do - insert(:submission, %{student: student, assessment: assessment, status: :attempting}) - end + submissions = + Enum.map(assessments, fn {assessment, _} -> + insert(:submission, %{student: student, assessment: assessment, status: :attempting}) + end) GradingJob.grade_all_due_yesterday() @@ -456,6 +459,12 @@ defmodule Cadet.Autograder.GradingJobTest do assert Enum.count(answers) == 9 + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == true + end) + for answer <- answers do assert answer.xp == 0 assert answer.autograding_status == :success @@ -495,10 +504,128 @@ defmodule Cadet.Autograder.GradingJobTest do questions = Enum.flat_map(assessments, fn {_, questions} -> questions end) answers = Enum.flat_map(submissions_answers, fn {_, answers} -> answers end) - for submission <- submissions do - assert Repo.get(Submission, submission.id).status == :submitted + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == true + end) + + for {answer, question} <- Enum.zip(answers, questions) do + answer_db = Repo.get(Answer, answer.id) + + # seeded questions have correct choice as 0 + if answer_db.answer["choice_id"] == 0 do + assert answer_db.xp == question.max_xp + else + assert answer_db.xp == 0 + end + + assert answer_db.autograding_status == :success + end + + assert Enum.empty?(JobsQueue.all()) + end + end + + describe "#grade_all_due_yesterday, all mcq questions, grading set to not auto publish" do + setup do + course = insert(:course) + + assessment_config = + insert(:assessment_config, %{course: course, is_grading_auto_published: false}) + + assessments = + insert_list(3, :assessment, %{ + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -4), + config: assessment_config, + course: course + }) + + questions = + for assessment <- assessments do + insert_list(3, :mcq_question, %{max_xp: 200, assessment: assessment}) + end + + %{course: course, assessments: Enum.zip(assessments, questions)} + end + + test "all assessments attempted, all questions unanswered, " <> + "should insert empty answers, should not enqueue any", + %{ + course: course, + assessments: assessments + } do + student = insert(:course_registration, %{course: course, role: :student}) + + submissions = + Enum.map(assessments, fn {assessment, _} -> + insert(:submission, %{student: student, assessment: assessment, status: :attempting}) + end) + + GradingJob.grade_all_due_yesterday() + + answers = + Submission + |> where(student_id: ^student.id) + |> join(:inner, [s], a in assoc(s, :answers)) + |> select([_, a], a) + |> Repo.all() + + assert Enum.count(answers) == 9 + + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == false + end) + + for answer <- answers do + assert answer.xp == 0 + assert answer.autograding_status == :success + assert answer.answer == %{"choice_id" => 0} end + assert Enum.empty?(JobsQueue.all()) + end + + test "all assessments attempted, all questions answered, " <> + "should grade all questions, should not enqueue any", + %{ + course: course, + assessments: assessments + } do + student = insert(:course_registration, %{course: course, role: :student}) + + submissions_answers = + Enum.map(assessments, fn {assessment, questions} -> + submission = + insert(:submission, %{student: student, assessment: assessment, status: :attempted}) + + answers = + Enum.map(questions, fn question -> + insert(:answer, %{ + question: question, + submission: submission, + answer: build(:mcq_answer) + }) + end) + + {submission, answers} + end) + + GradingJob.grade_all_due_yesterday() + submissions = Enum.map(submissions_answers, fn {submission, _} -> submission end) + questions = Enum.flat_map(assessments, fn {_, questions} -> questions end) + answers = Enum.flat_map(submissions_answers, fn {_, answers} -> answers end) + + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == false + end) + for {answer, question} <- Enum.zip(answers, questions) do answer_db = Repo.get(Answer, answer.id) @@ -516,10 +643,12 @@ defmodule Cadet.Autograder.GradingJobTest do end end - describe "#grade_all_due_yesterday, all voting questions" do + describe "#grade_all_due_yesterday, all voting questions, grading set to auto publish" do setup do course = insert(:course) - assessment_config = insert(:assessment_config, %{course: course}) + + assessment_config = + insert(:assessment_config, %{course: course, is_grading_auto_published: true}) assessments = insert_list(3, :assessment, %{ @@ -546,9 +675,10 @@ defmodule Cadet.Autograder.GradingJobTest do } do student = insert(:course_registration, %{course: course, role: :student}) - for {assessment, _} <- assessments do - insert(:submission, %{student: student, assessment: assessment, status: :attempting}) - end + submissions = + Enum.map(assessments, fn {assessment, _} -> + insert(:submission, %{student: student, assessment: assessment, status: :attempting}) + end) GradingJob.grade_all_due_yesterday() @@ -561,6 +691,12 @@ defmodule Cadet.Autograder.GradingJobTest do assert Enum.count(answers) == 9 + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == true + end) + for answer <- answers do assert answer.xp == 0 assert answer.autograding_status == :success @@ -605,10 +741,139 @@ defmodule Cadet.Autograder.GradingJobTest do submissions = Enum.map(submissions_answers, fn {submission, _} -> submission end) answers = Enum.flat_map(submissions_answers, fn {_, answers} -> answers end) - for submission <- submissions do - assert Repo.get(Submission, submission.id).status == :submitted + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == true + end) + + for {question, answer} <- Enum.zip(questions, answers) do + is_nil_entries = + SubmissionVotes + |> where(voter_id: ^student.id) + |> where(question_id: ^question.id) + |> where([sv], is_nil(sv.score)) + |> Repo.exists?() + + answer_db = Repo.get(Answer, answer.id) + + if is_nil_entries do + assert answer_db.xp == 0 + else + assert answer_db.xp == question.max_xp + end + + assert answer_db.autograding_status == :success end + assert Enum.empty?(JobsQueue.all()) + end + end + + describe "#grade_all_due_yesterday, all voting questions, grading set to not auto publish" do + setup do + course = insert(:course) + + assessment_config = + insert(:assessment_config, %{course: course, is_grading_auto_published: false}) + + assessments = + insert_list(3, :assessment, %{ + is_published: true, + open_at: Timex.shift(Timex.now(), days: -5), + close_at: Timex.shift(Timex.now(), hours: -4), + config: assessment_config, + course: course + }) + + questions = + for assessment <- assessments do + insert_list(3, :voting_question, %{max_xp: 20, assessment: assessment}) + end + + %{course: course, assessments: Enum.zip(assessments, questions)} + end + + test "all assessments attempted, all questions unanswered, " <> + "should insert empty answers, should not enqueue any", + %{ + course: course, + assessments: assessments + } do + student = insert(:course_registration, %{course: course, role: :student}) + + submissions = + Enum.map(assessments, fn {assessment, _} -> + insert(:submission, %{student: student, assessment: assessment, status: :attempting}) + end) + + GradingJob.grade_all_due_yesterday() + + answers = + Submission + |> where(student_id: ^student.id) + |> join(:inner, [s], a in assoc(s, :answers)) + |> select([_, a], a) + |> Repo.all() + + assert Enum.count(answers) == 9 + + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == false + end) + + for answer <- answers do + assert answer.xp == 0 + assert answer.autograding_status == :success + assert answer.answer == %{"completed" => false} + end + + assert Enum.empty?(JobsQueue.all()) + end + + test "all assessments attempted, all questions aswered, " <> + "should grade all questions, should not enqueue any", + %{ + course: course, + assessments: assessments + } do + student = insert(:course_registration, %{course: course, role: :student}) + + submissions_answers = + for {assessment, questions} <- assessments do + submission = + insert(:submission, %{student: student, assessment: assessment, status: :attempted}) + + answers = + for question <- questions do + case Enum.random(0..1) do + 0 -> insert(:submission_vote, %{voter: student, question: question, score: 1}) + 1 -> insert(:submission_vote, %{voter: student, question: question}) + end + + insert(:answer, %{ + question: question, + submission: submission, + answer: build(:voting_answer) + }) + end + + {submission, answers} + end + + GradingJob.grade_all_due_yesterday() + questions = Enum.flat_map(assessments, fn {_, questions} -> questions end) + submissions = Enum.map(submissions_answers, fn {submission, _} -> submission end) + answers = Enum.flat_map(submissions_answers, fn {_, answers} -> answers end) + + Enum.each(submissions, fn submission -> + submission = Repo.get(Submission, submission.id) + assert submission.status == :submitted + assert submission.is_grading_published == false + end) + for {question, answer} <- Enum.zip(questions, answers) do is_nil_entries = SubmissionVotes diff --git a/test/cadet/jobs/autograder/result_store_worker_test.exs b/test/cadet/jobs/autograder/result_store_worker_test.exs index a907b809d..57961ebef 100644 --- a/test/cadet/jobs/autograder/result_store_worker_test.exs +++ b/test/cadet/jobs/autograder/result_store_worker_test.exs @@ -3,11 +3,15 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do import ExUnit.CaptureLog - alias Cadet.Assessments.Answer + alias Cadet.Assessments.{Answer, Submission} alias Cadet.Autograder.ResultStoreWorker setup do - answer = insert(:answer, %{question: insert(:question), submission: insert(:submission)}) + assessment_config = insert(:assessment_config, %{is_grading_auto_published: true}) + assessment = insert(:assessment, %{config: assessment_config}) + question = insert(:question, %{assessment: assessment}) + submission = insert(:submission, %{status: :submitted, assessment: assessment}) + answer = insert(:answer, %{question: question, submission: submission}) success_no_errors = %{status: :success, score: 10, max_score: 10, result: []} success_with_errors = %{ @@ -68,7 +72,7 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do end describe "#perform, valid answer_id" do - test "before manual grading", %{answer: answer, results: results} do + test "before manual grading and grading auto published", %{answer: answer, results: results} do for result <- results do ResultStoreWorker.perform(%{answer_id: answer.id, result: result}) @@ -95,18 +99,27 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do ) end + submission = Repo.get(Submission, answer.submission_id) + + if result.status == :success do + assert submission.is_grading_published == true + else + assert submission.is_grading_published == false + end + assert answer.autograding_status == result.status assert answer.autograding_results == errors_string_keys end end - test "after manual grading", %{results: results} do + test "after manual grading and grading not auto published", %{results: results} do grader = insert(:course_registration, %{role: :staff}) + # Question uses default assessment config (is_grading_auto_published: false) answer = insert(:answer, %{ question: insert(:question), - submission: insert(:submission), + submission: insert(:submission, %{status: :submitted}), grader_id: grader.id }) @@ -136,6 +149,10 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do ) end + submission = Repo.get(Submission, answer.submission_id) + + assert submission.is_grading_published == false + assert answer.autograding_status == result.status assert answer.autograding_results == errors_string_keys end diff --git a/test/cadet/jobs/notification_worker/notification_worker_test.exs b/test/cadet/jobs/notification_worker/notification_worker_test.exs index f29a6e1c4..0092a10d6 100644 --- a/test/cadet/jobs/notification_worker/notification_worker_test.exs +++ b/test/cadet/jobs/notification_worker/notification_worker_test.exs @@ -21,7 +21,7 @@ defmodule Cadet.NotificationWorker.NotificationWorkerTest do {:ok, %{data: %{submissions: ungraded_submissions}}} = Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{ "group" => "true", - "notFullyGraded" => "true" + "isFullyGraded" => "false" }) Repo.update_all(NotificationType, set: [is_enabled: true]) diff --git a/test/cadet_web/admin_controllers/admin_courses_controller_test.exs b/test/cadet_web/admin_controllers/admin_courses_controller_test.exs index c9a3096c2..cb4c1e30f 100644 --- a/test/cadet_web/admin_controllers/admin_courses_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_courses_controller_test.exs @@ -176,7 +176,8 @@ defmodule CadetWeb.AdminCoursesControllerTest do "type" => "Mission1", "assessmentConfigId" => config1.id, "hasVotingFeatures" => false, - "hasTokenCounter" => false + "hasTokenCounter" => false, + "isGradingAutoPublished" => false }, %{ "earlySubmissionXp" => 200, @@ -186,7 +187,8 @@ defmodule CadetWeb.AdminCoursesControllerTest do "type" => "Mission2", "assessmentConfigId" => config2.id, "hasVotingFeatures" => true, - "hasTokenCounter" => true + "hasTokenCounter" => true, + "isGradingAutoPublished" => false }, %{ "earlySubmissionXp" => 200, @@ -196,7 +198,8 @@ defmodule CadetWeb.AdminCoursesControllerTest do "type" => "Mission3", "assessmentConfigId" => config3.id, "hasVotingFeatures" => false, - "hasTokenCounter" => false + "hasTokenCounter" => false, + "isGradingAutoPublished" => false } ] diff --git a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs index c427a6478..11d8df0f0 100644 --- a/test/cadet_web/admin_controllers/admin_grading_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_grading_controller_test.exs @@ -142,7 +142,8 @@ defmodule CadetWeb.AdminGradingControllerTest do "gradedCount" => 5, "unsubmittedBy" => nil, "unsubmittedAt" => nil, - "team" => nil + "team" => nil, + "isGradingPublished" => submission.is_grading_published } end) } @@ -216,7 +217,8 @@ defmodule CadetWeb.AdminGradingControllerTest do "gradedCount" => 5, "unsubmittedBy" => nil, "unsubmittedAt" => nil, - "team" => nil + "team" => nil, + "isGradingPublished" => submission.is_grading_published } end) } @@ -540,7 +542,12 @@ defmodule CadetWeb.AdminGradingControllerTest do student = List.first(students) submission = - insert(:submission, assessment: assessment, student: student, status: :submitted) + insert(:submission, + assessment: assessment, + student: student, + status: :submitted, + is_grading_published: false + ) answer = insert( @@ -606,6 +613,47 @@ defmodule CadetWeb.AdminGradingControllerTest do assert response(conn, 400) =~ "Assessment has not been submitted" end + @tag authenticate: :staff + test "assessments that have not been unpublished should not be allowed to unsubmit", %{ + conn: conn + } do + %{course: course, config: config, students: students} = seed_db(conn) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -1), + close_at: Timex.shift(Timex.now(), hours: 500), + is_published: true, + config: config, + course: course + ) + + question = insert(:programming_question, assessment: assessment) + student = List.first(students) + + submission = + insert(:submission, + assessment: assessment, + student: student, + status: :submitted, + is_grading_published: true + ) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn = + conn + |> post(build_url_unsubmit(course.id, submission.id)) + + assert response(conn, 403) =~ "Grading has not been unpublished" + end + @tag authenticate: :staff test "assessment that is not open anymore cannot be unsubmitted", %{conn: conn} do %{course: course, config: config, students: students} = seed_db(conn) @@ -697,7 +745,12 @@ defmodule CadetWeb.AdminGradingControllerTest do question = insert(:programming_question, assessment: assessment) submission = - insert(:submission, assessment: assessment, student: grader, status: :submitted) + insert(:submission, + assessment: assessment, + student: grader, + status: :submitted, + is_grading_published: false + ) insert( :answer, @@ -732,7 +785,12 @@ defmodule CadetWeb.AdminGradingControllerTest do question = insert(:programming_question, assessment: assessment) submission = - insert(:submission, assessment: assessment, student: grader, status: :submitted) + insert(:submission, + assessment: assessment, + student: grader, + status: :submitted, + is_grading_published: false + ) insert( :answer, @@ -770,7 +828,12 @@ defmodule CadetWeb.AdminGradingControllerTest do student = List.first(students) submission = - insert(:submission, assessment: assessment, student: student, status: :submitted) + insert(:submission, + assessment: assessment, + student: student, + status: :submitted, + is_grading_published: false + ) answer = insert( @@ -799,6 +862,283 @@ defmodule CadetWeb.AdminGradingControllerTest do end end + describe "POST /:submissionid/unpublish_grades" do + setup %{conn: conn} do + %{course: course, config: config, grader: grader, students: students} = seed_db(conn) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -1), + close_at: Timex.shift(Timex.now(), hours: 500), + is_published: true, + config: config, + course: course + ) + + question = insert(:programming_question, assessment: assessment) + + student = List.first(students) + + submission = + insert(:submission, + assessment: assessment, + student: student, + status: :submitted, + is_grading_published: true + ) + + _answer = + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"}, + grader_id: grader.id + ) + + %{ + course: course, + assessment: assessment, + submission: submission, + question: question, + students: students + } + end + + @tag authenticate: :staff + test "succeeds", %{conn: conn, course: course, submission: submission} do + conn + |> post(build_url_unpublish(course.id, submission.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.is_grading_published == false + end + + @tag authenticate: :staff + test "avenger should not be allowed to unpublish for students outside of their group", %{ + conn: conn, + course: course, + submission: submission + } do + other_grader = insert(:course_registration, %{role: :staff, course: course}) + + conn = + conn + |> sign_in(other_grader.user) + |> post(build_url_unpublish(course.id, submission.id)) + + assert response(conn, 403) =~ + "Only Avenger of student or Admin is permitted to unpublish grading" + end + + @tag authenticate: :admin + test "admin should be allowed to unpublish", %{ + conn: conn, + course: course, + submission: submission + } do + admin = insert(:course_registration, %{role: :admin, course: course}) + + conn + |> sign_in(admin.user) + |> post(build_url_unpublish(course.id, submission.id)) + + submission_db = Repo.get(Submission, submission.id) + assert submission_db.is_grading_published == false + end + end + + describe "POST /:submissionid/publish_grades" do + setup %{conn: conn} do + %{course: course, config: config, grader: grader, students: students} = seed_db(conn) + + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -1), + close_at: Timex.shift(Timex.now(), hours: 500), + is_published: true, + config: config, + course: course + ) + + question = insert(:programming_question, assessment: assessment) + + student = List.first(students) + + submission = + insert(:submission, + assessment: assessment, + student: student, + status: :submitted, + is_grading_published: false + ) + + _answer = + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"}, + grader_id: grader.id + ) + + %{ + course: course, + assessment: assessment, + submission: submission, + question: question, + students: students + } + end + + @tag authenticate: :staff + test "succeeds", %{conn: conn, course: course, submission: submission} do + conn + |> post(build_url_publish(course.id, submission.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.is_grading_published == true + end + + @tag authenticate: :staff + test "assessments which have not been submitted should not be allowed to publish", %{ + conn: conn, + course: course, + students: students, + assessment: assessment, + question: question + } do + student = List.last(students) + + submission = + insert(:submission, assessment: assessment, student: student, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn = + conn + |> post(build_url_publish(course.id, submission.id)) + + assert response(conn, 400) =~ "Assessment has not been submitted" + end + + @tag authenticate: :staff + test "avenger should not be allowed to publish for students outside of their group", %{ + conn: conn, + course: course, + submission: submission + } do + other_grader = insert(:course_registration, %{role: :staff, course: course}) + + conn = + conn + |> sign_in(other_grader.user) + |> post(build_url_publish(course.id, submission.id)) + + assert response(conn, 403) =~ + "Only Avenger of student or Admin is permitted to publish grading" + end + + @tag authenticate: :admin + test "admin should be allowed to publish", %{ + conn: conn, + course: course, + submission: submission + } do + admin = insert(:course_registration, %{role: :admin, course: course}) + + conn + |> sign_in(admin.user) + |> post(build_url_publish(course.id, submission.id)) + + submission_db = Repo.get(Submission, submission.id) + assert submission_db.is_grading_published == true + end + end + + describe "POST /:assessmentid/unpublish_all_grades" do + setup %{conn: conn} do + seed = Cadet.Test.Seeds.assessments() + assessment_id = seed[:assessments]["mission"][:assessment].id + %{conn: conn, assessment_id: assessment_id, course: seed[:courses][:course1]} + end + + @tag authenticate: :admin + test "successful", %{conn: conn, assessment_id: assessment_id, course: course} do + admin = insert(:course_registration, %{role: :admin, course: course}) + + conn = + conn + |> sign_in(admin.user) + |> post(build_url_unpublish_all(course.id, assessment_id)) + + assert response(conn, 200) == "OK" + end + + @tag authenticate: :staff + test "staff not allowed to unpublish all grades", %{ + conn: conn, + assessment_id: assessment_id, + course: course + } do + staff = insert(:course_registration, %{role: :staff, course: course}) + + conn = + conn + |> sign_in(staff.user) + |> post(build_url_unpublish_all(course.id, assessment_id)) + + assert response(conn, 403) == "Only Admin is permitted to unpublish all grades" + end + end + + describe "POST /:assessmentid/publish_all_grades" do + setup %{conn: conn} do + seed = Cadet.Test.Seeds.assessments() + assessment_id = seed[:assessments]["mission"][:assessment].id + %{conn: conn, assessment_id: assessment_id, course: seed[:courses][:course1]} + end + + @tag authenticate: :admin + test "successful", %{conn: conn, assessment_id: assessment_id, course: course} do + admin = insert(:course_registration, %{role: :admin, course: course}) + + conn = + conn + |> sign_in(admin.user) + |> post(build_url_publish_all(course.id, assessment_id)) + + assert response(conn, 200) == "OK" + end + + @tag authenticate: :staff + test "staff not allowed to publish all grades", %{ + conn: conn, + assessment_id: assessment_id, + course: course + } do + staff = insert(:course_registration, %{role: :staff, course: course}) + + conn = + conn + |> sign_in(staff.user) + |> post(build_url_publish_all(course.id, assessment_id)) + + assert response(conn, 403) == "Only Admin is permitted to publish all grades" + end + end + describe "GET /, admin" do @tag authenticate: :staff test "can see all submissions", %{conn: conn} do @@ -844,7 +1184,8 @@ defmodule CadetWeb.AdminGradingControllerTest do "gradedCount" => 5, "unsubmittedBy" => nil, "unsubmittedAt" => nil, - "team" => nil + "team" => nil, + "isGradingPublished" => submission.is_grading_published } end) } @@ -898,7 +1239,8 @@ defmodule CadetWeb.AdminGradingControllerTest do "gradedCount" => 5, "unsubmittedBy" => nil, "unsubmittedAt" => nil, - "team" => nil + "team" => nil, + "isGradingPublished" => submission.is_grading_published } end) } @@ -1320,6 +1662,18 @@ defmodule CadetWeb.AdminGradingControllerTest do defp build_url_unsubmit(course_id, submission_id), do: "#{build_url(course_id, submission_id)}/unsubmit" + defp build_url_unpublish(course_id, submission_id), + do: "#{build_url(course_id, submission_id)}/unpublish_grades" + + defp build_url_publish(course_id, submission_id), + do: "#{build_url(course_id, submission_id)}/publish_grades" + + defp build_url_unpublish_all(course_id, assessment_id), + do: "#{build_url(course_id)}#{assessment_id}/unpublish_all_grades" + + defp build_url_publish_all(course_id, assessment_id), + do: "#{build_url(course_id)}#{assessment_id}/publish_all_grades" + defp build_url_autograde(course_id, submission_id), do: "#{build_url(course_id, submission_id)}/autograde" diff --git a/test/cadet_web/admin_controllers/admin_user_controller_test.exs b/test/cadet_web/admin_controllers/admin_user_controller_test.exs index 786210dec..bd134876c 100644 --- a/test/cadet_web/admin_controllers/admin_user_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_user_controller_test.exs @@ -582,7 +582,8 @@ defmodule CadetWeb.AdminUserControllerTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index 83f52445e..741b7f708 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -93,6 +93,7 @@ defmodule CadetWeb.AssessmentsControllerTest do |> get(build_url(course1.id)) |> json_response(200) |> Enum.map(&Map.delete(&1, "xp")) + |> Enum.map(&Map.delete(&1, "isGradingPublished")) assert expected == resp end @@ -168,6 +169,7 @@ defmodule CadetWeb.AssessmentsControllerTest do "isPublished" => &1.is_published, "gradedCount" => 0, "questionCount" => 9, + "isGradingPublished" => false, "earlySubmissionXp" => &1.config.early_submission_xp, "hasVotingFeatures" => &1.has_voting_features, "hasTokenCounter" => &1.has_token_counter, @@ -215,15 +217,15 @@ defmodule CadetWeb.AssessmentsControllerTest do test "renders xp for students", %{ conn: conn, courses: %{course1: course1}, - role_crs: %{student: student}, assessment_configs: configs, - assessments: assessments + assessments: assessments, + student_grading_published: student_grading_published } do assessment = assessments[hd(configs).type].assessment resp = conn - |> sign_in(student.user) + |> sign_in(student_grading_published.user) |> get(build_url(course1.id)) |> json_response(200) |> Enum.find(&(&1["id"] == assessment.id)) @@ -286,6 +288,7 @@ defmodule CadetWeb.AssessmentsControllerTest do "hasTokenCounter" => &1.has_token_counter, "isVotingPublished" => Assessments.is_voting_published(&1.id), "earlySubmissionXp" => &1.config.early_submission_xp, + "isGradingPublished" => nil, "isPublished" => if &1.config.type == hd(configs).type do false @@ -790,8 +793,11 @@ defmodule CadetWeb.AssessmentsControllerTest do conn: conn, courses: %{course1: course1}, role_crs: role_crs, - assessments: assessments + assessments: assessments, + student_grading_published: student_grading_published } do + role_crs = Map.put(role_crs, :student, student_grading_published) + for role <- Role.__enum_map__() do course_reg = Map.get(role_crs, role) @@ -1413,7 +1419,12 @@ defmodule CadetWeb.AssessmentsControllerTest do course_reg = insert(:course_registration, role: :student, course: course1) submission = - insert(:submission, assessment: assessment, student: course_reg, status: :submitted) + insert(:submission, + assessment: assessment, + student: course_reg, + status: :submitted, + is_grading_published: true + ) Enum.each( [question_one, question_two], diff --git a/test/cadet_web/controllers/user_controller_test.exs b/test/cadet_web/controllers/user_controller_test.exs index 1b6465af5..c63067b0a 100644 --- a/test/cadet_web/controllers/user_controller_test.exs +++ b/test/cadet_web/controllers/user_controller_test.exs @@ -42,7 +42,8 @@ defmodule CadetWeb.UserControllerTest do assessment: assessment, student: cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -197,7 +198,8 @@ defmodule CadetWeb.UserControllerTest do assessment: assessment, student: test_cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ @@ -268,7 +270,8 @@ defmodule CadetWeb.UserControllerTest do assessment: assessment, student: cr, status: :submitted, - xp_bonus: 100 + xp_bonus: 100, + is_grading_published: true }) insert(:answer, %{ diff --git a/test/factories/accounts/notification_factory.ex b/test/factories/accounts/notification_factory.ex index 0dc933180..847d0d454 100644 --- a/test/factories/accounts/notification_factory.ex +++ b/test/factories/accounts/notification_factory.ex @@ -8,7 +8,7 @@ defmodule Cadet.Accounts.NotificationFactory do alias Cadet.Accounts.Notification def notification_factory do - valid_types = [:new, :autograded, :graded] + valid_types = [:new] %Notification{ type: Enum.random(valid_types), diff --git a/test/factories/assessments/submission_factory.ex b/test/factories/assessments/submission_factory.ex index da94d87b2..05a3ec35e 100644 --- a/test/factories/assessments/submission_factory.ex +++ b/test/factories/assessments/submission_factory.ex @@ -11,7 +11,8 @@ defmodule Cadet.Assessments.SubmissionFactory do %Submission{ student: build(:course_registration, %{role: :student}), team: nil, - assessment: build(:assessment) + assessment: build(:assessment), + is_grading_published: false } end end diff --git a/test/support/seeds.ex b/test/support/seeds.ex index ce9f55205..e263860e1 100644 --- a/test/support/seeds.ex +++ b/test/support/seeds.ex @@ -65,6 +65,7 @@ defmodule Cadet.Test.Seeds do student_submitted = insert(:user, %{latest_viewed_course: course1}) student_graded = insert(:user, %{latest_viewed_course: course1}) student_different_group = insert(:user, %{latest_viewed_course: course1}) + student_grading_published = insert(:user, %{latest_viewed_course: course1}) # CourseRegistration and Group avenger1_cr = insert(:course_registration, %{user: avenger1, course: course1, role: :staff}) @@ -129,6 +130,14 @@ defmodule Cadet.Test.Seeds do group: group2 }) + student_grading_published_cr = + insert(:course_registration, %{ + user: student_grading_published, + course: course1, + role: :student, + group: group + }) + students = [ student1a_cr, student1b_cr, @@ -136,18 +145,20 @@ defmodule Cadet.Test.Seeds do student_attempted_cr, student_submitted_cr, student_graded_cr, - student_different_group_cr + student_different_group_cr, + student_grading_published_cr ] - # {student_cr, submission_status, is_graded, avenger} + # {student_cr, submission_status, is_graded, is_grading_published, avenger} students_with_assessment_info = [ - {student1a_cr, :attempting, false, avenger1_cr}, - {student1b_cr, :attempting, false, avenger1_cr}, - {student1c_cr, :attempting, false, avenger1_cr}, - {student_attempted_cr, :attempted, false, avenger1_cr}, - {student_submitted_cr, :submitted, false, avenger1_cr}, - {student_graded_cr, :submitted, true, avenger1_cr}, - {student_different_group_cr, :attempting, false, avenger2_cr} + {student1a_cr, :attempting, false, false, avenger1_cr}, + {student1b_cr, :attempting, false, false, avenger1_cr}, + {student1c_cr, :attempting, false, false, avenger1_cr}, + {student_attempted_cr, :attempted, false, false, avenger1_cr}, + {student_submitted_cr, :submitted, false, false, avenger1_cr}, + {student_graded_cr, :submitted, true, false, avenger1_cr}, + {student_different_group_cr, :attempting, false, false, avenger2_cr}, + {student_grading_published_cr, :submitted, true, true, avenger1_cr} ] _admin2cr = @@ -205,7 +216,9 @@ defmodule Cadet.Test.Seeds do admin: admin1_cr }, assessment_configs: assessment_configs, - assessments: assessments + assessments: assessments, + students_with_assessment_info: students_with_assessment_info, + student_grading_published: student_grading_published_cr } end end @@ -250,14 +263,15 @@ defmodule Cadet.Test.Seeds do submissions_with_grader = students - |> Enum.map(fn {student, submission_status, is_graded, avenger} -> + |> Enum.map(fn {student, submission_status, is_graded, is_grading_published, avenger} -> grader = if is_graded, do: avenger, else: nil {grader, insert(:submission, %{ assessment: assessment, student: student, - status: submission_status + status: submission_status, + is_grading_published: is_grading_published })} end)