Moodle
  1. Moodle
  2. MDL-32913

Gradebook export is empty when calculation error

    Details

    • Testing Instructions:
      Hide

      You'll need a course with at least one student and at least two gradeable activities.

      Setup

      1. Create a new calculated grade item. Set it to be equal to the sum of two other grade items ie =[[itemA]]+[[itemB]]
      2. Go to the user report (in the gradebook) and check that you can select individual students from the drop down.
      3. Export the course grades and check that you can export the grades without error. Make sure that both your activities and your users have an ID number set (http://docs.moodle.org/23/en/Grade_export#XML_file_export).

      Check its broken

      1. Delete one of the activities that is relied upon by the calculated grade item.
      2. Now, the user report should give you an error. The error message should tell you that there is a circular reference or broken calculation formula.
      3. Similarly grade export should give you an error.

      Student still works

      1. Log in as a student and check that you can access the user report and that you don't see the exception message. Within the user report the calc grade item will report an error but the report itself should load.

      Check its fixable

      1. Log back in as teacher/admin, go to the categories and items screen and remove the reference to the deleted grade item by editing the calculation. The missing grade item name in the calculation will have been replaced with a token.
      2. Check that the user report now loads for the teacher/admin.
      Show
      You'll need a course with at least one student and at least two gradeable activities. Setup Create a new calculated grade item. Set it to be equal to the sum of two other grade items ie =[ [itemA] ]+[ [itemB] ] Go to the user report (in the gradebook) and check that you can select individual students from the drop down. Export the course grades and check that you can export the grades without error. Make sure that both your activities and your users have an ID number set ( http://docs.moodle.org/23/en/Grade_export#XML_file_export ). Check its broken Delete one of the activities that is relied upon by the calculated grade item. Now, the user report should give you an error. The error message should tell you that there is a circular reference or broken calculation formula. Similarly grade export should give you an error. Student still works Log in as a student and check that you can access the user report and that you don't see the exception message. Within the user report the calc grade item will report an error but the report itself should load. Check its fixable Log back in as teacher/admin, go to the categories and items screen and remove the reference to the deleted grade item by editing the calculation. The missing grade item name in the calculation will have been replaced with a token. Check that the user report now loads for the teacher/admin.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      brokencalc_24
    • Pull Master Branch:
      brokencalc
    • Rank:
      39993

      Description

      1.Test requires a course set up with more than one assignment
      2.In the gradebook, create a category
      3.Add at least two assignments to the gradebook category
      4.Navigate to the Categories and Items tab in the gradebook
      5.Click on the 'Edit Calculation' icon under the 'Actions' heading for the category
      6.Creating a calculation for the total which uses at least two assignments
      7.Return to the course front page and delete one of the assignments used in the calculation

      Any gradebook export will now be empty of students and no error is displayed

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          I've added a possible fix. My solution was to make the problem with the broken calculated grade item more obvious so that hopefully a teacher will be reminded to go and fix the calc grade item. After this fix, going to the user report or attempting a grade export will result in an explicit error being displayed.

          While it would be nice to work around the broken calc grade item my thinking is that, after you've deleted a necessary grade item, the gradebook is pretty much broken and needs fixing by the user. The more obvious we make the problem the sooner it will get fixed.

          Show
          Andrew Davis added a comment - I've added a possible fix. My solution was to make the problem with the broken calculated grade item more obvious so that hopefully a teacher will be reminded to go and fix the calc grade item. After this fix, going to the user report or attempting a grade export will result in an explicit error being displayed. While it would be nice to work around the broken calc grade item my thinking is that, after you've deleted a necessary grade item, the gradebook is pretty much broken and needs fixing by the user. The more obvious we make the problem the sooner it will get fixed.
          Hide
          Tim Hunt added a comment -

          Well, I have just spent several hours tracking down a "When I export grades I get an empty file" bug report to this cause, so many votes from me for fixing this.

          Thinking about the general approach:

          Another possible fix is: Improve the validation, so it is impossible to delete a grade item that is used by a calculation in another one.

          Hmm. I guess that is impossible, becuase some grade items are linked to, say, quizzes, and it would be really crazy to prevent deletion of the quiz in that case. Well, not really crazy.

          Now to peer review the patch:

          It is mostly sound, but

          1. I do not think you should be calling print_error in library code. You should me throwing a moodle_exception instead. (I know that practically it amounts to the same thing, but throwing an exception is right, and you should change it.)

          2. Do we really want a fatal error here? ... not sure. The alternative would be some sort of notify message, above the normal display. I can't see any point in that, but I hope that the error page contains all the usual gradebook navigation, so that the teacher can easily navigate to the grader report, and categories an items and fix the issue.

          3. I think you should add some steps to the testing instructions, to verify that when logged in as a student, you continue to see what you see now, and you don't see the error that is aimed at teachers.

          Show
          Tim Hunt added a comment - Well, I have just spent several hours tracking down a "When I export grades I get an empty file" bug report to this cause, so many votes from me for fixing this. Thinking about the general approach: Another possible fix is: Improve the validation, so it is impossible to delete a grade item that is used by a calculation in another one. Hmm. I guess that is impossible, becuase some grade items are linked to, say, quizzes, and it would be really crazy to prevent deletion of the quiz in that case. Well, not really crazy. Now to peer review the patch: It is mostly sound, but 1. I do not think you should be calling print_error in library code. You should me throwing a moodle_exception instead. (I know that practically it amounts to the same thing, but throwing an exception is right, and you should change it.) 2. Do we really want a fatal error here? ... not sure. The alternative would be some sort of notify message, above the normal display. I can't see any point in that, but I hope that the error page contains all the usual gradebook navigation, so that the teacher can easily navigate to the grader report, and categories an items and fix the issue. 3. I think you should add some steps to the testing instructions, to verify that when logged in as a student, you continue to see what you see now, and you don't see the error that is aimed at teachers.
          Hide
          Andrew Davis added a comment -

          I've switched over to throwing a moodle_exception. That is nicer.

          The error page does indeed still display all the blocks and gradebook navigation so the teacher can still easily reach the grader report and the categories and items screen to fix the problem.

          I'll add students to the testing instructions. The user report continues to function as it does now for students.

          Show
          Andrew Davis added a comment - I've switched over to throwing a moodle_exception. That is nicer. The error page does indeed still display all the blocks and gradebook navigation so the teacher can still easily reach the grader report and the categories and items screen to fix the problem. I'll add students to the testing instructions. The user report continues to function as it does now for students.
          Hide
          Andrew Davis added a comment -

          Adding 2.4 and 2.3 versions. Putting this up for integration.

          Show
          Andrew Davis added a comment - Adding 2.4 and 2.3 versions. Putting this up for integration.
          Hide
          Tim Hunt added a comment -

          Great. Thanks Andrew

          Drat! I only just noticed that the first line of your commit comment is too long. Probably not worth fixing that now.

          Show
          Tim Hunt added a comment - Great. Thanks Andrew Drat! I only just noticed that the first line of your commit comment is too long. Probably not worth fixing that now.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23 - thanks!

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks!
          Hide
          Mark Nelson added a comment -

          Ok, so what is the point of having those text inputs next to the gradeable item when they don't do anything? I had to actually visit the item settings themselves to assign an ID number to get the formula to work. Is this a bug or am I misinterpreting the usage of these text fields?

          Show
          Mark Nelson added a comment - Ok, so what is the point of having those text inputs next to the gradeable item when they don't do anything? I had to actually visit the item settings themselves to assign an ID number to get the formula to work. Is this a bug or am I misinterpreting the usage of these text fields?
          Hide
          Tim Hunt added a comment -

          Mark, No idea, but that is a separate issue.

          Show
          Tim Hunt added a comment - Mark, No idea, but that is a separate issue.
          Hide
          Mark Nelson added a comment -

          Hi Tim, I realise that was just wondering if it was something I was doing wrong before I created a tracker issue. I will create one anyway as it is not intuitive as to what is expected.

          I am failing this test because when I choose to export in a XML file format I can not select 'Gradeable item' (the gradeable item I created following the testing instructions) or 'Course total'. When I click to download the file there are no results.

          Show
          Mark Nelson added a comment - Hi Tim, I realise that was just wondering if it was something I was doing wrong before I created a tracker issue. I will create one anyway as it is not intuitive as to what is expected. I am failing this test because when I choose to export in a XML file format I can not select 'Gradeable item' (the gradeable item I created following the testing instructions) or 'Course total'. When I click to download the file there are no results.
          Hide
          Mark Nelson added a comment -

          I created MDL-39519 for the issue I experienced that was unrelated to this fix.

          Show
          Mark Nelson added a comment - I created MDL-39519 for the issue I experienced that was unrelated to this fix.
          Hide
          Mark Nelson added a comment -

          I should also point out that the other download formats worked as expected.

          Show
          Mark Nelson added a comment - I should also point out that the other download formats worked as expected.
          Hide
          Tim Hunt added a comment -

          Mark, users are only included in the Moodle XML export if the user has an idnumber in their profile. Is that the problem you hit?

          Surely gradebook export is documented?

          Show
          Tim Hunt added a comment - Mark, users are only included in the Moodle XML export if the user has an idnumber in their profile. Is that the problem you hit? Surely gradebook export is documented?
          Hide
          Andrew Davis added a comment -

          Tim, I suspect you're correct that missing ID numbers are to blame.

          I've updated the testing instructions to be more explicit. My bad. Mark, could you please retest this and let me know if you are still having a problem.

          I've also commented on MDL-39519

          Show
          Andrew Davis added a comment - Tim, I suspect you're correct that missing ID numbers are to blame. I've updated the testing instructions to be more explicit. My bad. Mark, could you please retest this and let me know if you are still having a problem. I've also commented on MDL-39519
          Hide
          Mark Nelson added a comment -

          Yep, seems that was the issue! I will retest this asap.

          Show
          Mark Nelson added a comment - Yep, seems that was the issue! I will retest this asap.
          Hide
          Mark Nelson added a comment -

          Hi guys, works as expected. There is only one trivial issue which I have created a fix for.

          Currently the error message looks like: "An error occurred during grade calculation: Probably circular reference or broken calculation formula,Probably circular reference or broken calculation formula". Is there any need to repeat the message twice? If the error involves more than two grade items it is going to be repeated numerous times, which will look bizarre. I created a patch that does not touch the commonly used function grade_regrade_final_grades but changes the behaviour of the newly created function export_verify_grades. I also use this function in grade/lib.php rather than repeating the same code again - see https://github.com/markn86/moodle/commit/901185494bce17e402deaf83f40b749455b1b789 and let me know what you think.

          Show
          Mark Nelson added a comment - Hi guys, works as expected. There is only one trivial issue which I have created a fix for. Currently the error message looks like: "An error occurred during grade calculation: Probably circular reference or broken calculation formula,Probably circular reference or broken calculation formula". Is there any need to repeat the message twice? If the error involves more than two grade items it is going to be repeated numerous times, which will look bizarre. I created a patch that does not touch the commonly used function grade_regrade_final_grades but changes the behaviour of the newly created function export_verify_grades. I also use this function in grade/lib.php rather than repeating the same code again - see https://github.com/markn86/moodle/commit/901185494bce17e402deaf83f40b749455b1b789 and let me know what you think.
          Hide
          Mark Nelson added a comment -

          I created a separate issue for this at MDL-39535.

          Show
          Mark Nelson added a comment - I created a separate issue for this at MDL-39535 .
          Hide
          Mark Nelson added a comment -

          If one of you guys could peer review it that would be great.

          Show
          Mark Nelson added a comment - If one of you guys could peer review it that would be great.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Did you think this day was not going to arrive ever?

          Your patience has been rewarded, yay, sent upstream, thanks!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: