Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

          Attachments

            Activity

            Hide
            poltawski 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
            poltawski 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
            daniss 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
            daniss 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
            daniss Daniele Cordella added a comment -

            Issue reopened because of my last comment.

            Show
            daniss Daniele Cordella added a comment - Issue reopened because of my last comment.
            Hide
            qosobrin 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
            qosobrin 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
            poltawski 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
            poltawski 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
            samhemelryk 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
            samhemelryk 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
            salvetore Michael de Raadt added a comment -

            Magically added to a future sprint.

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

            Ready for peer-review

            Show
            samhemelryk Sam Hemelryk added a comment - Ready for peer-review
            Hide
            rajeshtaneja 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
            rajeshtaneja 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
            samhemelryk 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
            samhemelryk 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
            stronk7 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
            stronk7 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
            samhemelryk Sam Hemelryk added a comment -

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

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

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            andyjdavis 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
            andyjdavis 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
            daniss 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
            daniss 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

            Closing, ciao

            Show
            stronk7 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:
                  Fix Release Date:
                  10/Oct/11