Max grade of 0 causes migration failure

[ericmerrill]

If a mod_hvp has a maximum grade of 0 set, then the migration is run you get:

Warning: Creating default object from empty value in /usr/local/var/moodles/hvpup/moodle/admin/tool/migratehvp2h5p/classes/api.php on line 482
	Exception: Coding error detected, it must be fixed by a programmer: moodle_database::update_record_raw() id field must be specified.

mod_hvp allows activities to have a max grade of 0, which basically makes the grade have no effect in the gradebook (but it is still there), while, at least through the GUI, mod_h5p does not allow a max grade of 0.

So when the duplication is happening, the new module doesn't create a grade item (I haven't checked to confirm that the module is created at all), but api::duplicate_grade_item() assumes the new grade item exists, causing this scenario.

While the actual error from a coding standpoint looks pretty simple to remedy, there is a larger question of what should happen to migrate these. Do we just programmatically do it, allowing it to be 0 points, and then the next time the teacher tries to edit it, it will require them to change it?

[ericmerrill]

One thing to note about the larger Moodle policy point, is that not all activities in Moodle core require the number of points to be more than 0. It's only if they use the standard mod form elements. Take quiz - you can set the points to 0, and then the grade item disappears from the gradebook, just like the old mod_hvp.

[sarjona]

Hi Eric!
Thanks for your comment. As you'll see, the approach I've followed for considering this scenario is to set "Grade type" to "None" when mod_hvp.maxgrade is set to 0 (so user grades won't appear in gradebook). So, for that reason, in that case, grade_grades are not migrated (because they are set to 0). However, attempts information is still migrated in that case because mod_h5pactivity supports it: attempts will be saved but grades won't be sent to the gradebook.

[ericmerrill]

Sounds good to me.
The lack of grade items seems perfectly fine to me, but one thing that might need to be checked is the (somewhat edge) case of feedback. I don't know if teachers could provide feedback in the GB if points was set to 0 in mod_hvp - and if so, if that would be lost, since I think that might be stored with grade_grades, I can't recall at the moment.
Thanks for working on this!

[sarjona]

Thanks for looking at the patch and pointing this edge scenario.
Although it's possible to define a mod_hvp activity with maxgrade = 0 and then override this information in the grade book and add some feedback, I still feel the best approach when maxgrade is set to 0 is to ignore these grades and lose this information.
However, if anybody can think in a better solution, I'll be more than happy to review it :-)

[ericmerrill]

I'm very hesitant to delete data, even if very niche (maybe even more so, since it's harder for an admin to notice), especially related to teacher grading/feedback. Personally, I think that we should put an error in the output (while still continuing the conversion) and either:

1. Hide the activity, even if delete was requested in the command line
2. Move the feedback to a manual grade item

[sarjona]

Hi Eric!
Thanks for sharing your opinion about this. I've added one extra commit in order to:

• Let admins know about this situation (when feedback is given but grades won't be migrated because maximum grade for the original mod_hvp activity is set to 0).
• Change temporarily the value for the keeporiginal setting from DELETE to HIDDEN (and warn about that too).

With the new behaviour, no crucial information should be removed (the mod_hvp activity will be deleted only if it has no feedback overridden).

[ericmerrill]

Thanks Sara, sounds great!