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:
    • Rank:
      6305

      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".

        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: