Thanks for reporting that, Aaron.

This should have a set of test instructions before it is peer reviewed.

If you are interested in what is (or should be) checked during peer review, see http://docs.moodle.org/dev/Peer_reviewing_checklist

Hi Aaron,

These changes look spot on thanks.
The only thing I noticed was re: coding style, should be } else { rather than }*\n*else {.
Otherwise spot on, add testing instructions and put up for integration when ready.
If you're unable to (sorry I loose track) just comment when ready and I'll put it up for you.

Cheers
Sam

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Hi Aaron,

I have some concerns about removing the require_login() call from blocks/completionstatus/details.php, I think that this means that if a user gets logged out they will be presented with an error message rather than redirected to the login screen to see what they want.

Was this change by design?

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Hi Dan,

Great spotting, an apologies for not getting you an answer last week. I have readded the require_login check (but without the course requirement).

Thanks,
Aaron

Thanks Aaron, i've integrated this now

Test result: Errors during testing

I'm seeing a string error on the report...

Invalid get_string() identifier: 'aggregateall' or component 'completion'. Perhaps you are missing $string['aggregateall'] = ''; in lang/en/completion.php?
line 6727 of \lib\moodlelib.php: call to debugging()
line 7388 of \lib\moodlelib.php: call to core_string_manager->get_string()
line 203 of \blocks\completionstatus\details.php: call to get_string()

Manually changing the userid as a GET parameter didn't seem to make any change. I only saw the report for the current user. I'm not sure if that is good or if I should have seen a warning/error.

When viewing the Course report as a teacher, there was an error repeated (probably once for each student).

Notice: Undefined variable: courseid in D:\xampp\htdocs\moodle_master_test_mysql\lib\completionlib.php on line 202

The get_string call I think is an existing problem.

But the second issue is from this patch and suggests its been tested down that avenue. Combined with the fact that this is last second, i'm going to have to revert this.

Fix misnamed variable and made testing instructions clearer.

Integrated, thanks Aaron

Test result: Success!

Tested in Master only.

I was able to switch users as a teacher, but not as a student.

I found the string error previously reported and I also found that the navigation is broken on the report page.

Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!