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

          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