From 0f0a092e3c09649d7b996b4cdc1f9d3e7e001ffd Mon Sep 17 00:00:00 2001 From: Alden <49450743+alcen@users.noreply.github.com> Date: Tue, 30 Jul 2019 16:08:39 +0800 Subject: [PATCH 1/2] Added 'comments' field to answers schema * Set up Ecto migrations for new 'comments' column * Updated Phoenix controllers and views * Configure unsubmit submission to remove 'comments' field * Updated test scripts --- lib/cadet/assessments/answer.ex | 5 +++-- lib/cadet/assessments/assessments.ex | 3 ++- .../controllers/assessments_controller.ex | 1 + .../controllers/grading_controller.ex | 1 + lib/cadet_web/views/assessments_helpers.ex | 3 ++- lib/cadet_web/views/grading_view.ex | 3 ++- .../20190729154942_restore_comments.exs | 9 +++++++++ .../assessments_controller_test.exs | 1 + .../controllers/grading_controller_test.exs | 19 +++++++++++++------ test/factories/assessments/answer_factory.ex | 3 ++- 10 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 priv/repo/migrations/20190729154942_restore_comments.exs diff --git a/lib/cadet/assessments/answer.ex b/lib/cadet/assessments/answer.ex index cb5d61570..569bd8156 100644 --- a/lib/cadet/assessments/answer.ex +++ b/lib/cadet/assessments/answer.ex @@ -16,6 +16,7 @@ defmodule Cadet.Assessments.Answer do field(:adjustment, :integer, default: 0) field(:xp, :integer, default: 0) field(:xp_adjustment, :integer, default: 0) + field(:comments, :string) field(:autograding_status, AutogradingStatus, default: :none) field(:autograding_results, {:array, :map}, default: []) field(:answer, :map) @@ -30,7 +31,7 @@ defmodule Cadet.Assessments.Answer do end @required_fields ~w(answer submission_id question_id type)a - @optional_fields ~w(xp xp_adjustment grade adjustment grader_id)a + @optional_fields ~w(xp xp_adjustment grade adjustment grader_id comments)a def changeset(answer, params) do answer @@ -49,7 +50,7 @@ defmodule Cadet.Assessments.Answer do answer |> cast( params, - ~w(grader_id xp xp_adjustment grade adjustment autograding_results autograding_status)a + ~w(grader_id xp xp_adjustment grade adjustment autograding_results autograding_status comments)a ) |> add_belongs_to_id_from_model(:grader, params) |> foreign_key_constraint(:grader_id) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index aef490fc3..fb92fff5f 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -482,7 +482,8 @@ defmodule Cadet.Assessments do xp_adjustment: 0, autograding_status: :none, autograding_results: [], - grader_id: nil + grader_id: nil, + comments: nil }) |> Repo.update()} end diff --git a/lib/cadet_web/controllers/assessments_controller.ex b/lib/cadet_web/controllers/assessments_controller.ex index ec3caba09..8bdd51c63 100644 --- a/lib/cadet_web/controllers/assessments_controller.ex +++ b/lib/cadet_web/controllers/assessments_controller.ex @@ -213,6 +213,7 @@ defmodule CadetWeb.AssessmentsController do xp(:integer, "Final XP given to this question. Only provided for students.") grade(:integer, "Final grade given to this question. Only provided for students.") + comments(:string, "String of comments given for a student's answer", required: false) maxGrade( :integer, diff --git a/lib/cadet_web/controllers/grading_controller.ex b/lib/cadet_web/controllers/grading_controller.ex index ee172336e..a45b6500c 100644 --- a/lib/cadet_web/controllers/grading_controller.ex +++ b/lib/cadet_web/controllers/grading_controller.ex @@ -285,6 +285,7 @@ defmodule CadetWeb.GradingController do xpAdjustment(:integer, "xp adjustment given") grader(Schema.ref(:GraderInfo)) gradedAt(:string, "Last graded at", format: "date-time", required: false) + comments(:string, "Comments given by grader") end end, Grading: diff --git a/lib/cadet_web/views/assessments_helpers.ex b/lib/cadet_web/views/assessments_helpers.ex index da367c923..d07d0cd62 100644 --- a/lib/cadet_web/views/assessments_helpers.ex +++ b/lib/cadet_web/views/assessments_helpers.ex @@ -85,7 +85,8 @@ defmodule CadetWeb.AssessmentsHelpers do xp: &((&1.xp || 0) + (&1.xp_adjustment || 0)), grade: &((&1.grade || 0) + (&1.adjustment || 0)), autogradingStatus: :autograding_status, - autogradingResults: build_results(%{results: answer.autograding_results}) + autogradingResults: build_results(%{results: answer.autograding_results}), + comments: :comments }) end diff --git a/lib/cadet_web/views/grading_view.ex b/lib/cadet_web/views/grading_view.ex index a21cc8941..f6583fcbb 100644 --- a/lib/cadet_web/views/grading_view.ex +++ b/lib/cadet_web/views/grading_view.ex @@ -72,7 +72,8 @@ defmodule CadetWeb.GradingView do adjustment: :adjustment, roomId: :room_id, xp: :xp, - xpAdjustment: :xp_adjustment + xpAdjustment: :xp_adjustment, + comments: :comments }) end end diff --git a/priv/repo/migrations/20190729154942_restore_comments.exs b/priv/repo/migrations/20190729154942_restore_comments.exs new file mode 100644 index 000000000..dd664acaf --- /dev/null +++ b/priv/repo/migrations/20190729154942_restore_comments.exs @@ -0,0 +1,9 @@ +defmodule Cadet.Repo.Migrations.RestoreComments do + use Ecto.Migration + + def change do + alter table(:answers) do + add(:comments, :text, null: true) + end + end +end diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index dd47ef9b5..9f69a88b4 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -302,6 +302,7 @@ defmodule CadetWeb.AssessmentsControllerTest do |> Enum.map(&Map.delete(&1, "gradedAt")) |> Enum.map(&Map.delete(&1, "autogradingResults")) |> Enum.map(&Map.delete(&1, "autogradingStatus")) + |> Enum.map(&Map.delete(&1, "comments")) assert expected_questions == resp_questions end diff --git a/test/cadet_web/controllers/grading_controller_test.exs b/test/cadet_web/controllers/grading_controller_test.exs index ee60c768a..d366027c8 100644 --- a/test/cadet_web/controllers/grading_controller_test.exs +++ b/test/cadet_web/controllers/grading_controller_test.exs @@ -295,7 +295,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, @@ -342,7 +343,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, @@ -414,7 +416,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, @@ -461,7 +464,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, @@ -614,6 +618,7 @@ defmodule CadetWeb.GradingControllerTest do assert answer_db.xp_adjustment == 0 assert answer_db.grade == 0 assert answer_db.adjustment == 0 + assert answer_db.comments == nil end @tag authenticate: :staff @@ -931,7 +936,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, @@ -978,7 +984,8 @@ defmodule CadetWeb.GradingControllerTest do "name" => grader.name, "id" => grader.id }, - "gradedAt" => format_datetime(&1.updated_at) + "gradedAt" => format_datetime(&1.updated_at), + "comments" => &1.comments }, "student" => %{ "name" => &1.submission.student.name, diff --git a/test/factories/assessments/answer_factory.ex b/test/factories/assessments/answer_factory.ex index 712412994..ab2e714d3 100644 --- a/test/factories/assessments/answer_factory.ex +++ b/test/factories/assessments/answer_factory.ex @@ -11,7 +11,8 @@ defmodule Cadet.Assessments.AnswerFactory do %Answer{ answer: %{}, autograding_status: :none, - room_id: Faker.Code.issn() + room_id: Faker.Code.issn(), + comments: Faker.Food.dish() } end From dfc631e9e2db01a5d0f39bd3f2b1152a773faf72 Mon Sep 17 00:00:00 2001 From: Alden <49450743+alcen@users.noreply.github.com> Date: Wed, 31 Jul 2019 22:07:21 +0800 Subject: [PATCH 2/2] Fix code readability issue - 'Line is too long' --- lib/cadet/assessments/answer.ex | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/cadet/assessments/answer.ex b/lib/cadet/assessments/answer.ex index 569bd8156..562ee384a 100644 --- a/lib/cadet/assessments/answer.ex +++ b/lib/cadet/assessments/answer.ex @@ -50,7 +50,16 @@ defmodule Cadet.Assessments.Answer do answer |> cast( params, - ~w(grader_id xp xp_adjustment grade adjustment autograding_results autograding_status comments)a + ~w( + grader_id + xp + xp_adjustment + grade + adjustment + autograding_results + autograding_status + comments + )a ) |> add_belongs_to_id_from_model(:grader, params) |> foreign_key_constraint(:grader_id)