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

          Attachments

            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