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

Group cache returns unexpected results when passing a field list that would cause the results to be indexed by a field other than id

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5, 2.6
    • Fix Version/s: 2.5.4, 2.6.1
    • Component/s: Groups

      Description

      The new groups cache returns unexpected results when you pass in a field parameter than would normally change the result array index

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            sry_not4sale Aaron Barnes added a comment -

            I have attached an example fix for one of the group functions as an example of how I think it should be fixed.

            Show
            sry_not4sale Aaron Barnes added a comment - I have attached an example fix for one of the group functions as an example of how I think it should be fixed.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for spotting that and sharing a fix.

            I wonder if you could transform your testing instructions so that they can be tested through the interface.

            It's likely that this will have unit tests amended/added.

            Show
            salvetore Michael de Raadt added a comment - Thanks for spotting that and sharing a fix. I wonder if you could transform your testing instructions so that they can be tested through the interface. It's likely that this will have unit tests amended/added.
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Hi Michael,

            It does not appear you can trigger this issue using core Moodle interface, it's an issue that involves contrib code / customisations.

            It also appears as if this is the only function in that file that is affected.

            Cheers,
            Aaron

            Show
            sry_not4sale Aaron Barnes added a comment - Hi Michael, It does not appear you can trigger this issue using core Moodle interface, it's an issue that involves contrib code / customisations. It also appears as if this is the only function in that file that is affected. Cheers, Aaron
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Aaron,

            The fix looks ok to me.

            I think we really need a unit test for this, then it would be good for integration.

            Show
            poltawski Dan Poltawski added a comment - Hi Aaron, The fix looks ok to me. I think we really need a unit test for this, then it would be good for integration.
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Unit tests added

            Show
            sry_not4sale Aaron Barnes added a comment - Unit tests added
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Thanks Dan

            Show
            sry_not4sale Aaron Barnes added a comment - Thanks Dan
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Aaron, Integrated to 25, 26 and master.

            Noting for others that the cache was introduced here in 25 - so no patch is needed for 24 - but we are still backporting to 24 for a little while yet.

            Show
            damyon Damyon Wiese added a comment - Thanks Aaron, Integrated to 25, 26 and master. Noting for others that the cache was introduced here in 25 - so no patch is needed for 24 - but we are still backporting to 24 for a little while yet.
            Hide
            damyon Damyon Wiese added a comment -

            I tested this in integration.

            Show
            damyon Damyon Wiese added a comment - I tested this in integration.
            Hide
            damyon Damyon Wiese added a comment -

            Passing!

            Show
            damyon Damyon Wiese added a comment - Passing!
            Hide
            poltawski Dan Poltawski added a comment -

            Congratulations, this change has now made its way upstream. Thanks for your contribution!

            “ Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. ” - Rick Osborne

            Show
            poltawski Dan Poltawski added a comment - Congratulations, this change has now made its way upstream. Thanks for your contribution! “ Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. ” - Rick Osborne

              People

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

                Dates

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