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

get_categories does not apply criteria filtering when addsubcategories is specified

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.1
    • Component/s: Web Services
    • Labels:

      Description

      The summary says it all, I guess.
      When addsubcategories is specified, the criteria should also be applied to the subcategories (take for instance category visibility).
      Currently, if we specify criteria['visible'] = 1 and addsubcategories = 1 (meaning get all visible categories and subcategories), only top categories get filtered for visibility, meaning that non visible subcategories will be returned as well. This should not happen.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting this.

              I've put that on the backlog.

              In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
              Hide
              jerome Jérôme Mouneyrac added a comment -

              I think all criteria should not be applied to the sub categories sql.

              How I understand the function currently works:
              the client dev sends key => id: the client dev wants this category + the sub categories
              the client dev sends key => visible: the client dev wants only visible categories

              What I suggest:
              id, name, parent: same behavior
              theme: the client dev wants only categories with this theme, so this param should applied on the subcategories sql.
              visible: the client dev wants only categories with this visibility, so this param should be applied on the subcategories

              I'll create a fix following these behaviors and update the description to clearly detail it to the client dev.

              Show
              jerome Jérôme Mouneyrac added a comment - I think all criteria should not be applied to the sub categories sql. How I understand the function currently works: the client dev sends key => id: the client dev wants this category + the sub categories the client dev sends key => visible: the client dev wants only visible categories What I suggest: id, name, parent: same behavior theme: the client dev wants only categories with this theme, so this param should applied on the subcategories sql. visible: the client dev wants only categories with this visibility, so this param should be applied on the subcategories I'll create a fix following these behaviors and update the description to clearly detail it to the client dev.
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Jerome,

              The $additionalselect on line 1025 and 1029 need to be assign as '.=' instead of just '=' to avoid overwriting it.

              Other than that, it looks good.

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Jerome, The $additionalselect on line 1025 and 1029 need to be assign as '.=' instead of just '=' to avoid overwriting it. Other than that, it looks good.
              Hide
              jerome Jérôme Mouneyrac added a comment -

              Thanks Rosie for the fix. I added it. Submitting to integration.

              Show
              jerome Jérôme Mouneyrac added a comment - Thanks Rosie for the fix. I added it. Submitting to integration.
              Hide
              poltawski Dan Poltawski added a comment -

              Integrated into master and 23_STABLE

              Show
              poltawski Dan Poltawski added a comment - Integrated into master and 23_STABLE
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Congratulations your code is upstream - gold star for you!

              This issue + 79 others made it in in time for the minor releases.
              Thank you everyone involved for your exuberant efforts.

              Show
              samhemelryk Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    9/Jul/12