Moodle
  1. Moodle
  2. MDL-24202

It is not possible to define the "Show course categories" setting when there is only 1 category

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.1.1, 2.2
    • Fix Version/s: 2.0.5, 2.1.2
    • Component/s: Administration
    • Labels:

      Description

      Go to Settings->Appearance->Navigation
      Select the checkbox: Show course categories
      Save changes.

      On my local installation at the reload time, the selection is lost and the page doesn't show the message "Changes saved".

        Gliffy Diagrams

          Activity

          Hide
          Dan Poltawski added a comment -

          Hi Daniele,

          I can't reproduce this issue - I suspect it has been fixed in the development cycle. Please reopen if it continues to be an issue.

          Show
          Dan Poltawski added a comment - Hi Daniele, I can't reproduce this issue - I suspect it has been fixed in the development cycle. Please reopen if it continues to be an issue.
          Hide
          Daniele Cordella added a comment -

          Ciao Dan.
          I just updated moodle2 from CVS.
          The problem is still where it was.
          Attached is a movie describing it better than any word.

          Show
          Daniele Cordella added a comment - Ciao Dan. I just updated moodle2 from CVS. The problem is still where it was. Attached is a movie describing it better than any word.
          Hide
          Daniele Cordella added a comment -

          Issue reopened because of my last comment.

          Show
          Daniele Cordella added a comment - Issue reopened because of my last comment.
          Hide
          Luis A. Martínez Sobrino added a comment - - edited

          The real problem in not that you cannot define the setting, which you actually can, but that you cannot see the actual status, since it always appears as unchecked after submitting the changes.

          If you dig a little bit into the database you can see the value for that variable changing:

          select * from mdl_config where name='navshowcategories';

          Regards

          Show
          Luis A. Martínez Sobrino added a comment - - edited The real problem in not that you cannot define the setting, which you actually can, but that you cannot see the actual status, since it always appears as unchecked after submitting the changes. If you dig a little bit into the database you can see the value for that variable changing: select * from mdl_config where name='navshowcategories'; Regards
          Hide
          Dan Poltawski added a comment -

          Ah, this is an issue when there is only 1 course category because navigationlib explicity sets the $CFG option to false.

          Not sure what the best way of dealing with this is really, I certainly agree its confusing!

          Assigning to Sam to consider!

          Show
          Dan Poltawski added a comment - Ah, this is an issue when there is only 1 course category because navigationlib explicity sets the $CFG option to false. Not sure what the best way of dealing with this is really, I certainly agree its confusing! Assigning to Sam to consider!
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          Definitely a bug! the navigation really shouldn't be changing the $CFG variable, it should instead be passing around an argument if it needs to change things.
          I'll ask Michael D to triage this bug for me given that it is a bug and see how it looks for inclusion within a sprint.
          If it doesn't shape I'll look to solve it when I have a few minutes spare some time.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, Definitely a bug! the navigation really shouldn't be changing the $CFG variable, it should instead be passing around an argument if it needs to change things. I'll ask Michael D to triage this bug for me given that it is a bug and see how it looks for inclusion within a sprint. If it doesn't shape I'll look to solve it when I have a few minutes spare some time. Cheers Sam
          Hide
          Michael de Raadt added a comment -

          Magically added to a future sprint.

          Show
          Michael de Raadt added a comment - Magically added to a future sprint.
          Hide
          Sam Hemelryk added a comment -

          Ready for peer-review

          Show
          Sam Hemelryk added a comment - Ready for peer-review
          Hide
          Rajesh Taneja added a comment -

          Looks Good to me. Just few questions:
          1. showcategories being boolean, shouldn't it be initialised as 'true/false'.

          /** @var bool */
          protected $showcategories = null;
          

          2. If yes for point 1, then show_categories() should be modified accordingly.

          Show
          Rajesh Taneja added a comment - Looks Good to me. Just few questions: 1. showcategories being boolean, shouldn't it be initialised as 'true/false'. /** @var bool */ protected $showcategories = null; 2. If yes for point 1, then show_categories() should be modified accordingly.
          Hide
          Sam Hemelryk added a comment -

          Thanks for looking at it Raj.
          The new showcategories property is initialised as null so that we can tell when it has been properly initialised as it will be a boolean..

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks for looking at it Raj. The new showcategories property is initialised as null so that we can tell when it has been properly initialised as it will be a boolean.. Cheers Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Tiny detail. Could you move the global declarations to the beginning of the method? We don't use conditional globals ever (or shouldn't be).

          I'll be happy to integrate this once that's amended. TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Tiny detail. Could you move the global declarations to the beginning of the method? We don't use conditional globals ever (or shouldn't be). I'll be happy to integrate this once that's amended. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          No probs Eloy, done now (rebased at the same time)

          Show
          Sam Hemelryk added a comment - No probs Eloy, done now (rebased at the same time)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Andrew Davis added a comment -

          Passing this as the bug description contains steps to reproduce the bug, ie defining the incorrect behavior. In future add testing instructions to define the correct behavior. Otherwise the tester has to extrapolate/guess the correct behavior in an area of Moodle they may have no experience with.

          Eloy, Rajesh and Sam. You're all HQ developers. If we can't do this right between 3 of us all hope is lost

          Show
          Andrew Davis added a comment - Passing this as the bug description contains steps to reproduce the bug, ie defining the incorrect behavior. In future add testing instructions to define the correct behavior. Otherwise the tester has to extrapolate/guess the correct behavior in an area of Moodle they may have no experience with. Eloy, Rajesh and Sam. You're all HQ developers. If we can't do this right between 3 of us all hope is lost
          Hide
          Daniele Cordella added a comment -

          Dear estimed professionals
          (I can not call you friends because I do not even have never seen most of you, but I would)
          let me add my cent of contribution to Andrew's comment.
          His comment simply underlines that the procedure to maintain moodle becames daily more difficult to follow. I agree it is necessary but a lot of time I spend more time to manage a pull request than to correct an issue. I have no idea about how to solve this my personal problem but... let me tell that the procedure seems to be done to take developers far from moodle.
          With love and respect. Daniele

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - Dear estimed professionals (I can not call you friends because I do not even have never seen most of you, but I would) let me add my cent of contribution to Andrew's comment. His comment simply underlines that the procedure to maintain moodle becames daily more difficult to follow. I agree it is necessary but a lot of time I spend more time to manage a pull request than to correct an issue. I have no idea about how to solve this my personal problem but... let me tell that the procedure seems to be done to take developers far from moodle. With love and respect. Daniele – Posted from Bugbox for iPhone
          Hide
          Eloy Lafuente (stronk7) added a comment -

          git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: