Moodle
  1. Moodle
  2. MDL-32191

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

    Details

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

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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 added a comment -

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

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Dan Poltawski added a comment -

          Intgreated to master, 25 and 24 - thanks guys.

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

          Nav loooking good here

          Show
          Dan Poltawski added a comment - Nav loooking good here
          Hide
          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 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: