Moodle
  1. Moodle
  2. MDL-31633

Grade Letters dropdown in Site Administration has courseid

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      38203

      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

        Activity

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

        This looks good Ankit

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

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

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

        Thanks for the patch

        thats been integrated into 22, 23 and master now.

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

        All things I was to notice were noticed. Passing.

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

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

        http://youtu.be/n64CdfDRnZY

        Closing as fixed, ciao

        Show
        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: