danmarsden/moodle-local_recompletion

Avoid that the reset error array gets blown up with empty error messages

Closed this issue · 3 comments

abias commented

Hi Dan,

I think I found a glitch on https://github.com/danmarsden/moodle-local_recompletion/blob/MOODLE_401_STABLE/classes/task/check_recompletion.php#L276

In the case when $config->recompletiontype is empty, the recompletionnotenabledincourse string gets added to the error array on https://github.com/danmarsden/moodle-local_recompletion/blob/MOODLE_401_STABLE/classes/task/check_recompletion.php#L260. However, in this case, the function is not returning the error array directly. Instead, the code flow is continued.

Then, on line 279, you are checking this:
if (!empty($errors)) {
In this case, the error array gets blown up with empty error messages from all plugins. This feels wrong.

I think you should either change line 279 to
if (!empty($error)) {
or you should add
return $errors;
directly after line 260.

I would leave this decision up to you and haven't composed a PR because of that.

Cheers,
Alex

danmarsden commented

thanks Alex - yes, this is a duplicate of #145 - hopefully I'll get to that sometime... but pull requests are always welcome :-)

abias commented

Ah, shame on me. I completely forgot to check the existing issues. On the other hand, it's great that you are aware of this issue already.

danmarsden commented

all good - you traced it further than I have so thanks for adding the details! :-)