Details

    • Rank:
      37741

      Description

      Offline Marking is a feature that lets markers download all assignments to a zip file along with a marking spreadsheet. The grades are updated in the spreadsheet along with feedback comments etc. Additional feedback files can be added to the zip (based on a naming convention). Then either the entire zip or just the marking spreadsheet is re-uploaded into Moodle and moodle auto assigns the files, feedback and marks to the correct students (with a confirmation step).

        Issue Links

          Activity

          Hide
          Grette Wilkinson added a comment -

          Currently the csv file using naming convention All Students Offline Grading Report.csv. Could the course name be included?

          "All students" is also misleading as the downloaded zip could be set to only download a specified group.

          Coursename_offline_grading_report.csv perhaps?

          Show
          Grette Wilkinson added a comment - Currently the csv file using naming convention All Students Offline Grading Report.csv. Could the course name be included? "All students" is also misleading as the downloaded zip could be set to only download a specified group. Coursename_offline_grading_report.csv perhaps?
          Hide
          Grette Wilkinson added a comment -

          Really interested in knowing how this feature will interact with advanced grading methods

          Show
          Grette Wilkinson added a comment - Really interested in knowing how this feature will interact with advanced grading methods
          Hide
          Minh-Tam Nguyen added a comment -

          Continuing Grette's thoughts regarding file name, I think it would be good to include the group name in the filename: Coursename_Groupname_offline_grading_report.csv or Coursename_Allstudents_offline_grading_report.csv perhaps?

          Thinking about a rubric as an example of an advanced grading method:
          I think trying to include a rubric (table) for each student in the offline grading report would make the file too complex and too difficult to use for teachers
          How about including an additional username_advanced_grading.csv for each student, which contains the rubric which the teacher could complete and then re-upload, in a similar way to how the feedback file is implemented?
          I do not have a suggestion on how to deal with future non-rubric advanced grading methods, unfortunately.

          Show
          Minh-Tam Nguyen added a comment - Continuing Grette's thoughts regarding file name, I think it would be good to include the group name in the filename: Coursename_Groupname_offline_grading_report.csv or Coursename_Allstudents_offline_grading_report.csv perhaps? Thinking about a rubric as an example of an advanced grading method: I think trying to include a rubric (table) for each student in the offline grading report would make the file too complex and too difficult to use for teachers How about including an additional username_advanced_grading.csv for each student, which contains the rubric which the teacher could complete and then re-upload, in a similar way to how the feedback file is implemented? I do not have a suggestion on how to deal with future non-rubric advanced grading methods, unfortunately.
          Hide
          Grette Wilkinson added a comment -

          I think the offline grading report should only include users with role=Students.

          Show
          Grette Wilkinson added a comment - I think the offline grading report should only include users with role=Students.
          Hide
          Michael Hughes added a comment -

          I think this should be an additional option that could be used as an alternative of the Advanced Grading methods, but primarily for just entering the marks (rather than structured like Rubrics etc).

          It would be good for us if it were possible to choose a group (from defined Moodle groups in a course) and package just those students together. This would only be a "packaging" option and separate from the activity's group setting.

          Show
          Michael Hughes added a comment - I think this should be an additional option that could be used as an alternative of the Advanced Grading methods, but primarily for just entering the marks (rather than structured like Rubrics etc). It would be good for us if it were possible to choose a group (from defined Moodle groups in a course) and package just those students together. This would only be a "packaging" option and separate from the activity's group setting.
          Hide
          Damyon Wiese added a comment -

          This patch is based on a tree with MDL-31291, MDL-31284, MDL-31295 all integrated so should be integrated after those patches.

          Show
          Damyon Wiese added a comment - This patch is based on a tree with MDL-31291 , MDL-31284 , MDL-31295 all integrated so should be integrated after those patches.
          Hide
          Damyon Wiese added a comment -

          Finished latest rebasing - this is ready to review.

          Show
          Damyon Wiese added a comment - Finished latest rebasing - this is ready to review.
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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
          Damyon Wiese added a comment -

          Rebased and backported some team-submission / offline assignment fixes.

          One major issue to discuss as part of this change is a rewrite in csvlib.php which was required to handle importing feedback comments with multi-lines etc.

          • Damyon
          Show
          Damyon Wiese added a comment - Rebased and backported some team-submission / offline assignment fixes. One major issue to discuss as part of this change is a rewrite in csvlib.php which was required to handle importing feedback comments with multi-lines etc. Damyon
          Hide
          Aparup Banerjee added a comment -

          stopping review for now sorry (time)

          Show
          Aparup Banerjee added a comment - stopping review for now sorry (time)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee added a comment - - edited

          Hi,
          I've just had a look thru this now. I've also added Adrian to have a glance as he's been working on the csvlib recently. After speaking to Adrian, he seems to have tested out multi line issues.

          The fact that we're moving away from fgetcsv() definitely begs a further discussion as you've mentioned: The idea in MDL-34075 was a move towards RFC compliance. I'm wondering what is the specific multi-line issue that you're facing here and how the fixes from MDL-34075 haven't catered to that.

          re unit tests: there are some test failures:

          • after the above patch the following string matching test fails:
            csvclass_testcase::test_csv_functions
            Failed asserting that two strings are equal.
            --- Expected
            +++ Actual
            @@ @@
            -'Error occur during loading CSV file!'
            +'The CSV file is empty'
            
            /Users/aparup/Sites/i/lib/tests/csvclass_test.php:115
            /Users/aparup/Sites/i/lib/phpunit/classes/advanced_testcase.php:76
            
            To re-run:
             phpunit csvclass_testcase lib/tests/csvclass_test.php
            

          If perhaps the multi-line issue isn't specifically resolvable by the current lib, could we wrap around that. This way we could handle only the exceptions and not everything being re-implemented.

          Show
          Aparup Banerjee added a comment - - edited Hi, I've just had a look thru this now. I've also added Adrian to have a glance as he's been working on the csvlib recently. After speaking to Adrian, he seems to have tested out multi line issues . The fact that we're moving away from fgetcsv() definitely begs a further discussion as you've mentioned: The idea in MDL-34075 was a move towards RFC compliance. I'm wondering what is the specific multi-line issue that you're facing here and how the fixes from MDL-34075 haven't catered to that. re unit tests: there are some test failures: see https://github.com/nebgor/moodle/commit/2d19b183b3419bf90801b744462a2051ce28ee9f after the above patch the following string matching test fails: csvclass_testcase::test_csv_functions Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -'Error occur during loading CSV file!' +'The CSV file is empty' /Users/aparup/Sites/i/lib/tests/csvclass_test.php:115 /Users/aparup/Sites/i/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit csvclass_testcase lib/tests/csvclass_test.php If perhaps the multi-line issue isn't specifically resolvable by the current lib, could we wrap around that. This way we could handle only the exceptions and not everything being re-implemented.
          Hide
          Damyon Wiese added a comment - - edited

          Thanks Aparup, I'll try reverting the CSV changes and do some testing to see if the updated Moodle library covers all my uses - I wrote that code in December so I'll have to refresh my memory on the things that the Moodle library couldn't handle.

          Specifically it should handle multi-line fields:

          a,b,"c","d
          and e", f
          

          And escaped quoted strings:

          a,b,"c","d and ""e"" and f"
          

          Because excel and openoffice both use these features when saving as csv.

          I'll retest and post an update.

          Show
          Damyon Wiese added a comment - - edited Thanks Aparup, I'll try reverting the CSV changes and do some testing to see if the updated Moodle library covers all my uses - I wrote that code in December so I'll have to refresh my memory on the things that the Moodle library couldn't handle. Specifically it should handle multi-line fields: a,b, "c" ,"d and e", f And escaped quoted strings: a,b, "c" , "d and " "e" " and f" Because excel and openoffice both use these features when saving as csv. I'll retest and post an update.
          Hide
          Damyon Wiese added a comment -

          Nope - the Moodle library definitely does not handle multi-line fields.

          In order to simplify this issue I think we should pull the csv changes out and create new issue(s) to fix the bugs with csv handling.

          Thoughts?

          Show
          Damyon Wiese added a comment - Nope - the Moodle library definitely does not handle multi-line fields. In order to simplify this issue I think we should pull the csv changes out and create new issue(s) to fix the bugs with csv handling. Thoughts?
          Hide
          Adrian Greeve added a comment -

          Hi Damyon,

          Could you please provide a file with the multi-line fields that aren't importing via the moodle library? I'm interested to see exactly what the problem is.

          Show
          Adrian Greeve added a comment - Hi Damyon, Could you please provide a file with the multi-line fields that aren't importing via the moodle library? I'm interested to see exactly what the problem is.
          Hide
          Damyon Wiese added a comment -

          Hi Sorry - when I said definitely what I meant was "I am an idiot and I should only test one thing at a time".

          The multi-lines fields appear to be working correctly - there is an issue with double quotes (the horrible non ascii kind) - but that is minor and can be reported separately.

          Show
          Damyon Wiese added a comment - Hi Sorry - when I said definitely what I meant was "I am an idiot and I should only test one thing at a time". The multi-lines fields appear to be working correctly - there is an issue with double quotes (the horrible non ascii kind) - but that is minor and can be reported separately.
          Hide
          Damyon Wiese added a comment -

          Repushed without csvlib changes.

          Show
          Damyon Wiese added a comment - Repushed without csvlib changes.
          Hide
          Aparup Banerjee added a comment - - edited

          Thanks for testing that Damyon, perhaps there'll be more to add to the unit test in csvclass_test.php for that non-ascii one then..

          ps: awaiting a patch not changing csvlib then.. oh done! cool!

          Show
          Aparup Banerjee added a comment - - edited Thanks for testing that Damyon, perhaps there'll be more to add to the unit test in csvclass_test.php for that non-ascii one then.. ps: awaiting a patch not changing csvlib then.. oh done! cool!
          Hide
          Damyon Wiese added a comment -

          FYI My quote issue was caused by me not opening the csv in "UTF-8" in Open Office.

          Show
          Damyon Wiese added a comment - FYI My quote issue was caused by me not opening the csv in "UTF-8" in Open Office.
          Hide
          Aparup Banerjee added a comment -

          Thanks Damyon, while the code seems fine overall, there are a few minor phpdoc related touch ups which i thought you may want to quickly fix up before pushing this up for integration testing. (see smurf file attached)

          Show
          Aparup Banerjee added a comment - Thanks Damyon, while the code seems fine overall, there are a few minor phpdoc related touch ups which i thought you may want to quickly fix up before pushing this up for integration testing. (see smurf file attached)
          Hide
          Damyon Wiese added a comment -

          Thanks I'll go through these before I go home today.

          Show
          Damyon Wiese added a comment - Thanks I'll go through these before I go home today.
          Hide
          Damyon Wiese added a comment -

          I just pushed a commit to cleanup all the phpdoc issues in the smurf file.

          Show
          Damyon Wiese added a comment - I just pushed a commit to cleanup all the phpdoc issues in the smurf file.
          Hide
          Aparup Banerjee added a comment -

          Thanks Damyon. Thats been integrated now.

          ps: i've just added a tiny phpdoc commit too for set_editor_text

          Show
          Aparup Banerjee added a comment - Thanks Damyon. Thats been integrated now. ps: i've just added a tiny phpdoc commit too for set_editor_text
          Hide
          Aparup Banerjee added a comment -

          i've set tester to Tim Barker for his coordination on any tracker based exploratory test sessions.

          Show
          Aparup Banerjee added a comment - i've set tester to Tim Barker for his coordination on any tracker based exploratory test sessions.
          Hide
          Tim Barker added a comment -

          Great, love it, bring on the exploratory session! I'll factor it into testing tomorrow and come up with a session plan.

          Show
          Tim Barker added a comment - Great, love it, bring on the exploratory session! I'll factor it into testing tomorrow and come up with a session plan.
          Hide
          Aparup Banerjee added a comment -

          Cool the linked issues would also benefit from some exploration there as they are new features too.

          Show
          Aparup Banerjee added a comment - Cool the linked issues would also benefit from some exploration there as they are new features too.
          Hide
          Tim Barker added a comment -

          Is this old assignment, new assignment or both? I don't think it clearly specifies this anywhere.

          Show
          Tim Barker added a comment - Is this old assignment, new assignment or both? I don't think it clearly specifies this anywhere.
          Hide
          Tim Barker added a comment -

          The stable team are running a 1hr exploratory session on this. Coverage is defined in the attached sessions.

          Show
          Tim Barker added a comment - The stable team are running a 1hr exploratory session on this. Coverage is defined in the attached sessions.
          Hide
          Aparup Banerjee added a comment -

          This is the component here http://tracker.moodle.org/browse/MDL/component/12131 , so this is mod_assign.
          the old assignment is "Assignment (2.2)". Here's a good listing : http://tracker.moodle.org/browse/MDL?selectedTab=com.atlassian.jira.plugin.system.project%3Acomponents-panel

          Show
          Aparup Banerjee added a comment - This is the component here http://tracker.moodle.org/browse/MDL/component/12131 , so this is mod_assign. the old assignment is "Assignment (2.2)". Here's a good listing : http://tracker.moodle.org/browse/MDL?selectedTab=com.atlassian.jira.plugin.system.project%3Acomponents-panel
          Hide
          Tim Barker added a comment -

          I had a hunch it was mod_assign as it's one of Damyons, but best to be sure. Thanks Aparup!

          Show
          Tim Barker added a comment - I had a hunch it was mod_assign as it's one of Damyons, but best to be sure. Thanks Aparup!
          Hide
          Rajesh Taneja added a comment -
          1. on View/grade all submissions page File submissions is active link (for sorting), clicking on it gives following error:
            Debug info: Unknown column 'plugin0' in 'order clause'
            SELECT
            u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email, u.id as userid, s.status as status, s.id as submissionid, s.timecreated as firstsubmission, s.timemodified as timesubmitted, g.id as gradeid, g.grade as grade, g.timemodified as timemarked, g.timecreated as firstmarked, g.mailed as mailed, g.locked as locked, g.extensionduedate as extensionduedate
            FROM mdl_user u LEFT JOIN mdl_assign_submission s ON u.id = s.userid AND s.assignment = ? LEFT JOIN mdl_assign_grades g ON u.id = g.userid AND g.assignment = ?
            WHERE u.id IN (?,?,?,?,?)
            ORDER BY plugin0 ASC LIMIT 0, 10
            [array (
            0 => 2,
            1 => 2,
            2 => 8,
            3 => 10,
            4 => 3,
            5 => 5,
            6 => 6,
            )]
            Error code: dmlreadexception
            Stack trace:
            line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
            line 949 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
            line 1371 of /lib/tablelib.php: call to mysqli_native_moodle_database->get_records_sql()
            line 1391 of /lib/tablelib.php: call to table_sql->query_db()
            line 782 of /mod/assign/renderer.php: call to table_sql->out()
            line 686 of /mod/assign/renderer.php: call to mod_assign_renderer->flexible_table()
            line 215 of /lib/outputrenderers.php: call to mod_assign_renderer->render_assign_grading_table()
            line 2321 of /mod/assign/locallib.php: call to plugin_renderer_base->render()
            line 2355 of /mod/assign/locallib.php: call to assign->view_grading_table()
            line 415 of /mod/assign/locallib.php: call to assign->view_grading_page()
            line 53 of /mod/assign/view.php: call to assign->view()
            
          2. Similar error is observed with Feedback comments
          3. In downloaded csv, there are few fields which are probably not required, like "Final grade". As changing this doesn't do anything. Not sure if they are meant to be like that.
          Show
          Rajesh Taneja added a comment - on View/grade all submissions page File submissions is active link (for sorting), clicking on it gives following error: Debug info: Unknown column 'plugin0' in 'order clause' SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email, u.id as userid, s.status as status, s.id as submissionid, s.timecreated as firstsubmission, s.timemodified as timesubmitted, g.id as gradeid, g.grade as grade, g.timemodified as timemarked, g.timecreated as firstmarked, g.mailed as mailed, g.locked as locked, g.extensionduedate as extensionduedate FROM mdl_user u LEFT JOIN mdl_assign_submission s ON u.id = s.userid AND s.assignment = ? LEFT JOIN mdl_assign_grades g ON u.id = g.userid AND g.assignment = ? WHERE u.id IN (?,?,?,?,?) ORDER BY plugin0 ASC LIMIT 0, 10 [array ( 0 => 2, 1 => 2, 2 => 8, 3 => 10, 4 => 3, 5 => 5, 6 => 6, )] Error code: dmlreadexception Stack trace: line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown line 949 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 1371 of /lib/tablelib.php: call to mysqli_native_moodle_database->get_records_sql() line 1391 of /lib/tablelib.php: call to table_sql->query_db() line 782 of /mod/assign/renderer.php: call to table_sql->out() line 686 of /mod/assign/renderer.php: call to mod_assign_renderer->flexible_table() line 215 of /lib/outputrenderers.php: call to mod_assign_renderer->render_assign_grading_table() line 2321 of /mod/assign/locallib.php: call to plugin_renderer_base->render() line 2355 of /mod/assign/locallib.php: call to assign->view_grading_table() line 415 of /mod/assign/locallib.php: call to assign->view_grading_page() line 53 of /mod/assign/view.php: call to assign->view() Similar error is observed with Feedback comments In downloaded csv, there are few fields which are probably not required, like "Final grade". As changing this doesn't do anything. Not sure if they are meant to be like that.
          Hide
          Frédéric Massart added a comment - - edited

          Assignment without submissions

          Got this error in the content of the CSV file for an assignment (file submissions) without submissions

          Warning: fseek() expects parameter 1 to be resource  null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 415
              
          Call Stack: 
              0.0003     657344   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 
              0.2044   55767568   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53  
              0.2044   55768304   3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 
              0.2045   55768896   4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668  
              0.2045   55768896   5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344    
              0.2350   56677728   6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332  
              0.2363   56678352   7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393  
              0.2363   56678432   8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670    
              0.2363   56678432   9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602   
              0.2363   56678968  10. csv_export_writer->print_csv_data() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:465   
              0.2363   56679208  11. fseek() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:415   
              
              
          Warning: fgets() expects parameter 1 to be resource  null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 417
              
          Call Stack: 
              0.0003     657344   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 
              0.2044   55767568   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53  
              0.2044   55768304   3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 
              0.2045   55768896   4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668  
              0.2045   55768896   5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344    
              0.2350   56677728   6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332  
              0.2363   56678352   7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393  
              0.2363   56678432   8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670    
              0.2363   56678432   9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602   
              0.2363   56678968  10. csv_export_writer->print_csv_data() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:465   
              0.2450   56679848  11. fgets() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:417   
              
              
          Warning: fclose() expects parameter 1 to be resource     null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 513
              
          Call Stack: 
              0.0003     657344   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 
              0.2044   55767568   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53  
              0.2044   55768304   3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 
              0.2045   55768896   4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668  
              0.2045   55768896   5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344    
              0.2350   56677728   6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332  
              0.2363   56678352   7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393  
              0.2363   56678432   8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670    
              0.2363   56678432   9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602   
              0.2627   55637504  10. csv_export_writer->__destruct() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:0 
              0.2627   55637584  11. fclose() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:513  
              
              
          Warning: unlink(): No such file or directory in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 514  
              
          Call Stack: 
              0.0003     657344   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 
              0.2044   55767568   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53  
              0.2044   55768304   3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 
              0.2045   55768896   4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668  
              0.2045   55768896   5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344    
              0.2350   56677728   6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332  
              0.2363   56678352   7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393  
              0.2363   56678432   8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670    
              0.2363   56678432   9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602   
              0.2627   55637504  10. csv_export_writer->__destruct() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:0 
              0.2630   55637904  11. unlink() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:514  
          

          Invalid grades should not be outputted

          As the only the last 3 grades are valid, we should not display the other ones.

          Set grade for Bebe Stevens to 125
          Set grade for Kyle Broflovski to abcde
          Set grade for Timmy Burch to 9999999999999
          Set grade for Clyde Donovan to ##
          Set grade for Kenny McCormick to 10*30
          Set grade for Stan Marsh to 0
          Set grade for Jimmy Valmer to 100
          Set grade for Butters Stotch to 76
          

          Memory limit exhausted

          Using a file with thousands of lines caused the memory to reach its limit. Also, the max execution time might cause the same sort of issue here.

          File naming

          I don't understand the bit with the time in the file name, it was between 1.30pm and 2.30pm and the files were:

          • Grades-Soldier-Assign me-12-20120912_0549-comma_separated.csv
          • Grades-Soldier-Assign me-12-20120912_0605-comma_separated.csv

          What does 0549 or 0605 stand for?

          Also, my +1 for a lowercased file name without special characters such as spaces, etc...

          Using a CSV with only the 3 required fields

          It generates the following notice as the feedback column was not present. We could perhaps smoothly ignore it.

          Notice: Undefined index: index in /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/importgradeslib.php on line 164
          
          Call Stack:
              0.0003     663024   1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0
              0.2075   52284672   2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53
              0.2075   52285408   3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427
              0.2076   52286000   4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668
              0.2076   52286000   5. assign_feedback_offline->upload_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:348
              0.2331   54295784   6. moodleform->moodleform() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:265
              0.2344   54304904   7. assignfeedback_offline_import_grades_form->definition() /home/fred/www/repositories/im/moodle/lib/formslib.php:191
              0.2774   54610944   8. assignfeedback_offline_grade_importer->next() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/importgradesform.php:88
          

          Mulitple lines referencing the same participant

          Each line is displayed (and probably saved in memory). We could warn the user for duplicate entry or skip/overwrite when existing. (To be tested with duplicated and similar grades for the same participant)

          Set grade for Eric Cartman to 10
          Set grade for Eric Cartman to 10
          

          Confusion on when the grades will be updated

          I was a bit confused (and still am) on when a grade is actually updated when the overwrite checkbox is unticked. Is it required to update the modified time in the CSV? If yes, that probably needs to be explained somewhere and it did not occur to me that I had to edit this value. If not, then I had some random behaviours and cannot tell when the grade will be updated or not.

          Cheers!

          Show
          Frédéric Massart added a comment - - edited Assignment without submissions Got this error in the content of the CSV file for an assignment (file submissions) without submissions Warning: fseek() expects parameter 1 to be resource null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 415 Call Stack: 0.0003 657344 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2044 55767568 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2044 55768304 3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 0.2045 55768896 4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668 0.2045 55768896 5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344 0.2350 56677728 6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332 0.2363 56678352 7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393 0.2363 56678432 8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670 0.2363 56678432 9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602 0.2363 56678968 10. csv_export_writer->print_csv_data() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:465 0.2363 56679208 11. fseek() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:415 Warning: fgets() expects parameter 1 to be resource null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 417 Call Stack: 0.0003 657344 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2044 55767568 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2044 55768304 3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 0.2045 55768896 4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668 0.2045 55768896 5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344 0.2350 56677728 6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332 0.2363 56678352 7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393 0.2363 56678432 8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670 0.2363 56678432 9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602 0.2363 56678968 10. csv_export_writer->print_csv_data() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:465 0.2450 56679848 11. fgets() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:417 Warning: fclose() expects parameter 1 to be resource null given in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 513 Call Stack: 0.0003 657344 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2044 55767568 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2044 55768304 3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 0.2045 55768896 4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668 0.2045 55768896 5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344 0.2350 56677728 6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332 0.2363 56678352 7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393 0.2363 56678432 8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670 0.2363 56678432 9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602 0.2627 55637504 10. csv_export_writer->__destruct() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:0 0.2627 55637584 11. fclose() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:513 Warning: unlink(): No such file or directory in /home/fred/www/repositories/im/moodle/lib/csvlib.class.php on line 514 Call Stack: 0.0003 657344 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2044 55767568 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2044 55768304 3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 0.2045 55768896 4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668 0.2045 55768896 5. assign_feedback_offline->download_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:344 0.2350 56677728 6. table_sql->out() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:332 0.2363 56678352 7. flexible_table->finish_output() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1393 0.2363 56678432 8. table_text_export_format_parent->finish_document() /home/fred/www/repositories/im/moodle/lib/tablelib.php:670 0.2363 56678432 9. csv_export_writer->download_file() /home/fred/www/repositories/im/moodle/lib/tablelib.php:1602 0.2627 55637504 10. csv_export_writer->__destruct() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:0 0.2630 55637904 11. unlink() /home/fred/www/repositories/im/moodle/lib/csvlib.class.php:514 Invalid grades should not be outputted As the only the last 3 grades are valid, we should not display the other ones. Set grade for Bebe Stevens to 125 Set grade for Kyle Broflovski to abcde Set grade for Timmy Burch to 9999999999999 Set grade for Clyde Donovan to ## Set grade for Kenny McCormick to 10*30 Set grade for Stan Marsh to 0 Set grade for Jimmy Valmer to 100 Set grade for Butters Stotch to 76 Memory limit exhausted Using a file with thousands of lines caused the memory to reach its limit. Also, the max execution time might cause the same sort of issue here. File naming I don't understand the bit with the time in the file name, it was between 1.30pm and 2.30pm and the files were: Grades-Soldier-Assign me-12-20120912_0549-comma_separated.csv Grades-Soldier-Assign me-12-20120912_0605-comma_separated.csv What does 0549 or 0605 stand for? Also, my +1 for a lowercased file name without special characters such as spaces, etc... Using a CSV with only the 3 required fields It generates the following notice as the feedback column was not present. We could perhaps smoothly ignore it. Notice: Undefined index: index in /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/importgradeslib.php on line 164 Call Stack: 0.0003 663024 1. {main}() /home/fred/www/repositories/im/moodle/mod/assign/view.php:0 0.2075 52284672 2. assign->view() /home/fred/www/repositories/im/moodle/mod/assign/view.php:53 0.2075 52285408 3. assign->view_plugin_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:427 0.2076 52286000 4. assign_feedback_offline->view_page() /home/fred/www/repositories/im/moodle/mod/assign/locallib.php:1668 0.2076 52286000 5. assign_feedback_offline->upload_grades() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:348 0.2331 54295784 6. moodleform->moodleform() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/locallib.php:265 0.2344 54304904 7. assignfeedback_offline_import_grades_form->definition() /home/fred/www/repositories/im/moodle/lib/formslib.php:191 0.2774 54610944 8. assignfeedback_offline_grade_importer->next() /home/fred/www/repositories/im/moodle/mod/assign/feedback/offline/importgradesform.php:88 Mulitple lines referencing the same participant Each line is displayed (and probably saved in memory). We could warn the user for duplicate entry or skip/overwrite when existing. (To be tested with duplicated and similar grades for the same participant) Set grade for Eric Cartman to 10 Set grade for Eric Cartman to 10 Confusion on when the grades will be updated I was a bit confused (and still am) on when a grade is actually updated when the overwrite checkbox is unticked. Is it required to update the modified time in the CSV? If yes, that probably needs to be explained somewhere and it did not occur to me that I had to edit this value. If not, then I had some random behaviours and cannot tell when the grade will be updated or not. Cheers!
          Hide
          Tim Barker added a comment -

          I think the general consensus here is that the documentation is way too inadequate. I did manage to derive some areas to cover from it, but actually using the application and running tests against it has been incredibly difficult for the guys to do.

          Saying that, there is some impressive feedback from the testers in spite of the inadequate documentation.

          Show
          Tim Barker added a comment - I think the general consensus here is that the documentation is way too inadequate. I did manage to derive some areas to cover from it, but actually using the application and running tests against it has been incredibly difficult for the guys to do. Saying that, there is some impressive feedback from the testers in spite of the inadequate documentation.
          Hide
          Tim Barker added a comment -

          There is plenty of feedback from this session.

          To Summarise:

          1. Everyone disliked the docs, they were inadequate and a lot of time was burned trying to work out what to do much to everyone's frustration.
          2. There were varying severities of issues raised.
          3. It is marked as QA test required, if this is still the case please provide us with some tests.

          I'm failing this on points 1 and 2.

          Show
          Tim Barker added a comment - There is plenty of feedback from this session. To Summarise: Everyone disliked the docs, they were inadequate and a lot of time was burned trying to work out what to do much to everyone's frustration. There were varying severities of issues raised. It is marked as QA test required, if this is still the case please provide us with some tests. I'm failing this on points 1 and 2.
          Hide
          Tim Barker added a comment -

          Oh and I also forgot to mention that it was noticed that there are no unit tests. Please provide these too.

          Show
          Tim Barker added a comment - Oh and I also forgot to mention that it was noticed that there are no unit tests. Please provide these too.
          Hide
          Aparup Banerjee added a comment -

          Hi all,
          thanks for testing. can i suggest we create the issues discovered here as separate MDLs for further fixing. keeping this now in master will aide further exploratory tests from anyone out there too so imo it'd be cool to keep this in master.

          Show
          Aparup Banerjee added a comment - Hi all, thanks for testing. can i suggest we create the issues discovered here as separate MDLs for further fixing. keeping this now in master will aide further exploratory tests from anyone out there too so imo it'd be cool to keep this in master.
          Hide
          Tim Barker added a comment -

          We have raised all bugs required, Fred has several to raise which he may need to do tomorrow.

          I re-iterate that we do need unit tests and QA tests for this issue

          Any linked are linked through the test session.

          Show
          Tim Barker added a comment - We have raised all bugs required, Fred has several to raise which he may need to do tomorrow. I re-iterate that we do need unit tests and QA tests for this issue Any linked are linked through the test session.
          Hide
          Aparup Banerjee added a comment -

          Hi Tim,
          the qa_tests_required tag is there for marking as needing qa tests.
          re unit tests, thanks for bringing that up . It seems the entire mod_assign is lacking any tests directory atm, i'll add a note or subtask to the assignment subtype refactoring going on at MDL-26997.

          so if there are no blocking issues i think this can be passed.

          Show
          Aparup Banerjee added a comment - Hi Tim, the qa_tests_required tag is there for marking as needing qa tests. re unit tests, thanks for bringing that up . It seems the entire mod_assign is lacking any tests directory atm, i'll add a note or subtask to the assignment subtype refactoring going on at MDL-26997 . so if there are no blocking issues i think this can be passed.
          Hide
          Aparup Banerjee added a comment -

          unit tests implementation work -> MDL-35413

          Show
          Aparup Banerjee added a comment - unit tests implementation work -> MDL-35413
          Hide
          Tim Barker added a comment -

          OK, the docs required flag is on too, that is good as the guys had a lot of concerns about those.

          One of Fred's issues was potentially a blocker, but he has raised it elsewhere and we decided it wasn't.

          I'm just a little concerned that this is not production-ready, but as the risks have been mitigated, then it may pass. Please reset it.

          Show
          Tim Barker added a comment - OK, the docs required flag is on too, that is good as the guys had a lot of concerns about those. One of Fred's issues was potentially a blocker, but he has raised it elsewhere and we decided it wasn't. I'm just a little concerned that this is not production-ready, but as the risks have been mitigated, then it may pass. Please reset it.
          Hide
          Aparup Banerjee added a comment -

          cool Tim. its reset now for test verdict


          I'd like to clarify about development branch and being production ready : master branch is the only branch where we're not production ready as it is for development, even with our integration process (ie version 2.4dev).

          The practise has been that if there are concerns towards any issues in master then MDLs are created to further refine that. However major improvements/features will usually out weigh more minor issues resulting in a general foreseeable improvement and therefore integration into master branch.

          During the upcoming freeze, concerns/issues towards making the branch deemed as production ready (dev->alpha->beta->stable) should be addressed. Hence the follow up issues being created. More can be read/discussed here : http://docs.moodle.org/dev/Major_release_process

          Primarily i see the dev/master branch as the wider circle of testing by everyone out there. So if there's nothing blocking that testing, imo its usually fine to let it out there within master.

          Show
          Aparup Banerjee added a comment - cool Tim. its reset now for test verdict I'd like to clarify about development branch and being production ready : master branch is the only branch where we're not production ready as it is for development, even with our integration process (ie version 2.4dev). The practise has been that if there are concerns towards any issues in master then MDLs are created to further refine that. However major improvements/features will usually out weigh more minor issues resulting in a general foreseeable improvement and therefore integration into master branch. During the upcoming freeze, concerns/issues towards making the branch deemed as production ready (dev->alpha->beta->stable) should be addressed. Hence the follow up issues being created. More can be read/discussed here : http://docs.moodle.org/dev/Major_release_process Primarily i see the dev/master branch as the wider circle of testing by everyone out there. So if there's nothing blocking that testing, imo its usually fine to let it out there within master.
          Hide
          Frédéric Massart added a comment -

          (Created MDL-35435, MDL-35436 and MDL-35437 to address my comments)

          Show
          Frédéric Massart added a comment - (Created MDL-35435 , MDL-35436 and MDL-35437 to address my comments)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!
          Hide
          Raymond Antonio added a comment -

          Hi All,
          Just wondering if some trackers have been created to address Rajesh's comment on this http://tracker.moodle.org/browse/MDL-31276?focusedCommentId=178287&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-178287 ? I think I have the patches for them ready for peer review.
          Ps: will fix issues on Fred's trackers soon

          Cheers

          Show
          Raymond Antonio added a comment - Hi All, Just wondering if some trackers have been created to address Rajesh's comment on this http://tracker.moodle.org/browse/MDL-31276?focusedCommentId=178287&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-178287 ? I think I have the patches for them ready for peer review. Ps: will fix issues on Fred's trackers soon Cheers
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this is documented here http://docs.moodle.org/24/en/Assignment_settings and also here http://docs.moodle.org/24/en/Using_Assignment

          Show
          Mary Cooch added a comment - Removing docs_required label as this is documented here http://docs.moodle.org/24/en/Assignment_settings and also here http://docs.moodle.org/24/en/Using_Assignment
          Hide
          Helen Foster added a comment -

          Re-adding docs_required label, as we still need to figure out and document how the new assignment feature upload multiple feedback files in zip actually works! If anyone could explain, perhaps in the discussion https://moodle.org/mod/forum/discuss.php?d=216391 that would be much appreciated!

          Show
          Helen Foster added a comment - Re-adding docs_required label, as we still need to figure out and document how the new assignment feature upload multiple feedback files in zip actually works! If anyone could explain, perhaps in the discussion https://moodle.org/mod/forum/discuss.php?d=216391 that would be much appreciated!
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this is now documented here http://docs.moodle.org/24/en/Assignment_settings thanks to Damyon's explanation here https://moodle.org/mod/forum/discuss.php?d=216391

          Show
          Mary Cooch added a comment - Removing docs_required label as this is now documented here http://docs.moodle.org/24/en/Assignment_settings thanks to Damyon's explanation here https://moodle.org/mod/forum/discuss.php?d=216391

            People

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

              Dates

              • Created:
                Updated:
                Resolved: