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:

      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

          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: