wooclap / moodle-mod_wooclap

Moodle plugin for Wooclap (https://www.wooclap.com/).
2 stars 5 forks source link

Rename wooclap_update_grades function fixes #20. #21

Closed lucaboesch closed 2 months ago

lucaboesch commented 1 year ago

That was reported in #20. This fix here addresses it.

marco-verbeek commented 8 months ago

Hello @lucaboesch 👋🏼

Thank you for your contribution to our plugin!

Moodle's Gradebook API seems to be using different function signatures, which I believe would require more than a rename of the function.

Here are the expected signatures, taken from the Moodle Gradebook API docs:

{$modname}_grade_item_update($modinstance, $grades=NULL)
{$modname}_update_grades($modinstance, $userid=0, $nullifnone=true)

None seem to match the current wooclap_update_grade's signature:

function wooclap_update_grade($wooclapinstance, $userid, $gradeval, $completionstatus) {

The log mentions that declaring one of those but not both will cause broken behaviour; however, I'm having a hard time to reproduce the issue to verify this. Could you provide us with reproduction steps or a video of the issue happening? Don't hesitate to share any additional Moodle documentation that could help us with this issue, that would be really helpful as well.

All the best, Marco

lucaboesch commented 4 months ago

@marco-verbeek, I think I have found a way. When using https://github.com/ndunand/moodle-tool_mergeusers and you merge two users, then the notice "You have declared one of wooclap_grade_item_update and wooclap_update_grades but not both. This will cause broken behaviour" is triggered.

lucaboesch commented 2 months ago

This Pull request is wrong.