Moodle

Course Reports - Export to Excel Time Issue

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Duplicate
  • Affects Version/s: 1.9.4
  • Fix Version/s: None
  • Component/s: Course
  • Labels:
    None
  • Database:
    MySQL
  • Difficulty:
    Easy
  • Affected Branches:
    MOODLE_19_STABLE

Description

When using the Course Reports functionality, exporting in Excel does not account for the timezone of the user. Exporting to a text file and showing the data on the screen shows the correct time. We have only verified that this is an issue in 1.9.4, and have not tested other versions.

Instructions to replicate:
1. Login as a Site Administrator
2. From the Site Administrator Nav Menu, click on Reports -> Logs
3. Run any report that contains data, make sure the "Display on Page" option is selected
4. Run the same report with the "Download in Excel format" option selected.
5. Compare the values of the Time column. The values of the Excel format are set to GMT time, while the values of the On Page report are correctly adjusted to the user's time zone.
6. (Optional) Run the same report with "Download in text format". These values are correct and consistent with the On Page report format.

This hopefully is a pretty quick fix. Thanks in advance!

Issue Links

Activity

Hide
Mihai Sucan added a comment -

I have made a patch to fix the problem reported. To apply the patch please use:

cd moodle
patch p0 < patchMDL-18394.diff

The patch is made against the current cvs MOODLE_19_WEEKLY.

The reports use the userdate() function to display the log date and time in the html page and in the text version of the report. However, in the print_log_xls() function the UNIX timestamp is simply adjusted to be an "excel timestamp" (starting from 1/1/1900). Based on reading some info about excel time, I couldn't find any info about their support for timezones. I tried OpenOffice Spreadsheet > format cell, still no notion of timezones.

Thus, my solution is to do something similar to what's done inside the userdate() function. I adjust the timestamp to include the user-defined timezone (or server timezone).

Additionally, I have changed the format of the date to be the same as in the html/text reports.

Show
Mihai Sucan added a comment - I have made a patch to fix the problem reported. To apply the patch please use: cd moodle patch p0 < patchMDL-18394.diff The patch is made against the current cvs MOODLE_19_WEEKLY. The reports use the userdate() function to display the log date and time in the html page and in the text version of the report. However, in the print_log_xls() function the UNIX timestamp is simply adjusted to be an "excel timestamp" (starting from 1/1/1900). Based on reading some info about excel time, I couldn't find any info about their support for timezones. I tried OpenOffice Spreadsheet > format cell, still no notion of timezones. Thus, my solution is to do something similar to what's done inside the userdate() function. I adjust the timestamp to include the user-defined timezone (or server timezone). Additionally, I have changed the format of the date to be the same as in the html/text reports.
Hide
Dan Poltawski added a comment -

Hi Mihai,

Thanks! Regarding:
>Additionally, I have changed the format of the date to be the same as in the html/text reports.

Is this the change to the change to log_excel_date_format language string? Have you checked if this language string is used anywhere else? (Try searching for that string).

I haven't checked but I suspect it is, and I think for minimal impact it'd be best not to do this change. Some users might do clever stuff with the generated reports in other places so changing the format of the generated excel times might cause them problems (although having said that, correcting the timezone might also do that).

Anyway great work - I am on the road this weekend but hope to look at this more on monday.

Show
Dan Poltawski added a comment - Hi Mihai, Thanks! Regarding: >Additionally, I have changed the format of the date to be the same as in the html/text reports. Is this the change to the change to log_excel_date_format language string? Have you checked if this language string is used anywhere else? (Try searching for that string). I haven't checked but I suspect it is, and I think for minimal impact it'd be best not to do this change. Some users might do clever stuff with the generated reports in other places so changing the format of the generated excel times might cause them problems (although having said that, correcting the timezone might also do that). Anyway great work - I am on the road this weekend but hope to look at this more on monday.
Hide
Mihai Sucan added a comment -

I did grep for log_excel_date_format and I didn't find any other places where it's used. Thus, it seems to be a fairly acceptable change.

I know what you mean about people already relying on the same format being used. I leave this choice to you. I did the change for consistency reasons.

Show
Mihai Sucan added a comment - I did grep for log_excel_date_format and I didn't find any other places where it's used. Thus, it seems to be a fairly acceptable change. I know what you mean about people already relying on the same format being used. I leave this choice to you. I did the change for consistency reasons.
Hide
David Mudrak added a comment -

I just spotted this bug as well. I can confirm the bug is still present in 1.8, 1.9 and HEAD. Mihai did a great work, thanks! Some additional notes:

  • course/lib.php is not the only place using XLS export. We will need to check all other places generating date/times using lib/excellib.class.php and inform about this issue in GDF so contrib developers are warned
  • ODS export works fine as it apparently uses standard UNIX timestamp
  • I suggest not to change $string['log_excel_date_format'], admins and lang pack maintainers can easily modify it for their needs
Show
David Mudrak added a comment - I just spotted this bug as well. I can confirm the bug is still present in 1.8, 1.9 and HEAD. Mihai did a great work, thanks! Some additional notes:
  • course/lib.php is not the only place using XLS export. We will need to check all other places generating date/times using lib/excellib.class.php and inform about this issue in GDF so contrib developers are warned
  • ODS export works fine as it apparently uses standard UNIX timestamp
  • I suggest not to change $string['log_excel_date_format'], admins and lang pack maintainers can easily modify it for their needs
Hide
David Mudrak added a comment -

I just realized the DST (day light saving) is not taken into account if a user has "server local time" as her timezone in the profile. Of course it is not (how could be?) but gosh, it took me ages

Show
David Mudrak added a comment - I just realized the DST (day light saving) is not taken into account if a user has "server local time" as her timezone in the profile. Of course it is not (how could be?) but gosh, it took me ages
Hide
Dan Poltawski added a comment -

Looks like this has just been fixed in MDL-14934!

Show
Dan Poltawski added a comment - Looks like this has just been fixed in MDL-14934!

People

Vote (3)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: