Moodle
  1. Moodle
  2. MDL-33776

get_categories does not apply criteria filtering when addsubcategories is specified

    Details

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

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          Jérôme Mouneyrac added a comment -

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

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

          Integrated into master and 23_STABLE

          Show
          Dan Poltawski added a comment - Integrated into master and 23_STABLE
          Hide
          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
          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: