Skip to content

Implement XP #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Aug 25, 2018
Merged

Implement XP #232

merged 15 commits into from
Aug 25, 2018

Conversation

@indocomsoft
Copy link
Member

@indocomsoft indocomsoft commented Aug 18, 2018 •
edited
Loading

XP Document

  • each mission will have a max #XP, and XP will scale according to the grade achieved
  • Each quest will have a max #XP, and XP will scale according to the test cases passed.
  • Paths: Fixed #XP max per path, autograded in the Academy
  • Early completion bonuses. Assuming every mission/quest has a flat 100 XP decaying, Decays starting 2 days from opening. 1 XP per hour. No XP after ~6 days.

Implied

  • No bonus xp for quest/path

Implement Endpoint

  • User, add total_xp
  • Grading, add xp, xp_bonus, xp_adjustment
  • Assessment, add max_xp, xp, grade

File issue in other repos

@coveralls
Copy link

coveralls commented Aug 18, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 1711

  • 62 of 62 (100.0%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 99.494%
Totals Coverage Status
Change from base Build 1707: 0.02%
Covered Lines: 786
Relevant Lines: 790
💛 - Coveralls

@indocomsoft
Copy link
Member Author

indocomsoft commented Aug 18, 2018

@ningyuansg will need you to add maxxp attribute to PROBLEM tag in the xml spec.

@indocomsoft indocomsoft force-pushed the xp branch 4 times, most recently from a932e45 to 7b1489d Compare August 20, 2018 14:45
@indocomsoft indocomsoft self-assigned this Aug 20, 2018
@indocomsoft indocomsoft requested a review from tuesmiddt August 20, 2018 15:13
@indocomsoft
Copy link
Member Author

indocomsoft commented Aug 20, 2018

@remo5000 need your input regarding endpoint to retreive XP

@indocomsoft indocomsoft mentioned this pull request Aug 21, 2018
Closed
@indocomsoft indocomsoft changed the title WIP: Implement XP Implement XP Aug 21, 2018
remo5000
remo5000 reviewed Aug 22, 2018
View reviewed changes
lib/cadet_web/views/assessments_view.ex
reading: :reading,
status: &(&1.user_status || "not_attempted"),
maxGrade: :max_grade,
maxXp: :max_xp,
Copy link

@remo5000 remo5000 Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

tuesmiddt
tuesmiddt requested changes Aug 23, 2018
View reviewed changes
Copy link
Contributor

@tuesmiddt tuesmiddt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed everything except tests. Some comments so far.

lib/cadet/assessments/assessment.ex Outdated
field(:max_grade, :integer, virtual: true)
field(:max_xp, :integer, virtual: true)
field(:xp, :integer, virtual: true)
field(:grade, :integer, virtual: true)
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can order together(max_grade, grade, max_xp, xp) or (max_grade, max_xp, grade, xp)

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/cadet/assessments/answer.ex Outdated
|> add_question_type_from_model(params)
|> validate_required(@required_fields)
|> validate_number(:grade, greater_than_or_equal_to: 0.0)
|> validate_number(:grade, greater_than_or_equal_to: 0)
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking, it might be better to add

    |> validate_number(:grade, greater_than_or_equal_to: 0)
    |> validate_number(:xp, greater_than_or_equal_to: 0)

to validate_xp_grade_adjustment_total, rename it, and use it in every changeset where grade/xp/adjustments can be set, including autograding_changeset, since we aren't validating the numeric results of autograder there.

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/cadet/assessments/assessments.ex Outdated
alias Cadet.Autograder.GradingJob
alias Ecto.Multi

@xp_early_submission_bonus 100
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this to @xp_early_submission_max_bonus for clarity

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/cadet/assessments/assessments.ex
if grade do
Decimal.to_integer(grade)
defp decimal_to_integer(decimal) do
if Decimal.decimal?(decimal) do
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something about this tickles me hahahahaha

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙃

lib/cadet_web/controllers/user_controller.ex Outdated
"Only provided for 'Student'"
)

xp(:integer, "Amount of xp. Only provided for 'Student'")
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Amount of xp. Value will be 0 for non-students" or something along those lines.

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/cadet_web/views/assessments_view.ex Outdated
status: &(&1.user_status || "not_attempted"),
maxGrade: :max_grade,
maxXp: :max_xp,
xp: &(&1.xp || 0),
Copy link
Contributor

@tuesmiddt tuesmiddt Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like xp: :xp will suffice since both user_total_grade() and user_total_xp() return default values of 0. Correct me if I'm wrong on this.

Copy link
Member Author

@indocomsoft indocomsoft Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right.

@indocomsoft indocomsoft force-pushed the xp branch 3 times, most recently from 3a41f82 to b20ffce Compare August 24, 2018 16:37
@indocomsoft indocomsoft merged commit b2fb633 into master Aug 25, 2018
@indocomsoft indocomsoft deleted the xp branch August 25, 2018 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@remo5000 remo5000 remo5000 left review comments

@tuesmiddt tuesmiddt tuesmiddt approved these changes

Assignees

@indocomsoft indocomsoft

Labels

critical

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@indocomsoft @coveralls @remo5000 @tuesmiddt