Moodle
  1. Moodle
  2. MDL-27920

Exporting gradebook in XML and Excel Format fails in Moodle 2.1

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.3, 2.1
    • Fix Version/s: None
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide

      Create a quiz and have two students take it. Then try to export the course using all four export formats. Note that the XML export excludes students who dont have an idnumber set and also excludes course totals.

      Show
      Create a quiz and have two students take it. Then try to export the course using all four export formats. Note that the XML export excludes students who dont have an idnumber set and also excludes course totals.
    • Workaround:
      Hide

      Export to ODL or .txt formats and then import those to Excel. I'm not sure how to get XML format if that is what is desired.

      Show
      Export to ODL or .txt formats and then import those to Excel. I'm not sure how to get XML format if that is what is desired.
    • URL:
      qa.moodle.net
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
      MDL-27920_grade_export
    • Rank:
      17564

      Description

      When exporting the gradebook in Moodle 2.1, OpenDocument and Plain Text export work fine. Excel spreadsheet export produces a spreadsheet that has grade information but not student information if opened using Excel 2010 on Windows 7. When trying to exort with XML format, none of the grade items can be selected in the export screen. The downloaded XML file has neither student information or grade information in it.

      This was done in a small course with two grades (one quiz and one course total) and with eight students. Two of the students had grades and the other 6 had none.

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          Hi Steve. I have made some changes so may have fixed this but can you elaborate on what you are seeing in the Excel export? I get the following columns next to the grade items:

          First name Surname ID number Institution Department Email address

          Show
          Andrew Davis added a comment - Hi Steve. I have made some changes so may have fixed this but can you elaborate on what you are seeing in the Excel export? I get the following columns next to the grade items: First name Surname ID number Institution Department Email address
          Hide
          Andrew Davis added a comment - - edited

          There are two related fixes in this commit. First, users with nothing in the idnumber column which is separate from the id column, are excluded from the XML export. This is because you're presumably exporting in XML format to import into another system and the user's idnumber is what is used to find students in that other system. id number is typically something like the student's student ID.

          I've expanded a few comments in the code to make this reasoning more clear. I've also altered the XML export so it actually outputs a message while you're previewing the export data. That should hopefully prevent those "where are my students?" moments.

          Secondly, course totals were always being excluded from exports. More specifically course total was always being exported as a hyphen/dash. This was because of a bug in grade_users_iterator::_pop(). This is pretty central so its a bit odd that this has gone undetected so long. $this->grades_rs contains grades. The _pop() function was calling next() then current() on $this->grades_rs thus it always skipped the first grade in the set. The grades seem to be sorted by ID or similar with the result that the first grade always seems to be the course total.

          Show
          Andrew Davis added a comment - - edited There are two related fixes in this commit. First, users with nothing in the idnumber column which is separate from the id column, are excluded from the XML export. This is because you're presumably exporting in XML format to import into another system and the user's idnumber is what is used to find students in that other system. id number is typically something like the student's student ID. I've expanded a few comments in the code to make this reasoning more clear. I've also altered the XML export so it actually outputs a message while you're previewing the export data. That should hopefully prevent those "where are my students?" moments. Secondly, course totals were always being excluded from exports. More specifically course total was always being exported as a hyphen/dash. This was because of a bug in grade_users_iterator::_pop(). This is pretty central so its a bit odd that this has gone undetected so long. $this->grades_rs contains grades. The _pop() function was calling next() then current() on $this->grades_rs thus it always skipped the first grade in the set. The grades seem to be sorted by ID or similar with the result that the first grade always seems to be the course total.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ho Andrew, the logic applied sounds perfect... anyway some minor points:

          1) Do we really need the $require_user_idnumber var? Is not a comment enough? Or some meaningful constant? Also note the var name does not fulfill coding norms.

          2) The _pop() iterator. Wow, the "exit condition" is really wrong (was ok on 1.9 but not since 2.0). To check if one recordset still has rows you should be using the valid() method (look for uses in codebase, is simple) and then the current() + next() as you are using them, that's correct.

          3) The problem commented in 2) seems to be present in other places @ grade/lib.php we should fix all them to use exclusively, valid(). This can go to another issue. np.

          4) Finally, as far as this is one bug... isn't it correct to backport it to 20_STABLE? Sounds natural for me.

          For your consideration, I'm keeping this under integration for now... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Ho Andrew, the logic applied sounds perfect... anyway some minor points: 1) Do we really need the $require_user_idnumber var? Is not a comment enough? Or some meaningful constant? Also note the var name does not fulfill coding norms. 2) The _pop() iterator. Wow, the "exit condition" is really wrong (was ok on 1.9 but not since 2.0). To check if one recordset still has rows you should be using the valid() method (look for uses in codebase, is simple) and then the current() + next() as you are using them, that's correct. 3) The problem commented in 2) seems to be present in other places @ grade/lib.php we should fix all them to use exclusively, valid(). This can go to another issue. np. 4) Finally, as far as this is one bug... isn't it correct to backport it to 20_STABLE? Sounds natural for me. For your consideration, I'm keeping this under integration for now... ciao
          Hide
          Andrew Davis added a comment -

          I believe I have fixed all of those including the backport to 20_STABLE. I have raised MDL-28005 to check the rest of gradebook for improper checks on result sets.

          Show
          Andrew Davis added a comment - I believe I have fixed all of those including the backport to 20_STABLE. I have raised MDL-28005 to check the rest of gradebook for improper checks on result sets.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing test without further action. This will be tested by MDLQA-1138 once this meets upstream.

          Show
          Eloy Lafuente (stronk7) added a comment - Passing test without further action. This will be tested by MDLQA-1138 once this meets upstream.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          All git & cvs servers have been updated with these cool changes, so closing, many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - All git & cvs servers have been updated with these cool changes, so closing, many thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: