bozoh / moodle-mod_simplecertificate

It's NOT RECOMMENDED to install version below 2.2.6 (MOODLE_31), due a security issues See more in README
17 stars 44 forks source link

fatal error creating new module #269

Closed jeremyvanlier closed 1 month ago

jeremyvanlier commented 1 month ago

Describe the bug When creating a new instance in a course, and not providing secondpagetext, we're faced with a fatal error.

Moodle Version(s) Moodle 4.4.1 (PHP 8.1.29) Module version 2024051101

To Reproduce Steps to reproduce the behavior: Create new instance provide name provide mandatory certificate text Press Save

Expected behavior Return to course page or same page, depending on button

Screenshots {2EFE4C20-4AEF-4393-8A22-8CC5CFE7F61E}

The problem is here: 450 if (isset($formdata->secondpagetext['text'])) { 451 $update->secondpagetext = $formdata->secondpagetext['text']; 452 if (!isset($formdata->secondpagetextformat)) { 453 $update->secondpagetextformat = $formdata->secondpagetext['format']; 454 } 455 unset($formdata->secondpagetext); 456 }

Reason being, 'text'is missing when we dump the data JUST before it's sent to populate_simplecertificate_instance(), see screenshot above. Your verification here should be done using
450 if (is_array($formdata->secondpagetext['text'])) {

or any other way taking into account the fact you're only focussing on 'text'to exist. This may be a problem that only exists in Moodle 4.4.1, it may also have other reasons. Fact is still your check is incomplete and prone to failure (personally, I'd either write a helper to do this default unformatting, use Moodles internals to do so or write a check that checks for is_array as well as the existince of both possible keys 'text'and 'format'... although that;'s more personal preference, I always like to be ahead of "all" possible failures. This also means: if it's not a strinf as we expect.... use a defaulted empty value)

jeremyvanlier commented 1 month ago

Hi...

I have created a branch with compatibility with Moodle 4.4+. Please test it and give me your feedback: https://github.com/bozoh/moodle-mod_simplecertificate/tree/tk_moodle404_compatibility

Saludos

This seems to also fix this issue.