search

danmarsden / moodle-local_recompletion

Allows course recompletion
29 stars 45 forks source link

check_recompletion::reset_user() generates a "recompletionnotenabledincourse" error #145

Closed mhughes2k closed 5 months ago

mhughes2k commented 9 months ago

I've noticed that in the case of using the "Modify course completion dates" page, using the "Reset all completion for selected" users in the case where recompletion is not configured for the course, the "recompletionnotenabledincourse" error will always be created.

This is not strictly accurate (in my view) as there is no requirement for it to be enabled, say for instance the plugin is only being used for manually resettting completion information.

This would suggest that the may be a difference between the recompletype values in that a "null" or empty value for this doesn't actually indicate that is disabled, but it's in an inderminate "off" state, vs an actual "0" value indicating that someone has actually configured it to be "off".

This came up as I was re-using the manual recompletion process in another tool, and found that $errors was always non-empty, but the reset of completion had been successfully done.

danmarsden commented 9 months ago

Thanks Michael - the behavior of that has changed a bit recently now that we have multiple schedule types - for the recompletion settings to work the completion type must not be empty - it must be set to period/schedule or manual - this forces the teacher to specifically choose what behavior should occur when the recompletion is being triggered - if that's not the case and I'm missing something else, let me know!

mhughes2k commented 9 months ago

The "recompletionnotenabledincourse" error message just seems to get stacked on to the $errors array, but there's nothing that then blocks the reset being done on this fact in the reset_user() function.

So if you were looking to force a configuration to be in place, it doesn't seem to do this when using the "interactive" reset process.

reset_completions() still gets called, and it's only after the process has run can the calling script examine the returned errors, it'll find that it's not-empty (so there was an error) but also the reset has been done, albeit with some implict / undefined configuration.

danmarsden commented 9 months ago

ah - yeah I see what you mean... I think we should probably block the reset (and maybe hide the link on the page) when it's empty/not enabled rather than showing that error - pull requests always welcome :-)

danmarsden commented 5 months ago

this one should be sorted now - but let me know if you're still getting this with the latest code.