Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35593

core_webservice_get_site_info returns version number set to PARAM_TEXT and PARAM_FLOAT

    Details

      Description

      core_webservice_get_site_info should always use the same PARAM_XXX for version numbers. Note that PARAM_FLOAT may be not the best choice (or may be), see MDL-31473 report.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            jerome Jérôme Mouneyrac added a comment -

            Note for integrators: this change will fix MDL-31473 and make the function consistant, however it could break all typed clients that are generated from the description (I can only see some SOAP client doing that yet).

            I didn't backport yet, I'd like your opinion.

            Show
            jerome Jérôme Mouneyrac added a comment - Note for integrators: this change will fix MDL-31473 and make the function consistant, however it could break all typed clients that are generated from the description (I can only see some SOAP client doing that yet). I didn't backport yet, I'd like your opinion.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Jerome,

            I agree this should be PARAM_TEXT, so the change looks good. As to whether to backport, i'd suggest seeing what the web services forum thinks about the impact?

            I'd personally be in favour of correcting it, but I really don't know what the impact would be on people out there.

            Show
            poltawski Dan Poltawski added a comment - Hi Jerome, I agree this should be PARAM_TEXT, so the change looks good. As to whether to backport, i'd suggest seeing what the web services forum thinks about the impact? I'd personally be in favour of correcting it, but I really don't know what the impact would be on people out there.
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            Thanks Dan, I'll ask on the forum.

            update: http://moodle.org/mod/forum/discuss.php?d=212105#p924278

            Show
            jerome Jérôme Mouneyrac added a comment - - edited Thanks Dan, I'll ask on the forum. update: http://moodle.org/mod/forum/discuss.php?d=212105#p924278
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I think it's better to have all versions matching the same correct validation. Nobody answered on the forum, I backported the fix.

            Show
            jerome Jérôme Mouneyrac added a comment - I think it's better to have all versions matching the same correct validation. Nobody answered on the forum, I backported the fix.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jerome this has been integrated now. Cheers for trying in the forums btw

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jerome this has been integrated now. Cheers for trying in the forums btw
            Hide
            damyon Damyon Wiese added a comment -

            Test passed - but only in master as the unit tests for the webservices are not in 22 or 23.

            Show
            damyon Damyon Wiese added a comment - Test passed - but only in master as the unit tests for the webservices are not in 22 or 23.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/Jan/13