Moodle

Update Course Completion report to include more useful information

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.2.4, 2.3.2
  • Fix Version/s: 2.4
  • Component/s: Course completion
  • Labels:
    None
  • Testing Instructions:
    Hide

    Enable course completion site-wide in advanced settings.
    Create a new course with completion enabled.
    Enrol multiple users in the course.
    Apply some criteria to the course view the Completion Tracking course settings page.
    Complete some of the criteria for various users (easiest way to do this is to use the Self Completion criteria and course block).
    View the completion course report.

    Expected:
    Report should look the same as before.
    When hovering over ticks it should show the time the user completed that criteria.
    Export links in footer should work, and exported reports should include times for each criteria completed. Exported reports should also contain a header row that matches the user data.

    Show
    Enable course completion site-wide in advanced settings. Create a new course with completion enabled. Enrol multiple users in the course. Apply some criteria to the course view the Completion Tracking course settings page. Complete some of the criteria for various users (easiest way to do this is to use the Self Completion criteria and course block). View the completion course report. Expected: Report should look the same as before. When hovering over ticks it should show the time the user completed that criteria. Export links in footer should work, and exported reports should include times for each criteria completed. Exported reports should also contain a header row that matches the user data.
  • Affected Branches:
    MOODLE_22_STABLE, MOODLE_23_STABLE
  • Fixed Branches:
    MOODLE_24_STABLE
  • Pull from Repository:
  • Pull Master Branch:
  • Pull Master Diff URL:

Description

I have taken Matt's code and added to it to do the following:

  • include the course completed date, in both the on screen report and the csv export.
  • show criteria completion dates in csv
  • make all completion dates the same formats
  • use moodle_url and csv_reader
  • add header rows to the csv!
  • fix activity criteria display logic
  • changed csv file name to be correct

Issue Links

is a QA test written for

Improvement - An enhancement to an existing Moodle feature. MDL-35556 Improve user completion data permission checking

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
will help resolve

Improvement - An enhancement to an existing Moodle feature. MDL-35135 Course completion status export CSV should have a header row

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Porritt added a comment -

The updated plugin code

Show
Matt Porritt added a comment - The updated plugin code
Hide
Permalink
Sam Hemelryk added a comment -

Hi Aaron,

Things look really good thanks.
Just a couple of trivial notes, up to you if you clean them up before putting it up for integration. They're not show stoppers but are perhaps worth cleaning up.

  1. There are a couple of places were you've ended up with two empty lines separating code. We normally avoid this, simply to keep the code uniform and make it easies to read.
  2. There are a couple of ternary operators being used. In cases where it is very simple (lines 205, and 227 as exmaples) that is tolerated, however as soon as they start getting complex (line 649) we prefer them to be converted into proper if..else statements.

Otherwise looks spot on thanks.

Cheers
Sam

Show
Sam Hemelryk added a comment - Hi Aaron, Things look really good thanks. Just a couple of trivial notes, up to you if you clean them up before putting it up for integration. They're not show stoppers but are perhaps worth cleaning up. There are a couple of places were you've ended up with two empty lines separating code. We normally avoid this, simply to keep the code uniform and make it easies to read. There are a couple of ternary operators being used. In cases where it is very simple (lines 205, and 227 as exmaples) that is tolerated, however as soon as they start getting complex (line 649) we prefer them to be converted into proper if..else statements. Otherwise looks spot on thanks. Cheers Sam
Hide
Permalink
Matt Porritt added a comment -

Hi Aaron,
The other thing I have noticed in testing is the current code that handles paging doesn't do large number of users (2000+ in my testing environment) well. It is showing 86 pages before the "next" link appears, in my example.
Is it worth changing it to use the paging_bar output renderer function, as used in the partipant view (/user/index.php)?
I'm happy to make the changes if you like.

Cheers,
Matt P

Show
Matt Porritt added a comment - Hi Aaron, The other thing I have noticed in testing is the current code that handles paging doesn't do large number of users (2000+ in my testing environment) well. It is showing 86 pages before the "next" link appears, in my example. Is it worth changing it to use the paging_bar output renderer function, as used in the partipant view (/user/index.php)? I'm happy to make the changes if you like. Cheers, Matt P
Hide
Permalink
Aaron Barnes added a comment -

Hi guys,

I've fixed your issues Sam.

I had a look at the paging bar Matt talks about however using the same code as user/index.php would require implementing the report in flexible_table() to do things properly - which is just too big a job for me to look at right now.

You could just implement the paging part (as opposed to the initials paging part) and avoid flexible_table() I guess. Feels kinda like cheating though

Cheers,
Aaron

Show
Aaron Barnes added a comment - Hi guys, I've fixed your issues Sam. I had a look at the paging bar Matt talks about however using the same code as user/index.php would require implementing the report in flexible_table() to do things properly - which is just too big a job for me to look at right now. You could just implement the paging part (as opposed to the initials paging part) and avoid flexible_table() I guess. Feels kinda like cheating though Cheers, Aaron
Hide
Permalink
Matt Porritt added a comment -

Hi Aaron,
I cheated on my install with the paging bar...
I'm doing some work with course completions for my companies Moodle, if I end up changing to using the flexible_table() I'll supply the update.
Otherwise I'm happy

Matt P

Show
Matt Porritt added a comment - Hi Aaron, I cheated on my install with the paging bar... I'm doing some work with course completions for my companies Moodle, if I end up changing to using the flexible_table() I'll supply the update. Otherwise I'm happy Matt P
Hide
Permalink
Aparup Banerjee added a comment -

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

Show
Aparup Banerjee added a comment - 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
Hide
Permalink
Dan Poltawski added a comment -

Thanks Aaron, i've integrated this now.

Show
Dan Poltawski added a comment - Thanks Aaron, i've integrated this now.
Hide
Permalink
David Monllaó added a comment -

Tested in master, it passes. The time is displayed and exported in the .csv files, headers as well

FYI, I've seen that in Linux + Chrome the download .csv links appears centered, but the <li> dots appears in the left side.

Show
David Monllaó added a comment - Tested in master, it passes. The time is displayed and exported in the .csv files, headers as well FYI, I've seen that in Linux + Chrome the download .csv links appears centered, but the <li> dots appears in the left side.
Hide
Permalink
Dan Poltawski added a comment -

Hurray!

You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

Show
Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: