Moodle
  1. Moodle
  2. MDL-34908

External functions: timestamp should be set to a new PARAM_TIMESTAMP?

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1.7, 2.2.4, 2.3.1
    • Fix Version/s: STABLE backlog
    • Component/s: Web Services
    • Labels:
    • Rank:
      43434

      Description

      Review all external description functions, check timestamp are set to PARAM_XXXX (PARAM_TIMESTAMP ?) so then can be checked by external function validation.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          PARAM_FLOAT worries me. Floats are not precise, and there is no guarantee what they are in PHP. Typically they are IEEE double, which gives you more than 32 bits for the mantissa, so they can be used to reliably represent bigger ints, but the point at which they stop working is not clear.

          Also, there is no guarantee that PHP can do anything with these FLOATS when you call any of the date functions. E.g. http://php.net/manual/en/function.strftime.php says it takes an int as an argument, and there is a warning about out-of-int-range values.

          Is it worth introducing a PARAM_DATESTAMP for this? That would better document what is going on, and make it easier to change in future. Hmm. I guess that does not work, because we are taking web services here, so the whole point is to document our internals for people who want to call Moodle.

          Show
          Tim Hunt added a comment - PARAM_FLOAT worries me. Floats are not precise, and there is no guarantee what they are in PHP. Typically they are IEEE double, which gives you more than 32 bits for the mantissa, so they can be used to reliably represent bigger ints, but the point at which they stop working is not clear. Also, there is no guarantee that PHP can do anything with these FLOATS when you call any of the date functions. E.g. http://php.net/manual/en/function.strftime.php says it takes an int as an argument, and there is a warning about out-of-int-range values. Is it worth introducing a PARAM_DATESTAMP for this? That would better document what is going on, and make it easier to change in future. Hmm. I guess that does not work, because we are taking web services here, so the whole point is to document our internals for people who want to call Moodle.
          Hide
          Jérôme Mouneyrac added a comment -

          Ah I found the range explanation in the Note of http://php.net/manual/en/function.strtotime.php where it's saying:

          The valid range of a timestamp is typically from Fri, 13 Dec 1901 20:45:54 UTC to Tue, 19 Jan 2038 03:14:07 UTC.

          I checked on Moodle, if I create by web service a timestamp over 2020, it doesn't make sens on the user interface, as for example you can not have a start date after 2020 for a course.

          I'll fix the PHP unit test timestamp.

          Otherwise I do think it would be good to have a PARAM_TIMESTAMP, so we could return a more appropriate error than this is not an int.

          Show
          Jérôme Mouneyrac added a comment - Ah I found the range explanation in the Note of http://php.net/manual/en/function.strtotime.php where it's saying: The valid range of a timestamp is typically from Fri, 13 Dec 1901 20:45:54 UTC to Tue, 19 Jan 2038 03:14:07 UTC. I checked on Moodle, if I create by web service a timestamp over 2020, it doesn't make sens on the user interface, as for example you can not have a start date after 2020 for a course. I'll fix the PHP unit test timestamp. Otherwise I do think it would be good to have a PARAM_TIMESTAMP, so we could return a more appropriate error than this is not an int.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Note: the phpunit timestamp has been fixed in MDL-34941.

          Show
          Jérôme Mouneyrac added a comment - - edited Note: the phpunit timestamp has been fixed in MDL-34941 .
          Hide
          Jérôme Mouneyrac added a comment -

          I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Jérôme Mouneyrac added a comment - I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            People

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

              Dates

              • Created:
                Updated: