-
Improvement
-
Resolution: Fixed
-
Minor
-
2.7.4, 2.8.2, 2.9
-
MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
-
MOODLE_29_STABLE
-
Having the setting for "Time limit" using a text form element is not very good UI, it should use a duration element.
But there are 2 obstacles
- currently $lesson->maxtime is in minutes not seconds (and if you look in the code there is many places where it is multiplicated by 60 before being used, because minutes is a very bad unit for calculations !)
- there is a separate boolean setting $lesson->timed to decide whether the lesson is timed or not. Other module use a zero duration value.
So the question how far should we go about refactoring this area
- extreme solution : get rid of the minutes unit and decide maxtime is now expressed in seconds, multiply all maxtimes by 60 in the database for existing records during upgrade. Set maxtime = 0 for lessons where timed = false and suppress the timed field. This would allow full use of the duration form element and simplify the code, but will break compatibility.
- non invasive solution: only use duration element for the UI but in the code do the back and forth conversion to still have maxtime in minutes and timed boolean. This would complicate the code but not break compatibility.