moodle-logstore_xapi
moodle-logstore_xapi copied to clipboard
The mere installation presence of this plugin messes up the moodle timezone
Description
- This plugin messes up the timezones because it resets it here for unknown reasons:
https://github.com/xAPI-vle/moodle-logstore_xapi/blob/master/src/transformer/utils/create_timestamp.php#L20
Version
- {{branch}} at {{commit}} on {{version - found in your copy of the VERSION file}}
Steps to reproduce the bug
- During the moodle boostrap which ends up loading this page it sets the timezone which is then incorrect for part or all of the remainder of the request. Under some conditions it will be corrected later on. Having the timezone different in different parts of the request creates some very odd edge cases bugs in other places.
Expected behaviour
- It should not affect the moodle timezone
Actual behaviour
- It sets the timezone to london
Server information
- na
Client information
- OS: na
- Browser: na
I can't seem to find a relevant reason as to why the time is hard coded. If it had been set to UTC that'd make a bit more sense. Looking at the logs over the last couple of years, the activity timestamp is in fact UTC+1 (UK time).
This raises the question that without a requirement from the xAPI spec, what is the preferred method?
The next question is, if changing this causes issues with already stored statements?
@ryansmith94 @garemoko @brendanheywood thoughts?
Honestly don’t know and my interest is fairly low. The client who hit this wasn’t even using this plug-in it was just installed but idle. I would say it is kinda deprecated now that moodle core has its own xapi api. No matter what this plug-in shouldn’t touch Moodles settings.
@brendanheywood not to worry. This plugin is getting cleaned up to go into core. The current xapi implementation (in core) is the reverse of this in that it takes xAPI statements and turns them into events (while giving a round-about way to retrieve progress via Moodle - rather than an LRS).
There are rumblings that the xAPI 2.0 spec is going to require statements be in UTC, so I'm going to push forward with going that route.