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

Unfriendly error when Grading essays of empty lesson

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6, 2.7
    • Fix Version/s: 2.5.6, 2.6.3
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide

      1. Create or go to a course.
      2. Create a new lesson.
      3. Go to the "Grade essays" tab.
      4. Here you will now see a user friendly error with lesson tabs (Preview / Edit / Reports / Grade essays) still visible.

      Show
      1. Create or go to a course. 2. Create a new lesson. 3. Go to the "Grade essays" tab. 4. Here you will now see a user friendly error with lesson tabs (Preview / Edit / Reports / Grade essays) still visible.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      When trying to grade essays of an empty lesson, the user sees a non-friendly error. A user-friendly error is already included in Moodle's code, but it get's hijacked by a print_error statement, if DB query returns empty.

        Gliffy Diagrams

          Activity

          Hide
          kriv Krister Viirsaar added a comment -

          The fix is just one line, please review and merge.

          Show
          kriv Krister Viirsaar added a comment - The fix is just one line, please review and merge.
          Hide
          kriv Krister Viirsaar added a comment -

          I understand this is a minor issue, but it's really easy to fix. Why is this not being reviewed?

          Show
          kriv Krister Viirsaar added a comment - I understand this is a minor issue, but it's really easy to fix. Why is this not being reviewed?
          Hide
          kriv Krister Viirsaar added a comment -

          Updated to current master

          Show
          kriv Krister Viirsaar added a comment - Updated to current master
          Hide
          cibot CiBoT added a comment -

          Results for MDL-42635

          • Remote repository: git://github.com/KristerV/moodle
          Show
          cibot CiBoT added a comment - Results for MDL-42635 Remote repository: git://github.com/KristerV/moodle Remote branch MDL-42635 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/944 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/944/artifact/work/smurf.html
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi Krister,

          Thank you for proving patch to fix this issue.

          Patch looks great and should be backported to 2.5 and 2.6.

          Could you please provide patch for those branches.

          Cheers
          Rosie

          Show
          rwijaya Rossiani Wijaya added a comment - Hi Krister, Thank you for proving patch to fix this issue. Patch looks great and should be backported to 2.5 and 2.6. Could you please provide patch for those branches. Cheers Rosie
          Hide
          kriv Krister Viirsaar added a comment -

          Thank you for reviewing. Backporting done!

          Show
          kriv Krister Viirsaar added a comment - Thank you for reviewing. Backporting done!
          Hide
          cibot CiBoT added a comment -

          Results for MDL-42635

          • Remote repository: git://github.com/KristerV/moodle
          Show
          cibot CiBoT added a comment - Results for MDL-42635 Remote repository: git://github.com/KristerV/moodle Remote branch MDL-42635 -M26 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1383 Error: The MDL-42635 -M26 branch at git://github.com/KristerV/moodle is very old (>60 days ago). Please rebase against current MOODLE_25_STABLE. Remote branch MDL-42635 -M26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1384 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1384/artifact/work/smurf.html Remote branch MDL-42635 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1385 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1385/artifact/work/smurf.html
          Hide
          salvetore Michael de Raadt added a comment -

          As Rosie has left HQ, I'm opening this issue up to other peer reviewers.

          Show
          salvetore Michael de Raadt added a comment - As Rosie has left HQ, I'm opening this issue up to other peer reviewers.
          Hide
          andyjdavis Andrew Davis added a comment -

          Its a minor point but the inline comment should have a full-stop. http://docs.moodle.org/dev/Coding_style#Inline_comments

          The testing instructions should describe what the tester should see once your fix is applied, not the behaviour before the fix.

          Once both of those are fixed you can put this up for integration. If you do not have the capability required let me know and I will do it for you.

          Show
          andyjdavis Andrew Davis added a comment - Its a minor point but the inline comment should have a full-stop. http://docs.moodle.org/dev/Coding_style#Inline_comments The testing instructions should describe what the tester should see once your fix is applied, not the behaviour before the fix. Once both of those are fixed you can put this up for integration. If you do not have the capability required let me know and I will do it for you.
          Hide
          kriv Krister Viirsaar added a comment -

          Updated the branches to current.

          Thanks for the review. Both issues fixed (thanks for pointing the minor one out as well).

          Show
          kriv Krister Viirsaar added a comment - Updated the branches to current. Thanks for the review. Both issues fixed (thanks for pointing the minor one out as well).
          Hide
          kriv Krister Viirsaar added a comment - - edited

          Andrew Davis, please put this up for integration, I don't have the capability. Thanks

          Show
          kriv Krister Viirsaar added a comment - - edited Andrew Davis , please put this up for integration, I don't have the capability. Thanks
          Hide
          marina Marina Glancy added a comment -

          Thanks Krister, integrated in 2.5, 2.6 and master

          I squashed your commits, 5 commits for one-line change was 4 commits too much
          Also I changed your commit message to indicate the component "lesson"

          Show
          marina Marina Glancy added a comment - Thanks Krister, integrated in 2.5, 2.6 and master I squashed your commits, 5 commits for one-line change was 4 commits too much Also I changed your commit message to indicate the component "lesson"
          Hide
          fred Frédéric Massart added a comment -

          Passing, thanks.

          Show
          fred Frédéric Massart added a comment - Passing, thanks.
          Hide
          kriv Krister Viirsaar added a comment -

          Marina Glancy, I find squashing on command line very confusing (otherwise love it). What command or tool do you use?
          Also, is this issue good to be closed?

          Show
          kriv Krister Viirsaar added a comment - Marina Glancy , I find squashing on command line very confusing (otherwise love it). What command or tool do you use? Also, is this issue good to be closed?
          Hide
          marina Marina Glancy added a comment -

          Hi Krister, I use "git rebase -i" and then indicate "squash" for the commits to be squashed, google it, I'm sure there are tutorials online.
          This issue will be closed when it goes through workflow and the weeklies are released (I guess it will happen on Thursday)

          Show
          marina Marina Glancy added a comment - Hi Krister, I use "git rebase -i" and then indicate "squash" for the commits to be squashed, google it, I'm sure there are tutorials online. This issue will be closed when it goes through workflow and the weeklies are released (I guess it will happen on Thursday)
          Hide
          kriv Krister Viirsaar added a comment -

          I've used rebase for squashing purposes before. What interested me was that I had merged master in between my own changes, so the list of rebase commits must have been huge.. how did you pick out the correct ones?

          Show
          kriv Krister Viirsaar added a comment - I've used rebase for squashing purposes before. What interested me was that I had merged master in between my own changes, so the list of rebase commits must have been huge.. how did you pick out the correct ones?
          Hide
          poltawski Dan Poltawski added a comment -

          Thank you for your contribution! This change is now part of Moodle!

          Nothing is impossible, the word itself says 'I'm possible'!

          --Audrey Hepburn

          Show
          poltawski Dan Poltawski added a comment - Thank you for your contribution! This change is now part of Moodle! Nothing is impossible, the word itself says 'I'm possible'! --Audrey Hepburn

            People

            • Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                12/May/14