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

Not sending correct grade because the last-grade is seen as the current-grade #51

Open
SAHellauer opened this issue Sep 11, 2017 · 2 comments

Comments

@SAHellauer
Copy link

SAHellauer commented Sep 11, 2017

Please review the following line:

if ( intval($grade) == $user->lastgrade ) {

Should the above code be an exact check and then we store the exact grade being sent in the db?

Use Case

  1. A quiz is worth 10 points.
  2. Student gets a 9.2 on their first attempt.
  3. Cron script runs, sends the float value and then sets the last-grade to 9 in the db.
  4. Student gets a 9.8 on their second attempt.
  5. Cron script runs and does not send the float value, because the check in the code referenced above considers 9.2 and 9.8 as the same.

Or at the very least could we round the grade and check that way?

$roundedGrade = round($grade);

if (intval($roundedGrade) == $user->lastgrade) {
    continue;
}

And then store the rounded grade instead.

$DB->set_field('local_ltiprovider_user', 'lastgrade', intval($roundedGrade), array('id' => $user->id));

This solution still does not provide an exact check. 9.2 and 9.3 would still be see as 9. But I believe it would be a little more accurate.

@SAHellauer SAHellauer changed the title Not sending grades because the last-grade equals the current grade Not sending correct grade because the last-grade is seen as the current-grade Sep 11, 2017
@SAHellauer
Copy link
Author

SAHellauer commented Sep 12, 2017

You could also use $grade->dategraded and not check/store lastgrade at all.

https://github.com/jleyva/moodle-local_ltiprovider/compare/MOODLE_29_STABLE...SAHellauer:grade_sync?expand=1

@jleyva
Copy link
Owner

jleyva commented Sep 13, 2017

Hi, yes, you are right. This is something that has been there since the beginning and I totally forget about it.
I suppose this happened because I was considering that Moodle only returns grades from 0 to 100 so I created the lastgrade field as int, but that's not true so this needs addressing in someway (ideally changing the field type and checks)

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

No branches or pull requests

2 participants