Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: SCORM
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      32294

      Description

      After running uni tests I noticed some strange failures in mod/scorm,
      seems there is a big scary hack in mod/scorm/locallib.php function scorm_format_date_time($datetime)...

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          yeah - that ugly stuff is needed to convert SCORM 2004 time date format like this: P1DT1H2M32.1S to a human readable format - more background info on MDL-18835

          not really a hack - more a translation of SCORM data - although the tests (written for it) shouldn't be failing.

          • interesting that the tests are failing - I'm getting pretty busy this week but I'll see if I can have a closer look
          Show
          Dan Marsden added a comment - yeah - that ugly stuff is needed to convert SCORM 2004 time date format like this: P1DT1H2M32.1S to a human readable format - more background info on MDL-18835 not really a hack - more a translation of SCORM data - although the tests (written for it) shouldn't be failing. interesting that the tests are failing - I'm getting pretty busy this week but I'll see if I can have a closer look
          Hide
          Dan Marsden added a comment -

          although thinking on that some more - I suppose the function could return a standard timestamp that we use another function to convert to human readable format.......

          Show
          Dan Marsden added a comment - although thinking on that some more - I suppose the function could return a standard timestamp that we use another function to convert to human readable format.......
          Hide
          Petr Škoda added a comment -

          Hmm, it looks like a duration (seconds from start to end), not "date/time notation" as noted in the function description. Why is is called scorm_format_date_time()?
          If that function does dates it would normally convert it to unix timestamp and use our string formatting, but I guess this is not the case. Sorry for the confusion.

          Our standard way is to do format_time(), so I guess it could work if we just get then rounded number of seconds from the SCORM format and use standard API.
          We could also add the milliseconds if really necessary to core API.

          To David: there is some old caching in format_time() that should not be there because the string subsystem should be a lot faster now, I suppose we could use the second params for milliseconds now instead...

          Show
          Petr Škoda added a comment - Hmm, it looks like a duration (seconds from start to end), not "date/time notation" as noted in the function description. Why is is called scorm_format_date_time()? If that function does dates it would normally convert it to unix timestamp and use our string formatting, but I guess this is not the case. Sorry for the confusion. Our standard way is to do format_time(), so I guess it could work if we just get then rounded number of seconds from the SCORM format and use standard API. We could also add the milliseconds if really necessary to core API. To David: there is some old caching in format_time() that should not be there because the string subsystem should be a lot faster now, I suppose we could use the second params for milliseconds now instead...
          Hide
          Dan Marsden added a comment -

          I've renamed the functions to scorm_format_duration and fixed the tests so they pass, - still need to rename the tests that are run with the new function name.

          I'm not sure if there's much point in changing to use format_time - it seems like it will cause extra effort for it to be converted to seconds then back to minutes/days/years etc

          Show
          Dan Marsden added a comment - I've renamed the functions to scorm_format_duration and fixed the tests so they pass, - still need to rename the tests that are run with the new function name. I'm not sure if there's much point in changing to use format_time - it seems like it will cause extra effort for it to be converted to seconds then back to minutes/days/years etc
          Hide
          Dan Marsden added a comment -

          renamed tests - closing - thanks Petr!

          Show
          Dan Marsden added a comment - renamed tests - closing - thanks Petr!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: