diff --git a/lib/cadet/assessments/answer.ex b/lib/cadet/assessments/answer.ex index 165317f10..279630304 100644 --- a/lib/cadet/assessments/answer.ex +++ b/lib/cadet/assessments/answer.ex @@ -12,6 +12,8 @@ defmodule Cadet.Assessments.Answer do schema "answers" do field(:grade, :integer, default: 0) + field(:xp, :integer, default: 0) + field(:xp_adjustment, :integer, default: 0) field(:autograding_status, AutogradingStatus, default: :none) field(:autograding_errors, {:array, :map}, default: []) field(:answer, :map) @@ -24,7 +26,7 @@ defmodule Cadet.Assessments.Answer do end @required_fields ~w(answer submission_id question_id type)a - @optional_fields ~w(grade comment adjustment)a + @optional_fields ~w(xp xp_adjustment grade comment adjustment)a def changeset(answer, params) do answer @@ -32,42 +34,56 @@ defmodule Cadet.Assessments.Answer do |> add_belongs_to_id_from_model([:submission, :question], params) |> add_question_type_from_model(params) |> validate_required(@required_fields) - |> validate_number(:grade, greater_than_or_equal_to: 0.0) |> foreign_key_constraint(:submission_id) |> foreign_key_constraint(:question_id) |> validate_answer_content() + |> validate_xp_grade_adjustment_total() end @spec grading_changeset(%__MODULE__{} | Ecto.Changeset.t(), map()) :: Ecto.Changeset.t() def grading_changeset(answer, params) do answer - |> cast(params, ~w(adjustment comment)a) - |> validate_grade_adjustment_total() + |> cast(params, ~w(xp_adjustment adjustment comment)a) + |> validate_xp_grade_adjustment_total() end @spec autograding_changeset(%__MODULE__{} | Ecto.Changeset.t(), map()) :: Ecto.Changeset.t() def autograding_changeset(answer, params) do - cast(answer, params, ~w(grade adjustment autograding_status autograding_errors)a) + answer + |> cast(params, ~w(grade adjustment xp autograding_status autograding_errors)a) + |> validate_xp_grade_adjustment_total() end - @spec validate_grade_adjustment_total(Ecto.Changeset.t()) :: Ecto.Changeset.t() - defp validate_grade_adjustment_total(changeset) do + @spec validate_xp_grade_adjustment_total(Ecto.Changeset.t()) :: Ecto.Changeset.t() + defp validate_xp_grade_adjustment_total(changeset) do answer = apply_changes(changeset) - total = answer.grade + answer.adjustment + total_grade = answer.grade + answer.adjustment + + total_xp = answer.xp + answer.xp_adjustment with {:question_id, question_id} when is_ecto_id(question_id) <- {:question_id, answer.question_id}, - question <- Repo.get(Question, question_id), - {:total, true} <- {:total, total >= 0 and total <= question.max_grade} do + {:question, %{max_grade: max_grade, max_xp: max_xp}} <- + {:question, Repo.get(Question, question_id)}, + {:total_grade, true} <- {:total_grade, total_grade >= 0 and total_grade <= max_grade}, + {:total_xp, true} <- {:total_xp, total_xp >= 0 and total_xp <= max_xp} do changeset else {:question_id, _} -> add_error(changeset, :question_id, "is required") - {:total, false} -> + {:question, _} -> + add_error(changeset, :question_id, "refers to non-existent question") + + {:total_grade, false} -> add_error(changeset, :adjustment, "must make total be between 0 and question.max_grade") + + {:total_xp, false} -> + add_error(changeset, :xp_adjustment, "must make total be between 0 and question.max_xp") end + |> validate_number(:grade, greater_than_or_equal_to: 0) + |> validate_number(:xp, greater_than_or_equal_to: 0) end defp add_question_type_from_model(changeset, params) do diff --git a/lib/cadet/assessments/assessment.ex b/lib/cadet/assessments/assessment.ex index d83c8e2a8..88a7be1f5 100644 --- a/lib/cadet/assessments/assessment.ex +++ b/lib/cadet/assessments/assessment.ex @@ -10,6 +10,9 @@ defmodule Cadet.Assessments.Assessment do schema "assessments" do field(:max_grade, :integer, virtual: true) + field(:grade, :integer, virtual: true) + field(:max_xp, :integer, virtual: true) + field(:xp, :integer, virtual: true) field(:user_status, SubmissionStatus, virtual: true) field(:title, :string) field(:is_published, :boolean, default: false) diff --git a/lib/cadet/assessments/assessments.ex b/lib/cadet/assessments/assessments.ex index 43421eaac..8fed0e203 100644 --- a/lib/cadet/assessments/assessments.ex +++ b/lib/cadet/assessments/assessments.ex @@ -12,42 +12,62 @@ defmodule Cadet.Assessments do alias Cadet.Autograder.GradingJob alias Ecto.Multi + @xp_early_submission_max_bonus 100 + @xp_bonus_assessment_type ~w(mission sidequest)a @submit_answer_roles ~w(student)a @grading_roles ~w(staff)a @open_all_assessment_roles ~w(staff admin)a - @spec user_max_grade(%User{}) :: integer() - def user_max_grade(%User{id: user_id}) when is_ecto_id(user_id) do - max_grade = + @spec user_total_xp(%User{}) :: integer() + def user_total_xp(%User{id: user_id}) when is_ecto_id(user_id) do + total_xp_bonus = Submission - |> where(status: ^:submitted) |> where(student_id: ^user_id) - |> join( - :inner, - [s], - a in subquery(Query.all_assessments_with_max_grade()), - s.assessment_id == a.id - ) - |> select([_, a], sum(a.max_grade)) + |> Repo.aggregate(:sum, :xp_bonus) + |> case do + nil -> 0 + xp when is_integer(xp) -> xp + end + + total_xp = + Query.all_submissions_with_xp() + |> subquery() + |> where(student_id: ^user_id) + |> select([q], fragment("? + ?", sum(q.xp), sum(q.xp_adjustment))) |> Repo.one() + |> decimal_to_integer() - if max_grade do - Decimal.to_integer(max_grade) - else - 0 - end + total_xp_bonus + total_xp + end + + @spec user_max_grade(%User{}) :: integer() + def user_max_grade(%User{id: user_id}) when is_ecto_id(user_id) do + Submission + |> where(status: ^:submitted) + |> where(student_id: ^user_id) + |> join( + :inner, + [s], + a in subquery(Query.all_assessments_with_max_grade()), + s.assessment_id == a.id + ) + |> select([_, a], sum(a.max_grade)) + |> Repo.one() + |> decimal_to_integer() end def user_total_grade(%User{id: user_id}) do - grade = - Query.all_submissions_with_grade() - |> subquery() - |> where(student_id: ^user_id) - |> select([q], fragment("? + ?", sum(q.grade), sum(q.adjustment))) - |> Repo.one() + Query.all_submissions_with_grade() + |> subquery() + |> where(student_id: ^user_id) + |> select([q], fragment("? + ?", sum(q.grade), sum(q.adjustment))) + |> Repo.one() + |> decimal_to_integer() + end - if grade do - Decimal.to_integer(grade) + defp decimal_to_integer(decimal) do + if Decimal.decimal?(decimal) do + Decimal.to_integer(decimal) else 0 end @@ -141,10 +161,20 @@ defmodule Cadet.Assessments do """ def all_published_assessments(user = %User{}) do assessments = - Query.all_assessments_with_max_grade() + Query.all_assessments_with_max_xp_and_grade() |> subquery() - |> join(:left, [a], s in Submission, a.id == s.assessment_id and s.student_id == ^user.id) - |> select([a, s], %{a | user_status: s.status}) + |> join( + :left, + [a], + s in subquery(Query.all_submissions_with_xp_and_grade()), + a.id == s.assessment_id and s.student_id == ^user.id + ) + |> select([a, s], %{ + a + | xp: fragment("? + ? + ?", s.xp, s.xp_adjustment, s.xp_bonus), + grade: fragment("? + ?", s.grade, s.adjustment), + user_status: s.status + }) |> where(is_published: true) |> order_by(:open_at) |> Repo.all() @@ -315,8 +345,7 @@ defmodule Cadet.Assessments do with {:submission_found?, true} <- {:submission_found?, is_map(submission)}, {:is_open?, true} <- is_open?(submission.assessment), {:status, :attempted} <- {:status, submission.status}, - {:ok, updated_submission} <- - submission |> Submission.changeset(%{status: :submitted}) |> Repo.update() do + {:ok, updated_submission} <- update_submission_status_and_xp_bonus(submission) do GradingJob.force_grade_individual_submission(updated_submission) {:ok, nil} @@ -341,6 +370,29 @@ defmodule Cadet.Assessments do end end + @spec update_submission_status_and_xp_bonus(%Submission{}) :: + {:ok, %Submission{}} | {:error, Ecto.Changeset.t()} + defp update_submission_status_and_xp_bonus(submission = %Submission{}) do + assessment = submission.assessment + + xp_bonus = + cond do + assessment.type not in @xp_bonus_assessment_type -> + 0 + + Timex.before?(Timex.now(), Timex.shift(assessment.open_at, hours: 48)) -> + @xp_early_submission_max_bonus + + true -> + deduction = Timex.diff(Timex.now(), assessment.open_at, :hours) - 48 + Enum.max([0, @xp_early_submission_max_bonus - deduction]) + end + + submission + |> Submission.changeset(%{status: :submitted, xp_bonus: xp_bonus}) + |> Repo.update() + end + def update_submission_status(submission = %Submission{}, assessment = %Assessment{}) do model_assoc_count = fn model, assoc, id -> model @@ -375,18 +427,25 @@ defmodule Cadet.Assessments do submissions = Submission - |> join(:inner, [s], x in subquery(Query.submissions_grade()), s.id == x.submission_id) + |> join( + :inner, + [s], + x in subquery(Query.submissions_xp_and_grade()), + s.id == x.submission_id + ) |> join(:inner, [s], st in subquery(students), s.student_id == st.id) |> join( :inner, [s], - a in subquery(Query.all_assessments_with_max_grade()), + a in subquery(Query.all_assessments_with_max_xp_and_grade()), s.assessment_id == a.id ) |> select([s, x, st, a], %Submission{ s | grade: x.grade, adjustment: x.adjustment, + xp: x.xp, + xp_adjustment: x.xp_adjustment, student: st, assessment: a }) diff --git a/lib/cadet/assessments/query.ex b/lib/cadet/assessments/query.ex index a81b9d063..91ef4f650 100644 --- a/lib/cadet/assessments/query.ex +++ b/lib/cadet/assessments/query.ex @@ -6,6 +6,34 @@ defmodule Cadet.Assessments.Query do alias Cadet.Assessments.{Answer, Assessment, Question, Submission} + @doc """ + Returns a query with the following bindings: + [submissions_with_xp_and_grade, answers] + """ + @spec all_submissions_with_xp_and_grade :: Ecto.Query.t() + def all_submissions_with_xp_and_grade do + Submission + |> join(:inner, [s], q in subquery(submissions_xp_and_grade()), s.id == q.submission_id) + |> select([s, q], %Submission{ + s + | xp: q.xp, + xp_adjustment: q.xp_adjustment, + grade: q.grade, + adjustment: q.adjustment + }) + end + + @doc """ + Returns a query with the following bindings: + [submissions_with_xp, answers] + """ + @spec all_submissions_with_xp :: Ecto.Query.t() + def all_submissions_with_xp do + Submission + |> join(:inner, [s], q in subquery(submissions_xp()), s.id == q.submission_id) + |> select([s, q], %Submission{s | xp: q.xp, xp_adjustment: q.xp_adjustment}) + end + @doc """ Returns a query with the following bindings: [submissions_with_grade, answers] @@ -17,6 +45,17 @@ defmodule Cadet.Assessments.Query do |> select([s, q], %Submission{s | grade: q.grade, adjustment: q.adjustment}) end + @doc """ + Returns a query with the following bindings: + [assessments_with_xp_and_grade, questions] + """ + @spec all_assessments_with_max_xp_and_grade :: Ecto.Query.t() + def all_assessments_with_max_xp_and_grade do + Assessment + |> join(:inner, [a], q in subquery(assessments_max_xp_and_grade()), a.id == q.assessment_id) + |> select([a, q], %Assessment{a | max_grade: q.max_grade, max_xp: q.max_xp}) + end + @doc """ Returns a query with the following bindings: [assessments_with_grade, questions] @@ -28,6 +67,19 @@ defmodule Cadet.Assessments.Query do |> select([a, q], %Assessment{a | max_grade: q.max_grade}) end + @spec submissions_xp_and_grade :: Ecto.Query.t() + def submissions_xp_and_grade do + Answer + |> group_by(:submission_id) + |> select([a], %{ + submission_id: a.submission_id, + grade: sum(a.grade), + adjustment: sum(a.adjustment), + xp: sum(a.xp), + xp_adjustment: sum(a.xp_adjustment) + }) + end + @spec submissions_grade :: Ecto.Query.t() def submissions_grade do Answer @@ -39,10 +91,32 @@ defmodule Cadet.Assessments.Query do }) end + @spec submissions_xp :: Ecto.Query.t() + def submissions_xp do + Answer + |> group_by(:submission_id) + |> select([a], %{ + submission_id: a.submission_id, + xp: sum(a.xp), + xp_adjustment: sum(a.xp_adjustment) + }) + end + @spec assessments_max_grade :: Ecto.Query.t() def assessments_max_grade do Question |> group_by(:assessment_id) |> select([q], %{assessment_id: q.assessment_id, max_grade: sum(q.max_grade)}) end + + @spec assessments_max_xp_and_grade :: Ecto.Query.t() + def assessments_max_xp_and_grade do + Question + |> group_by(:assessment_id) + |> select([q], %{ + assessment_id: q.assessment_id, + max_grade: sum(q.max_grade), + max_xp: sum(q.max_xp) + }) + end end diff --git a/lib/cadet/assessments/question.ex b/lib/cadet/assessments/question.ex index bf7045d8c..f939d7751 100644 --- a/lib/cadet/assessments/question.ex +++ b/lib/cadet/assessments/question.ex @@ -13,6 +13,7 @@ defmodule Cadet.Assessments.Question do field(:question, :map) field(:type, QuestionType) field(:max_grade, :integer) + field(:max_xp, :integer) field(:answer, :map, virtual: true) embeds_one(:library, Library) embeds_one(:grading_library, Library) @@ -21,7 +22,7 @@ defmodule Cadet.Assessments.Question do end @required_fields ~w(question type assessment_id)a - @optional_fields ~w(display_order max_grade)a + @optional_fields ~w(display_order max_grade max_xp)a @required_embeds ~w(library)a def changeset(question, params) do diff --git a/lib/cadet/assessments/submission.ex b/lib/cadet/assessments/submission.ex index d1b19f0e7..93be3afe0 100644 --- a/lib/cadet/assessments/submission.ex +++ b/lib/cadet/assessments/submission.ex @@ -8,6 +8,9 @@ defmodule Cadet.Assessments.Submission do schema "submissions" do field(:grade, :integer, virtual: true) field(:adjustment, :integer, virtual: true) + field(:xp, :integer, virtual: true) + field(:xp_adjustment, :integer, virtual: true) + field(:xp_bonus, :integer, default: 0) field(:status, SubmissionStatus, default: :attempting) belongs_to(:assessment, Assessment) @@ -18,10 +21,17 @@ defmodule Cadet.Assessments.Submission do end @required_fields ~w(student_id assessment_id status)a + @optional_fields ~w(xp_bonus)a + @xp_early_submission_max_bonus 100 def changeset(submission, params) do submission - |> cast(params, @required_fields) + |> cast(params, @required_fields ++ @optional_fields) + |> validate_number( + :xp_bonus, + greater_than_or_equal_to: 0, + less_than_or_equal_to: @xp_early_submission_max_bonus + ) |> add_belongs_to_id_from_model([:student, :assessment], params) |> validate_required(@required_fields) |> foreign_key_constraint(:student_id) diff --git a/lib/cadet/jobs/autograder/grading_job.ex b/lib/cadet/jobs/autograder/grading_job.ex index 250b7a466..cc2f0c723 100644 --- a/lib/cadet/jobs/autograder/grading_job.ex +++ b/lib/cadet/jobs/autograder/grading_job.ex @@ -119,8 +119,17 @@ defmodule Cadet.Autograder.GradingJob do grade = if answer.answer["choice_id"] == correct_choice, do: question.max_grade, else: 0 + xp = + if question.max_grade == 0, + do: 0, + else: Integer.floor_div(question.max_xp * grade, question.max_grade) + answer - |> Answer.autograding_changeset(%{grade: grade, autograding_status: :success}) + |> Answer.autograding_changeset(%{ + grade: grade, + xp: xp, + autograding_status: :success + }) |> Repo.update() end diff --git a/lib/cadet/jobs/autograder/result_store_worker.ex b/lib/cadet/jobs/autograder/result_store_worker.ex index bb92cd0b1..a4bf7328f 100644 --- a/lib/cadet/jobs/autograder/result_store_worker.ex +++ b/lib/cadet/jobs/autograder/result_store_worker.ex @@ -8,6 +8,7 @@ defmodule Cadet.Autograder.ResultStoreWorker do require Logger import Cadet.SharedHelper + import Ecto.Query alias Ecto.Multi @@ -34,7 +35,11 @@ defmodule Cadet.Autograder.ResultStoreWorker do end defp fetch_answer(answer_id) when is_ecto_id(answer_id) do - answer = Repo.get(Answer, answer_id) + answer = + Answer + |> join(:inner, [a], q in assoc(a, :question)) + |> preload([_, q], question: q) + |> Repo.get(answer_id) if answer do {:ok, answer} @@ -44,9 +49,17 @@ defmodule Cadet.Autograder.ResultStoreWorker do end defp update_answer(answer = %Answer{}, result = %{status: status}) do + xp = + if answer.question.max_grade == 0 do + 0 + else + Integer.floor_div(answer.question.max_xp * result.grade, answer.question.max_grade) + end + changes = %{ adjustment: 0, grade: result.grade, + xp: xp, autograding_status: status, autograding_errors: result.errors } diff --git a/lib/cadet/jobs/xml_parser.ex b/lib/cadet/jobs/xml_parser.ex index f8e303241..3d9b02e01 100644 --- a/lib/cadet/jobs/xml_parser.ex +++ b/lib/cadet/jobs/xml_parser.ex @@ -197,6 +197,7 @@ defmodule Cadet.Updater.XMLParser do ~x"//PROBLEMS/PROBLEM"el, type: ~x"./@type"o |> transform_by(&process_charlist/1), max_grade: ~x"./@maxgrade"oi, + max_xp: ~x"./@maxxp"oi, entity: ~x"." ) |> Enum.map(fn param -> diff --git a/lib/cadet_web/controllers/assessments_controller.ex b/lib/cadet_web/controllers/assessments_controller.ex index 914051eaa..fa02855d3 100644 --- a/lib/cadet_web/controllers/assessments_controller.ex +++ b/lib/cadet_web/controllers/assessments_controller.ex @@ -118,6 +118,16 @@ defmodule CadetWeb.AssessmentsController do required: true ) + maxXp( + :integer, + "The maximum xp for this assessment", + required: true + ) + + xp(:integer, "The xp earned for this assessment", required: true) + + grade(:integer, "The grade earned for this assessment", required: true) + coverImage(:string, "The URL to the cover picture", required: true) end end, diff --git a/lib/cadet_web/controllers/grading_controller.ex b/lib/cadet_web/controllers/grading_controller.ex index af14d6f08..81e6be454 100644 --- a/lib/cadet_web/controllers/grading_controller.ex +++ b/lib/cadet_web/controllers/grading_controller.ex @@ -37,12 +37,19 @@ defmodule CadetWeb.GradingController do %{ "submissionid" => submission_id, "questionid" => question_id, - "grading" => grading + "grading" => raw_grading } ) when is_ecto_id(submission_id) and is_ecto_id(question_id) do user = conn.assigns[:current_user] + grading = + if raw_grading["xpAdjustment"] do + Map.put(raw_grading, "xp_adjustment", raw_grading["xpAdjustment"]) + else + raw_grading + end + case Assessments.update_grading_info( %{submission_id: submission_id, question_id: question_id}, grading, @@ -130,7 +137,9 @@ defmodule CadetWeb.GradingController do properties do id(:integer, "submission id", required: true) grade(:integer, "grade given") - adjustment(:integer, "adjustment given") + xp(:integer, "xp earned") + xpAdjustment(:integer, "xp adjustment given") + adjustment(:integer, "grade adjustment given") assessment(Schema.ref(:AssessmentInfo)) student(Schema.ref(:StudentInfo)) end @@ -148,6 +157,12 @@ defmodule CadetWeb.GradingController do "The max grade for this assessment", required: true ) + + maxXp( + :integer, + "The max xp for this assessment", + required: true + ) end end, StudentInfo: @@ -183,6 +198,12 @@ defmodule CadetWeb.GradingController do "the max grade that can be given to this question", required: true ) + + maxXp( + :integer, + "the max xp that can be given to this question", + required: true + ) end end ) @@ -191,8 +212,10 @@ defmodule CadetWeb.GradingController do swagger_schema do properties do grade(:integer, "Grade awarded by autograder") + xp(:integer, "XP awarded by autograder") comment(:string, "comment given") - adjustment(:integer, "adjustment given") + adjustment(:integer, "grade adjustment given") + xpAdjustment(:integer, "xp adjustment given") end end, Grading: @@ -202,7 +225,8 @@ defmodule CadetWeb.GradingController do Schema.new do properties do comment(:string, "comment given") - adjustment(:integer, "adjustment given") + adjustment(:integer, "grade adjustment given") + xpAdjustment(:integer, "xp adjustment given") end end ) diff --git a/lib/cadet_web/controllers/user_controller.ex b/lib/cadet_web/controllers/user_controller.ex index 7dd7f4f00..e43e9d9a8 100644 --- a/lib/cadet_web/controllers/user_controller.ex +++ b/lib/cadet_web/controllers/user_controller.ex @@ -13,7 +13,17 @@ defmodule CadetWeb.UserController do grade = user_total_grade(user) max_grade = user_max_grade(user) story = user_current_story(user) - render(conn, "index.json", user: user, grade: grade, max_grade: max_grade, story: story) + xp = user_total_xp(user) + + render( + conn, + "index.json", + user: user, + grade: grade, + max_grade: max_grade, + story: story, + xp: xp + ) end swagger_path :index do @@ -47,13 +57,22 @@ defmodule CadetWeb.UserController do story(Schema.ref(:UserStory), "Story to displayed to current user. ") - grade(:integer, "Amount of grade. Only provided for 'Student'") + grade( + :integer, + "Amount of grade. Only provided for 'Student'." <> + "Value will be 0 for non-students." + ) maxGrade( :integer, "Total maximum grade achievable based on submitted assessments." <> "Only provided for 'Student'" ) + + xp( + :integer, + "Amount of xp. Only provided for 'Student'." <> "Value will be 0 for non-students." + ) end end, UserStory: diff --git a/lib/cadet_web/views/assessments_view.ex b/lib/cadet_web/views/assessments_view.ex index d8c855de8..5fe13d61e 100644 --- a/lib/cadet_web/views/assessments_view.ex +++ b/lib/cadet_web/views/assessments_view.ex @@ -21,6 +21,9 @@ defmodule CadetWeb.AssessmentsView do reading: :reading, status: &(&1.user_status || "not_attempted"), maxGrade: :max_grade, + maxXp: :max_xp, + xp: :xp, + grade: &(&1.grade || 0), coverImage: :cover_picture }) end diff --git a/lib/cadet_web/views/grading_view.ex b/lib/cadet_web/views/grading_view.ex index 7f82f6863..d5ee3dbd2 100644 --- a/lib/cadet_web/views/grading_view.ex +++ b/lib/cadet_web/views/grading_view.ex @@ -12,6 +12,8 @@ defmodule CadetWeb.GradingView do def render("submission.json", %{submission: submission}) do transform_map_for_view(submission, %{ grade: :grade, + xp: :xp, + xpAdjustment: :xp_adjustment, adjustment: :adjustment, id: :id, student: &transform_map_for_view(&1.student, [:name, :id]), @@ -19,6 +21,7 @@ defmodule CadetWeb.GradingView do &transform_map_for_view(&1.assessment, %{ type: :type, maxGrade: :max_grade, + maxXp: :max_xp, id: :id, title: :title, coverImage: :cover_picture @@ -34,9 +37,17 @@ defmodule CadetWeb.GradingView do :answer, &1.answer["code"] || &1.answer["choice_id"] ), - maxGrade: & &1.question.max_grade, solution: &(&1.question.question["solution"] || ""), - grade: &transform_map_for_view(&1, [:grade, :adjustment, :comment]) + maxGrade: & &1.question.max_grade, + maxXp: & &1.question.max_xp, + grade: + &transform_map_for_view(&1, %{ + grade: :grade, + adjustment: :adjustment, + comment: :comment, + xp: :xp, + xpAdjustment: :xp_adjustment + }) }) end end diff --git a/lib/cadet_web/views/user_view.ex b/lib/cadet_web/views/user_view.ex index 97323adf2..f8b3e711b 100644 --- a/lib/cadet_web/views/user_view.ex +++ b/lib/cadet_web/views/user_view.ex @@ -1,11 +1,12 @@ defmodule CadetWeb.UserView do use CadetWeb, :view - def render("index.json", %{user: user, grade: grade, max_grade: max_grade, story: story}) do + def render("index.json", %{user: user, grade: grade, max_grade: max_grade, xp: xp, story: story}) do %{ name: user.name, role: user.role, grade: grade, + xp: xp, maxGrade: max_grade, story: transform_map_for_view(story, %{ diff --git a/priv/repo/migrations/20180818060805_add_xp_to_assessments.exs b/priv/repo/migrations/20180818060805_add_xp_to_assessments.exs new file mode 100644 index 000000000..189797d6b --- /dev/null +++ b/priv/repo/migrations/20180818060805_add_xp_to_assessments.exs @@ -0,0 +1,18 @@ +defmodule Cadet.Repo.Migrations.AddXpToAssessments do + use Ecto.Migration + + def change do + alter table(:questions) do + add(:max_xp, :integer, default: 0) + end + + alter table(:submissions) do + add(:xp_bonus, :integer, default: 0) + end + + alter table(:answers) do + add(:xp, :integer, default: 0) + add(:xp_adjustment, :integer, default: 0) + end + end +end diff --git a/priv/repo/migrations/20180821164317_add_index_on_student_id_in_submission.exs b/priv/repo/migrations/20180821164317_add_index_on_student_id_in_submission.exs new file mode 100644 index 000000000..b5c27f500 --- /dev/null +++ b/priv/repo/migrations/20180821164317_add_index_on_student_id_in_submission.exs @@ -0,0 +1,7 @@ +defmodule Cadet.Repo.Migrations.AddIndexOnStudentIdInSubmission do + use Ecto.Migration + + def change do + create(index(:submissions, :student_id)) + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 527a775a7..11dfe16a2 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -28,13 +28,15 @@ if Application.get_env(:cadet, :environment) == :dev do programming_questions = insert_list(3, :programming_question, %{ assessment: assessment, - max_grade: 200 + max_grade: 200, + max_xp: 1_000 }) mcq_questions = insert_list(3, :mcq_question, %{ assessment: assessment, - max_grade: 40 + max_grade: 40, + max_xp: 500 }) submissions = @@ -53,6 +55,7 @@ if Application.get_env(:cadet, :environment) == :dev do question <- programming_questions do insert(:answer, %{ grade: Enum.random(0..200), + xp: Enum.random(0..1_000), question: question, submission: submission, answer: build(:programming_answer) @@ -64,6 +67,7 @@ if Application.get_env(:cadet, :environment) == :dev do question <- mcq_questions do insert(:answer, %{ grade: Enum.random(0..40), + xp: Enum.random(0..500), question: question, submission: submission, answer: build(:mcq_answer) diff --git a/test/cadet/jobs/autograder/grading_job_test.exs b/test/cadet/jobs/autograder/grading_job_test.exs index 6b82a2178..0ee816bf8 100644 --- a/test/cadet/jobs/autograder/grading_job_test.exs +++ b/test/cadet/jobs/autograder/grading_job_test.exs @@ -311,6 +311,7 @@ defmodule Cadet.Autograder.GradingJobTest do for answer <- inserted_empty_answers do assert answer.grade == 0 + assert answer.xp == 0 assert answer.autograding_status == :success assert answer.answer == %{"code" => "// Question not answered by student."} assert answer.comment == "Question not attempted by student" @@ -361,6 +362,7 @@ defmodule Cadet.Autograder.GradingJobTest do for answer <- answers do assert answer.grade == 0 + assert answer.xp == 0 assert answer.autograding_status == :success assert answer.answer == %{"choice_id" => 0} assert answer.comment == "Question not attempted by student" @@ -408,8 +410,10 @@ defmodule Cadet.Autograder.GradingJobTest do # seeded questions have correct choice as 0 if answer_db.answer["choice_id"] == 0 do assert answer_db.grade == question.max_grade + assert answer_db.xp == question.max_xp else assert answer_db.grade == 0 + assert answer_db.xp == 0 end assert answer_db.autograding_status == :success diff --git a/test/cadet/jobs/autograder/result_store_worker_test.exs b/test/cadet/jobs/autograder/result_store_worker_test.exs index bd2700b44..b8a84b517 100644 --- a/test/cadet/jobs/autograder/result_store_worker_test.exs +++ b/test/cadet/jobs/autograder/result_store_worker_test.exs @@ -37,7 +37,7 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do status: :failed } - %{answer: answer, results: [success_no_errors, success_with_errors, failed]} + %{answer: answer, results: [failed, success_with_errors, success_no_errors]} end describe "#perform, invalid answer_id" do @@ -56,7 +56,11 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do for result <- results do ResultStoreWorker.perform(%{answer_id: answer.id, result: result}) - answer = Repo.get(Answer, answer.id) + answer = + Answer + |> join(:inner, [a], q in assoc(a, :question)) + |> preload([_, q], question: q) + |> Repo.get(answer.id) errors_string_keys = Enum.map(result.errors, fn err -> @@ -67,6 +71,17 @@ defmodule Cadet.Autograder.ResultStoreWorkerTest do assert answer.grade == result.grade assert answer.adjustment == 0 + + if answer.question.max_grade == 0 do + assert answer.xp == 0 + else + assert answer.xp == + Integer.floor_div( + answer.question.max_xp * answer.grade, + answer.question.max_grade + ) + end + assert answer.autograding_status == result.status assert answer.autograding_errors == errors_string_keys end diff --git a/test/cadet_web/controllers/assessments_controller_test.exs b/test/cadet_web/controllers/assessments_controller_test.exs index c00c14cdb..ca6279623 100644 --- a/test/cadet_web/controllers/assessments_controller_test.exs +++ b/test/cadet_web/controllers/assessments_controller_test.exs @@ -6,7 +6,7 @@ defmodule CadetWeb.AssessmentsControllerTest do import Mock alias Cadet.Accounts.{Role, User} - alias Cadet.Assessments.{Assessment, Submission, SubmissionStatus} + alias Cadet.Assessments.{Assessment, AssessmentType, Submission, SubmissionStatus} alias Cadet.Autograder.GradingJob alias Cadet.Repo alias CadetWeb.AssessmentsController @@ -15,6 +15,9 @@ defmodule CadetWeb.AssessmentsControllerTest do Cadet.Test.Seeds.assessments() end + @xp_early_submission_max_bonus 100 + @xp_bonus_assessment_type ~w(mission sidequest)a + test "swagger" do AssessmentsController.swagger_definitions() AssessmentsController.swagger_path_index(nil) @@ -58,6 +61,7 @@ defmodule CadetWeb.AssessmentsControllerTest do "type" => "#{&1.type}", "coverImage" => &1.cover_picture, "maxGrade" => 720, + "maxXp" => 4500, "status" => get_assessment_status(user, &1) } ) @@ -67,6 +71,8 @@ defmodule CadetWeb.AssessmentsControllerTest do |> sign_in(user) |> get(build_url()) |> json_response(200) + |> Enum.map(&Map.delete(&1, "xp")) + |> Enum.map(&Map.delete(&1, "grade")) assert expected == resp end @@ -104,6 +110,7 @@ defmodule CadetWeb.AssessmentsControllerTest do "type" => "#{&1.type}", "coverImage" => &1.cover_picture, "maxGrade" => 720, + "maxXp" => 4500, "status" => get_assessment_status(user, &1) } ) @@ -113,6 +120,8 @@ defmodule CadetWeb.AssessmentsControllerTest do |> sign_in(user) |> get(build_url()) |> json_response(200) + |> Enum.map(&Map.delete(&1, "xp")) + |> Enum.map(&Map.delete(&1, "grade")) assert expected == resp end @@ -144,6 +153,42 @@ defmodule CadetWeb.AssessmentsControllerTest do assert get_assessment_status(student, assessment) == resp end end + + test "renders xp for students", %{ + conn: conn, + users: %{student: student}, + assessments: assessments + } do + assessment = assessments.mission.assessment + + resp = + conn + |> sign_in(student) + |> get(build_url()) + |> json_response(200) + |> Enum.find(&(&1["id"] == assessment.id)) + |> Map.get("xp") + + assert resp == 1000 * 3 + 500 * 3 + end + + test "renders grade for students", %{ + conn: conn, + users: %{student: student}, + assessments: assessments + } do + assessment = assessments.mission.assessment + + resp = + conn + |> sign_in(student) + |> get(build_url()) + |> json_response(200) + |> Enum.find(&(&1["id"] == assessment.id)) + |> Map.get("grade") + + assert resp == 200 * 3 + 40 * 3 + end end describe "GET /assessment_id, all roles" do @@ -541,10 +586,172 @@ defmodule CadetWeb.AssessmentsControllerTest do # Preloading is necessary because Mock does an exact match, including metadata submission_db = Submission |> Repo.get(submission.id) |> Repo.preload(:assessment) + assert submission_db.status == :submitted + assert_called(GradingJob.force_grade_individual_submission(submission_db)) end end + test "submission of answer within 2 days of opening grants full XP bonus", %{conn: conn} do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + for type <- @xp_bonus_assessment_type do + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -40), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + type: type + ) + + question = insert(:programming_question, assessment: assessment) + user = insert(:user, role: :student) + + submission = + insert(:submission, assessment: assessment, student: user, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn + |> sign_in(user) + |> post(build_url_submit(assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == @xp_early_submission_max_bonus + end + end + end + + test "submission of answer after 2 days within the next 100 hours of opening grants decaying XP bonus", + %{conn: conn} do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + for hours_after <- 48..148, + type <- @xp_bonus_assessment_type do + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -hours_after), + close_at: Timex.shift(Timex.now(), hours: 500), + is_published: true, + type: type + ) + + question = insert(:programming_question, assessment: assessment) + user = insert(:user, role: :student) + + submission = + insert(:submission, assessment: assessment, student: user, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn + |> sign_in(user) + |> post(build_url_submit(assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == @xp_early_submission_max_bonus - (hours_after - 48) + end + end + end + + test "submission of answer after 2 days and after the next 100 hours yield 0 XP bonus", %{ + conn: conn + } do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + for type <- @xp_bonus_assessment_type do + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -150), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + type: type + ) + + question = insert(:programming_question, assessment: assessment) + user = insert(:user, role: :student) + + submission = + insert(:submission, assessment: assessment, student: user, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn + |> sign_in(user) + |> post(build_url_submit(assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == 0 + end + end + end + + test "does not give bonus for non-bonus eligible assessment types", %{conn: conn} do + with_mock GradingJob, force_grade_individual_submission: fn _ -> nil end do + non_eligible_types = + Enum.filter(AssessmentType.__enum_map__(), &(&1 not in @xp_bonus_assessment_type)) + + for hours_after <- 0..148, + type <- non_eligible_types do + assessment = + insert( + :assessment, + open_at: Timex.shift(Timex.now(), hours: -hours_after), + close_at: Timex.shift(Timex.now(), days: 7), + is_published: true, + type: type + ) + + question = insert(:programming_question, assessment: assessment) + user = insert(:user, role: :student) + + submission = + insert(:submission, assessment: assessment, student: user, status: :attempted) + + insert( + :answer, + submission: submission, + question: question, + answer: %{code: "f => f(f);"} + ) + + conn + |> sign_in(user) + |> post(build_url_submit(assessment.id)) + |> response(200) + + submission_db = Repo.get(Submission, submission.id) + + assert submission_db.status == :submitted + assert submission_db.xp_bonus == 0 + end + end + end + # This also covers unpublished and assessments that are not open yet since they cannot be # answered. test "is not permitted for unattempted assessments", %{ diff --git a/test/cadet_web/controllers/grading_controller_test.exs b/test/cadet_web/controllers/grading_controller_test.exs index ec27f2ed8..601fc7880 100644 --- a/test/cadet_web/controllers/grading_controller_test.exs +++ b/test/cadet_web/controllers/grading_controller_test.exs @@ -76,6 +76,8 @@ defmodule CadetWeb.GradingControllerTest do expected = Enum.map(submissions, fn submission -> %{ + "xp" => 4000, + "xpAdjustment" => -2000, "grade" => 800, "adjustment" => -400, "id" => submission.id, @@ -86,6 +88,7 @@ defmodule CadetWeb.GradingControllerTest do "assessment" => %{ "type" => "mission", "maxGrade" => 800, + "maxXp" => 4000, "id" => mission.id, "title" => mission.title, "coverImage" => mission.cover_picture @@ -146,10 +149,13 @@ defmodule CadetWeb.GradingControllerTest do }, "solution" => &1.question.question.solution, "maxGrade" => &1.question.max_grade, + "maxXp" => &1.question.max_xp, "grade" => %{ "grade" => &1.grade, "adjustment" => &1.adjustment, - "comment" => &1.comment + "comment" => &1.comment, + "xp" => &1.xp, + "xpAdjustment" => &1.xp_adjustment } } @@ -179,10 +185,13 @@ defmodule CadetWeb.GradingControllerTest do }, "solution" => "", "maxGrade" => &1.question.max_grade, + "maxXp" => &1.question.max_xp, "grade" => %{ "grade" => &1.grade, "adjustment" => &1.adjustment, - "comment" => &1.comment + "comment" => &1.comment, + "xp" => &1.xp, + "xpAdjustment" => &1.xp_adjustment } } end @@ -213,7 +222,11 @@ defmodule CadetWeb.GradingControllerTest do conn = post(conn, build_url(answer.submission.id, answer.question.id), %{ - "grading" => %{"adjustment" => -10, "comment" => "Never gonna give you up"} + "grading" => %{ + "adjustment" => -10, + "comment" => "Never gonna give you up", + "xpAdjustment" => -10 + } }) assert response(conn, 200) == "OK" @@ -221,7 +234,7 @@ defmodule CadetWeb.GradingControllerTest do end @tag authenticate: :staff - test "invalid param fails", %{conn: conn} do + test "invalid adjustment fails", %{conn: conn} do %{answers: answers} = seed_db(conn) answer = List.first(answers) @@ -235,6 +248,21 @@ defmodule CadetWeb.GradingControllerTest do "adjustment must make total be between 0 and question.max_grade" end + @tag authenticate: :staff + test "invalid xp_adjustment fails", %{conn: conn} do + %{answers: answers} = seed_db(conn) + + answer = List.first(answers) + + conn = + post(conn, build_url(answer.submission.id, answer.question.id), %{ + "grading" => %{"xpAdjustment" => -9_999_999_999} + }) + + assert response(conn, 400) == + "xp_adjustment must make total be between 0 and question.max_xp" + end + @tag authenticate: :staff test "staff who isn't the grader of said answer fails", %{conn: conn} do %{mentor: mentor, answers: answers} = seed_db(conn) @@ -308,6 +336,7 @@ defmodule CadetWeb.GradingControllerTest do insert(:programming_question, %{ assessment: mission, max_grade: 200, + max_xp: 1000, display_order: 4 - index }) end ++ @@ -315,6 +344,7 @@ defmodule CadetWeb.GradingControllerTest do insert(:mcq_question, %{ assessment: mission, max_grade: 200, + max_xp: 1000, display_order: 1 }) ] @@ -330,6 +360,8 @@ defmodule CadetWeb.GradingControllerTest do insert(:answer, %{ grade: 200, adjustment: -100, + xp: 1000, + xp_adjustment: -500, question: question, submission: submission, answer: diff --git a/test/cadet_web/controllers/user_controller_test.exs b/test/cadet_web/controllers/user_controller_test.exs index 12f4ca0b9..8495073ad 100644 --- a/test/cadet_web/controllers/user_controller_test.exs +++ b/test/cadet_web/controllers/user_controller_test.exs @@ -20,9 +20,21 @@ defmodule CadetWeb.UserControllerTest do question = insert(:question, %{assessment: assessment}) submission = - insert(:submission, %{assessment: assessment, student: user, status: :submitted}) - - insert(:answer, %{question: question, submission: submission, grade: 50, adjustment: -10}) + insert(:submission, %{ + assessment: assessment, + student: user, + status: :submitted, + xp_bonus: 100 + }) + + insert(:answer, %{ + question: question, + submission: submission, + grade: 50, + adjustment: -10, + xp: 20, + xp_adjustment: -10 + }) not_submitted_assessment = insert(:assessment, is_published: true) not_submitted_question = insert(:question, assessment: not_submitted_assessment) @@ -47,6 +59,7 @@ defmodule CadetWeb.UserControllerTest do expected = %{ "name" => user.name, "role" => "#{user.role}", + "xp" => 110, "grade" => 40, "maxGrade" => question.max_grade } @@ -194,7 +207,13 @@ defmodule CadetWeb.UserControllerTest do |> json_response(200) |> Map.delete("story") - expected = %{"name" => user.name, "role" => "#{user.role}", "grade" => 0, "maxGrade" => 0} + expected = %{ + "name" => user.name, + "role" => "#{user.role}", + "grade" => 0, + "maxGrade" => 0, + "xp" => 0 + } assert expected == resp end diff --git a/test/factories/assessments/question_factory.ex b/test/factories/assessments/question_factory.ex index 30a87647e..ca8277f27 100644 --- a/test/factories/assessments/question_factory.ex +++ b/test/factories/assessments/question_factory.ex @@ -17,6 +17,7 @@ defmodule Cadet.Assessments.QuestionFactory do %Question{ type: :programming, max_grade: 10, + max_xp: 100, assessment: build(:assessment, %{is_published: true}), library: library, grading_library: Enum.random([build(:library), library]), @@ -42,6 +43,7 @@ defmodule Cadet.Assessments.QuestionFactory do %Question{ type: :mcq, max_grade: 10, + max_xp: 100, assessment: build(:assessment, %{is_published: true}), library: build(:library), grading_library: Enum.random([build(:library), library]), diff --git a/test/support/seeds.ex b/test/support/seeds.ex index f950b0211..1ecda4fdb 100644 --- a/test/support/seeds.ex +++ b/test/support/seeds.ex @@ -79,7 +79,8 @@ defmodule Cadet.Test.Seeds do insert(:programming_question, %{ display_order: id, assessment: assessment, - max_grade: 200 + max_grade: 200, + max_xp: 1000 }) end) @@ -88,7 +89,8 @@ defmodule Cadet.Test.Seeds do insert(:mcq_question, %{ display_order: id, assessment: assessment, - max_grade: 40 + max_grade: 40, + max_xp: 500 }) end) @@ -103,6 +105,7 @@ defmodule Cadet.Test.Seeds do Enum.map(programming_questions, fn question -> insert(:answer, %{ grade: 200, + xp: 1000, question: question, submission: submission, answer: build(:programming_answer) @@ -115,6 +118,7 @@ defmodule Cadet.Test.Seeds do Enum.map(mcq_questions, fn question -> insert(:answer, %{ grade: 40, + xp: 500, question: question, submission: submission, answer: build(:mcq_answer)