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

Add keys to module, block, and category settings nav nodes

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.2, 2.4
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Navigation
    • Labels:

      Description

      This is useful in conjunction with MDL-30506 to provide the opportunity to easily extend module, block, and category settings trees. Currently these nodes are not readily addressable because they lack a handle. Other navigation subtrees, like user, course, and front page, already have keys.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              jonof Jonathon Fowler added a comment -

              I chose TYPE_SETTING for the node type based on the type given to the 'frontpage' node (it was the first example I found, and the nodes were defaulting to TYPE_CUSTOM before). There are two other examples I could see: 'usercurrentsettings' = TYPE_CONTAINER, 'courseadmin' = TYPE_COURSE. Would it be better if I use TYPE_CONTAINER instead?

              Show
              jonof Jonathon Fowler added a comment - I chose TYPE_SETTING for the node type based on the type given to the 'frontpage' node (it was the first example I found, and the nodes were defaulting to TYPE_CUSTOM before). There are two other examples I could see: 'usercurrentsettings' = TYPE_CONTAINER, 'courseadmin' = TYPE_COURSE. Would it be better if I use TYPE_CONTAINER instead?
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Hi Jono,

              Thanks for creating the issue and especially for providing a fix, sorry it took so long for me to get to it!
              I've had a look now and it looks spot on, I've turned it into all the required branches and will put it up for integration immediately.

              Many thanks
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Jono, Thanks for creating the issue and especially for providing a fix, sorry it took so long for me to get to it! I've had a look now and it looks spot on, I've turned it into all the required branches and will put it up for integration immediately. Many thanks Sam
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Sorry but it seems that there is some difference between the solution provided for categories @ 22_STABLE and how it's right now, for categories, in 23, 24 & master.

              Can you plz, clarify which one is correct, both the $type and the $key params?

              In the mean time I'm reopening this...thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Sorry but it seems that there is some difference between the solution provided for categories @ 22_STABLE and how it's right now, for categories, in 23, 24 & master. Can you plz, clarify which one is correct, both the $type and the $key params? In the mean time I'm reopening this...thanks!
              Hide
              cibot CiBoT added a comment -

              Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              jonof Jonathon Fowler added a comment -

              @Eloy: True. Looks like the modification to load_category_settings() is missing from the 2.3+ branches that Sam prepared.

              Show
              jonof Jonathon Fowler added a comment - @Eloy: True. Looks like the modification to load_category_settings() is missing from the 2.3+ branches that Sam prepared.
              Hide
              jonof Jonathon Fowler added a comment -

              Just been doing some housekeeping and came across this issue again.

              Sam's wip-MDL-32191m23, wipMDL-32191m24, and wipMDL-32191-m25 branches are fine. It's the 2.2 branch that's the anomaly because it uses a different key for the category node than what got added in the later branches by MDL-37034, ie. categoryadmin vs categorysettings.

              The question is whether the 2.3+ branches should be tweaked to follow the 'settings' naming pattern of MDL-37034, ie. modulesettings, blocksettings. Assuming so, I've made new branches with this change and will update the issue and submit for peer review. I dropped the 2.2 version too.

              Show
              jonof Jonathon Fowler added a comment - Just been doing some housekeeping and came across this issue again. Sam's wip- MDL-32191 m23, wip MDL-32191 m24, and wip MDL-32191 -m25 branches are fine. It's the 2.2 branch that's the anomaly because it uses a different key for the category node than what got added in the later branches by MDL-37034 , ie. categoryadmin vs categorysettings. The question is whether the 2.3+ branches should be tweaked to follow the 'settings' naming pattern of MDL-37034 , ie. modulesettings, blocksettings. Assuming so, I've made new branches with this change and will update the issue and submit for peer review. I dropped the 2.2 version too.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              My apologies Jono, this slipped through the cracks.

              The changes look good so I've rebased and produced branches as need be using your commits and am putting this up for integration review now.

              Many thanks
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - My apologies Jono, this slipped through the cracks. The changes look good so I've rebased and produced branches as need be using your commits and am putting this up for integration review now. Many thanks Sam
              Hide
              poltawski Dan Poltawski added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              poltawski Dan Poltawski added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              poltawski Dan Poltawski added a comment -

              Intgreated to master, 25 and 24 - thanks guys.

              Show
              poltawski Dan Poltawski added a comment - Intgreated to master, 25 and 24 - thanks guys.
              Hide
              poltawski Dan Poltawski added a comment -

              Nav loooking good here

              Show
              poltawski Dan Poltawski added a comment - Nav loooking good here
              Hide
              damyon Damyon Wiese added a comment -

              Thanks for your efforts. This issue is one of the outstanding issues that passed all our testing and was accepted into Moodle this week.

              Hurray!

              Show
              damyon Damyon Wiese added a comment - Thanks for your efforts. This issue is one of the outstanding issues that passed all our testing and was accepted into Moodle this week. Hurray!

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    9/Sep/13