From 52de51f52807542d93a40e17822b1f18f0590c6d Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Thu, 9 Aug 2018 22:12:45 +0800 Subject: [PATCH 1/2] Add comment field to the response of AssessmentsController.show --- .../controllers/assessments_controller.ex | 1 + lib/cadet_web/views/assessments_view.ex | 8 ++++-- .../assessments_controller_test.exs | 28 +++++++++++++++++++ test/factories/assessments/answer_factory.ex | 3 +- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/lib/cadet_web/controllers/assessments_controller.ex b/lib/cadet_web/controllers/assessments_controller.ex index 6554fa8d8..914051eaa 100644 --- a/lib/cadet_web/controllers/assessments_controller.ex +++ b/lib/cadet_web/controllers/assessments_controller.ex @@ -154,6 +154,7 @@ defmodule CadetWeb.AssessmentsController do id(:integer, "The question id", required: true) type(:string, "The question type (mcq/programming)", required: true) content(:string, "The question content", required: true) + comment(:string, "Comment given by group leader. Might be null.") choices( Schema.new do diff --git a/lib/cadet_web/views/assessments_view.ex b/lib/cadet_web/views/assessments_view.ex index 229cabe31..7f031e131 100644 --- a/lib/cadet_web/views/assessments_view.ex +++ b/lib/cadet_web/views/assessments_view.ex @@ -73,7 +73,7 @@ defmodule CadetWeb.AssessmentsView do }) do components = [ build_question(%{question: question}), - build_answer_by_question_type(%{question: question}), + build_answer_and_comment_by_question_type(%{question: question}), build_solution_if_ungraded_by_type(%{question: question, assessment: assessment}) ] @@ -105,7 +105,9 @@ defmodule CadetWeb.AssessmentsView do end end - defp build_answer_by_question_type(%{question: %{answer: answer, type: question_type}}) do + defp build_answer_and_comment_by_question_type(%{ + question: %{answer: answer, type: question_type} + }) do # No need to check if answer exists since empty answer would be a # `%Answer{..., answer: nil}` and nil["anything"] = nil @@ -115,7 +117,7 @@ defmodule CadetWeb.AssessmentsView do :mcq -> & &1.answer["choice_id"] end - transform_map_for_view(answer, %{answer: answer_getter}) + transform_map_for_view(answer, %{answer: answer_getter, comment: :comment}) end def build_choice(%{choice: choice}) do diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index 710ccaf4e..e545ec167 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -235,6 +235,7 @@ defmodule CadetWeb.AssessmentsControllerTest do |> Enum.map(&Map.delete(&1, "answer")) |> Enum.map(&Map.delete(&1, "solution")) |> Enum.map(&Map.delete(&1, "library")) + |> Enum.map(&Map.delete(&1, "comment")) assert expected_questions == resp_questions end @@ -399,6 +400,33 @@ defmodule CadetWeb.AssessmentsControllerTest do end end + test "it renders comment", %{ + conn: conn, + users: %{student: student}, + assessments: assessments + } do + for {_type, + %{ + assessment: assessment, + mcq_answers: [mcq_answers | _], + programming_answers: [programming_answers | _] + }} <- assessments do + # Programming questions should come first due to seeding order + expected_comments = + Enum.map(programming_answers ++ mcq_answers, &%{"comment" => &1.comment}) + + resp_comments = + conn + |> sign_in(student) + |> get(build_url(assessment.id)) + |> json_response(200) + |> Map.get("questions", []) + |> Enum.map(&Map.take(&1, ["comment"])) + + assert expected_comments == resp_comments + end + end + test "it does not permit access to not yet open assessments", %{ conn: conn, users: %{student: student}, diff --git a/test/factories/assessments/answer_factory.ex b/test/factories/assessments/answer_factory.ex index d3442d4b0..cd105fd46 100644 --- a/test/factories/assessments/answer_factory.ex +++ b/test/factories/assessments/answer_factory.ex @@ -9,7 +9,8 @@ defmodule Cadet.Assessments.AnswerFactory do def answer_factory do %Answer{ - answer: %{} + answer: %{}, + comment: Faker.Lorem.Shakespeare.En.romeo_and_juliet() } end From f8540e18e53aeff437bf0ee2efa61136a8dbd67e Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Thu, 9 Aug 2018 23:38:20 +0800 Subject: [PATCH 2/2] Address code review --- lib/cadet/updater/CS1101S.ex | 2 +- lib/cadet_web/views/assessments_view.ex | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cadet/updater/CS1101S.ex b/lib/cadet/updater/CS1101S.ex index d4364751d..751c5a88c 100644 --- a/lib/cadet/updater/CS1101S.ex +++ b/lib/cadet/updater/CS1101S.ex @@ -28,7 +28,7 @@ defmodule Cadet.Updater.CS1101S do def clone do Logger.info("Cloning CS1101S: Started") - if repo_cloned? do + if repo_cloned?() do Logger.info("CS1101S is already cloned.") else git("clone", [@remote_repo, @local_name]) diff --git a/lib/cadet_web/views/assessments_view.ex b/lib/cadet_web/views/assessments_view.ex index 7f031e131..d8c855de8 100644 --- a/lib/cadet_web/views/assessments_view.ex +++ b/lib/cadet_web/views/assessments_view.ex @@ -73,7 +73,7 @@ defmodule CadetWeb.AssessmentsView do }) do components = [ build_question(%{question: question}), - build_answer_and_comment_by_question_type(%{question: question}), + build_answer_fields_by_question_type(%{question: question}), build_solution_if_ungraded_by_type(%{question: question, assessment: assessment}) ] @@ -105,7 +105,7 @@ defmodule CadetWeb.AssessmentsView do end end - defp build_answer_and_comment_by_question_type(%{ + defp build_answer_fields_by_question_type(%{ question: %{answer: answer, type: question_type} }) do # No need to check if answer exists since empty answer would be a