Skip to content
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.

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

fix(Quiz Attempt Submitted): Replace raw score with raw grade. #489

Merged
merged 4 commits into from
Apr 10, 2019

Conversation

BrendanHalley
Copy link
Contributor

Description
The raw score now returned in the xAPI statement is the grade for the attempt. This fixes the issue where scaling was wrong because the matching fields are being used for max and min.

In the past successfully completed questions were returned as the raw value. This change removes no data as it's already in the answered statements for each question.

Related Issues

PR Type

  • Fix

@garemoko
Copy link
Contributor

garemoko commented Apr 1, 2019

Are you able to explain the difference between the two fields i.e. the one currently being used for the score vs. the one that you are changing to?

@BrendanHalley
Copy link
Contributor Author

Old: Score based on a total of correctly answered questions. Moodle defaults to 1 per question.

New: Grade from quiz attempt - Moodle handles all logic based around grading and scaling the grade. This is used for marking so seems most appropriate to be used in this context in the statement.

@garemoko
Copy link
Contributor

garemoko commented Apr 1, 2019

That makes sense to me.

Copy link
Member

@ryasmi ryasmi left a comment

Choose a reason for hiding this comment

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

Thanks @BrendanHalley, looks good and thanks for updating the tests. Since I don't know Moodle very well, I'm going to have to trust you and the community on this one.

@ryasmi ryasmi changed the title Replace raw score with raw grade on quiz attempt submitted fix(Quiz Attempt Submitted): Replace raw score with raw grade. Apr 10, 2019
@ryasmi
Copy link
Member

ryasmi commented Apr 10, 2019

Hmmm this seems to be failing tests now @BrendanHalley. @garemoko do you think this might have been affected by your changes since I just merged those?

@BrendanHalley
Copy link
Contributor Author

@ryansmith94 The change's merged in include a new test that's missing some data that this pull request requires. I'll fix it up now :)

@BrendanHalley
Copy link
Contributor Author

@ryansmith94 All sorted

@ryasmi
Copy link
Member

ryasmi commented Apr 10, 2019

Wow that was fast work, thanks @BrendanHalley 👍

@ryasmi ryasmi merged commit f7bdd5e into xAPI-vle:master Apr 10, 2019
@ryasmi ryasmi added the fix label Apr 10, 2019
@ryasmi ryasmi self-assigned this Apr 10, 2019
@HT2Bot
Copy link
Member

HT2Bot commented Apr 10, 2019

🎉 This PR is included in version 4.2.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants