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

Grade Letters dropdown in Site Administration has courseid

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Gradebook
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      TEST 1

      1. "Site Administration -> Grades -> Letters"
      2. Select the grade dropdown in the upper left "Letters -> Edit"
      3. Notice that there is no id param in the url
      4. Notice that there is no checkbox for overriding site defaults

      TEST 2

      1. Goto "course > grades"
      2. Select the grade dropdown in the upper left "Letters -> Edit"
      3. Notice that there is id param in the url
      4. Notice that there is checkbox for overriding site defaults
      Show
      TEST 1 "Site Administration -> Grades -> Letters" Select the grade dropdown in the upper left "Letters -> Edit" Notice that there is no id param in the url Notice that there is no checkbox for overriding site defaults TEST 2 Goto "course > grades" Select the grade dropdown in the upper left "Letters -> Edit" Notice that there is id param in the url Notice that there is checkbox for overriding site defaults
    • Workaround:
      Hide

      When editing the system letter grades, ALWAYS click on the "Edit grade letters" link.

      Show
      When editing the system letter grades, ALWAYS click on the "Edit grade letters" link.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-31633-master

      Description

      When trying to edit the Letters grade from the system site administration, the drop down routes the user to the SITEID course instead of the system. The "Edit grade letters" link works as expected.

      Replication steps:

      1. "Site Administration -> Grades -> Letters"
      2. Select the grade dropdown in the upper left "Letters -> Edit"
      3. Notice the id param in the url
      4. Notice the checkbox for overriding site defaults
      5. You are editing the letters for the SITEID course and NOT the system

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this.

            I've put it on our backlog.

            In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
            Hide
            isuru89 Isuru Madushanka Weerarathna added a comment -

            Hi,

            I have looked into this little bug and I could solve it. Here I have attached the fix and corresponding git locations would be, https://github.com/isuru89/moodle/compare/MDL-31633-grade-letter-dropdown-courseid

            -Thanks

            Show
            isuru89 Isuru Madushanka Weerarathna added a comment - Hi, I have looked into this little bug and I could solve it. Here I have attached the fix and corresponding git locations would be, https://github.com/isuru89/moodle/compare/MDL-31633-grade-letter-dropdown-courseid -Thanks
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            @Isuru
            Removing the id parameter completely might not be a good idea. As than there would be no way to overwrite grade letters for courses.

            @Andrew
            Do we support grade reports for front page? I can hack the url to see the report with a undefined variable error, but I donot see it linked from the Front page settings navigation.
            The reason I ask is, if it is not supported, than I can simply remove the id parameter from the url when $context->id == $SITE->id , but if we do support it, than I will look for an alternative way to do it.

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - @Isuru Removing the id parameter completely might not be a good idea. As than there would be no way to overwrite grade letters for courses. @Andrew Do we support grade reports for front page? I can hack the url to see the report with a undefined variable error, but I donot see it linked from the Front page settings navigation. The reason I ask is, if it is not supported, than I can simply remove the id parameter from the url when $context->id == $SITE->id , but if we do support it, than I will look for an alternative way to do it. Thanks
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            The only grade report that makes sense to be accessible from the front page is the overview report. Other than that they don't make any sense if accessed from the front page.

            This can be more broadly interpreted as access to the grade reports being unsupported from the front page.

            Show
            andyjdavis Andrew Davis added a comment - - edited The only grade report that makes sense to be accessible from the front page is the overview report. Other than that they don't make any sense if accessed from the front page. This can be more broadly interpreted as access to the grade reports being unsupported from the front page.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks Andrew for the feedback. I have pushed a patch based on that.
            Will update stable branches once, it passes review.

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks Andrew for the feedback. I have pushed a patch based on that. Will update stable branches once, it passes review. Thanks
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks good Ankit

            Show
            rwijaya Rossiani Wijaya added a comment - This looks good Ankit
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks Rosie for the review.
            I have put up stable branches.
            Submitting for integration.
            Thanks!

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks Rosie for the review. I have put up stable branches. Submitting for integration. Thanks!
            Hide
            nebgor Aparup Banerjee added a comment -

            Thanks for the patch

            thats been integrated into 22, 23 and master now.

            Show
            nebgor Aparup Banerjee added a comment - Thanks for the patch thats been integrated into 22, 23 and master now.
            Hide
            andyjdavis Andrew Davis added a comment -

            All things I was to notice were noticed. Passing.

            Show
            andyjdavis Andrew Davis added a comment - All things I was to notice were noticed. Passing.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm so proud...of you, many thanks!

            http://youtu.be/n64CdfDRnZY

            Closing as fixed, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12