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

Base event JSON encoding decoding check will fail with whole float values.

    Details

    • Story Points (Obsolete):
      5
    • Sprint:
      BACKEND Sprint 7

      Description

      Checks are done to ensure that the 'other' event field does not contain an object.

      If the 'other' field contains a float which is a whole number (e.g. 90) then the encoding / decoding will result in the number being returned as an integer. When the comparison is done it will fail.

      Solution: The comparison does not have to be so strict to catch if objects are being passed. '!=' will suffice.
      @see lib/classes/event/base.php line 447

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Adrian,
            Nice find, the patch looks perfect. Just for your second set of asserts, we are already testing those in test_problematic_events(), you can just add your first assert in the same method as well.

            Feel free to submit once updated.

            Cheers

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Adrian, Nice find, the patch looks perfect. Just for your second set of asserts, we are already testing those in test_problematic_events(), you can just add your first assert in the same method as well. Feel free to submit once updated. Cheers
            Hide
            abgreeve Adrian Greeve added a comment -

            Thanks Ankit,

            I've moved the tests around as you suggested.

            Submitting for integration review.

            Show
            abgreeve Adrian Greeve added a comment - Thanks Ankit, I've moved the tests around as you suggested. Submitting for integration review.
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            abgreeve Adrian Greeve added a comment -

            Sorry, I just rebased this.

            Show
            abgreeve Adrian Greeve added a comment - Sorry, I just rebased this.
            Hide
            marina Marina Glancy added a comment -

            Thanks Adrian, integrated in 2.6 and master

            Show
            marina Marina Glancy added a comment - Thanks Adrian, integrated in 2.6 and master
            Hide
            dmonllao David Monllaó added a comment -

            Passing as unit tests are passing in the CI server

            Show
            dmonllao David Monllaó added a comment - Passing as unit tests are passing in the CI server
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for the code, its now upstream!

            Heres a fun trick to try in the spirit of Friday the 13th.
            I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

              People

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

                Dates

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

                  Agile