Project

General

Profile

Bug #6884

Exercise time limits change when including it in learning path

Added by Yannick Warnier about 6 years ago. Updated about 6 years ago.

Status:
Bug resolved
Priority:
High
Category:
Exercises
Target version:
Start date:
20/12/2013
Due date:
20/12/2013
% Done:

100%

Estimated time:
2.00 h
Spent time:
Complexity:
Challenging
SCRUM pts - complexity:
?

Description

There's an issue with the current flow of creating an exercise with time limits and including it in a learning path.

The first data is that the exercise.class.php::save() function assumes the data set as the object comes from a form, readable by a human, and that the incoming date is in local time, and that it has to convert it to UTC time before saving it to the database.

The second data is that, when adding an exercise to a learning path, we use the exercise::read() function (which reads data straight from the database), then use the save() function (almost straight away after read()).

In this latter situation, the data saved does not come from a form with a localized time. It comes straight in UTC, which means that, when processed by save(), it is incremented (or subtracted) by the number of hours that you have in difference with UTC!!!

One solution could be to update the time manually inside the LP to counterweight the change, but that would mean a double operation for nothing.

Another solution, which seems much better to me, would be to convert the time from the different forms before calling save(), so the save() function remains safe, so to speak.

A third solution would be to keep two additional attributes in the exercise class: start_time, end_time (the current ones, in UTC), and then start_time_local and end_time_local. This third way would make sure there is no possible error... (right?).

Associated revisions

Revision 3a4c9441 (diff)
Added by Yannick Warnier about 6 years ago

Fix issue with exercise time limits being changed relatively to UTC time difference - refs #6884 refs BT#7165

History

#1

Updated by Yannick Warnier about 6 years ago

The logic to fix it will be the second one: to only change from UTC to the local time when printing a form, something on screen, or (the reverse process) before sending a call to save().

In exercise.class.php, several functions manage the date saving/printing:
  • create_quiz() (inserts start_time and end_time directly to the c_quiz table)
  • is_visible() uses the (UTC) start_time and end_time from the current object attributes and transforms them only when printing user messages
  • processCreation() is called as a save()-caller, so it would be a right place to change the time! (provided everything passes through there)
  • createForm() shows the current time (somehow without adapting to local time)
  • save() (obviously where the local to UTC conversion shouldn't go)
  • read() just reads the data from the database. It doesn't process it in anyway, so by calling read(), you are getting the UTC values
#2

Updated by Yannick Warnier about 6 years ago

  • % Done changed from 10 to 20

From a series of cases in exercice.php, it really looks like the save() function is generally called with the only intention of saving the data that was already there, meaning that having a time change in the save() function would alter time most of the time when doing any kind of change to the exercise.

And... that is verified when changing an exercise's visibility: +5h if I'm at GMT+5... OK, this is crazy now. How could nobody report this in the past!?

#3

Updated by Yannick Warnier about 6 years ago

  • % Done changed from 20 to 70

I proceeded with removing the api_get_utc_datetime() and api_get_local_time() from the save() function, and tested the creation, edition and assignment of several tests to a learning path. As a student, you can see the right expiration dates (local time). In the database, you have the UTC time as expected, after creation and update of the course.

#4

Updated by Yannick Warnier about 6 years ago

  • Status changed from Assigned to Needs testing
  • Assignee changed from Yannick Warnier to Daniel Barreto
  • % Done changed from 70 to 90

In the end that was just that: moving the time change code out of save() and into processForm()! Needs testing. Easy to test out: just try and create an exercise with start and/or end date, then change the visibility or include it in a learning path and check afterwards (editing the exercise) that the time remained the same (it didn't do so, previously).

#5

Updated by Daniel Barreto about 6 years ago

  • Status changed from Needs testing to Bug resolved
  • % Done changed from 90 to 100

Tested and worked fine, the date didn't change

Also available in: Atom PDF