Moodle
  1. Moodle
  2. MDL-34794

Reset Course not implemented in New assignment

    Details

    • Testing Instructions:
      Hide

      Login as editing teacher (or above)
      Make sure the course has a start date of today
      Create an assignment with a due date in 1 weeks time and a cut off date in 2 weeks time (cut off date only in 2.4)
      Login as a student and make a submission
      Login as an editing teacher
      Reset the course (In the course menu)
      Reset Options:
      Change the start date to be in 1 weeks time
      Select "Assignment submissions"
      Verify that the student submissions for the assignment were deleted and the due date and cut off date were incremented by a week (cut off date only in 2.4).

      Show
      Login as editing teacher (or above) Make sure the course has a start date of today Create an assignment with a due date in 1 weeks time and a cut off date in 2 weeks time (cut off date only in 2.4) Login as a student and make a submission Login as an editing teacher Reset the course (In the course menu) Reset Options: Change the start date to be in 1 weeks time Select "Assignment submissions" Verify that the student submissions for the assignment were deleted and the due date and cut off date were incremented by a week (cut off date only in 2.4).
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      43289

      Description

      The new assignment module does not have the course reset implemented.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          Raymond Antonio added a comment - - edited

          Hi Michael and Jacob,

          This is my proposed patch for adding the reset course functionality and it sits on my github repo : MDL-34794
          https://github.com/raymondAntonio/moodle/tree/MDL-34794

          and here is the diff:

          https://github.com/raymondAntonio/moodle/commit/21a17eb8ff712199cffa72bb8daadc46f347f206

          Cheers

          Show
          Raymond Antonio added a comment - - edited Hi Michael and Jacob, This is my proposed patch for adding the reset course functionality and it sits on my github repo : MDL-34794 https://github.com/raymondAntonio/moodle/tree/MDL-34794 and here is the diff: https://github.com/raymondAntonio/moodle/commit/21a17eb8ff712199cffa72bb8daadc46f347f206 Cheers
          Hide
          Raymond Antonio added a comment -

          Hi Damyon,

          This is an updated patch for adding the reset course functionality after taking your feedbacks into account. Thanks

          and here is the new diff:

          https://github.com/raymondAntonio/moodle/commit/d40202a02f4b0f4d7e03bc652c34ac2e664699d2

          Cheers

          Show
          Raymond Antonio added a comment - Hi Damyon, This is an updated patch for adding the reset course functionality after taking your feedbacks into account. Thanks and here is the new diff: https://github.com/raymondAntonio/moodle/commit/d40202a02f4b0f4d7e03bc652c34ac2e664699d2 Cheers
          Hide
          Damyon Wiese added a comment -

          Hi Raymond can you rebase this and add "cutoffdate" to this list of dates that get shifted?

           shift_course_mod_dates('assign', array('duedate', 'allowsubmissionsfromdate'), $data->timeshift, $data->courseid);
          

          Thanks, Damyon

          Show
          Damyon Wiese added a comment - Hi Raymond can you rebase this and add "cutoffdate" to this list of dates that get shifted? shift_course_mod_dates('assign', array('duedate', 'allowsubmissionsfromdate'), $data->timeshift, $data->courseid); Thanks, Damyon
          Hide
          Raymond Antonio added a comment -

          Hi Damyon,

          Thanks for your feedback and 'cutoffdate' has been added . Cheers

          and here is the update diff:

          https://github.com/raymondAntonio/moodle/commit/0be5d09f091ab2edea5a599a8b6ae7213c02cdb0

          Cheers
          Ps: this one needs reviewing too http://tracker.moodle.org/browse/MDL-34088

          Show
          Raymond Antonio added a comment - Hi Damyon, Thanks for your feedback and 'cutoffdate' has been added . Cheers and here is the update diff: https://github.com/raymondAntonio/moodle/commit/0be5d09f091ab2edea5a599a8b6ae7213c02cdb0 Cheers Ps: this one needs reviewing too http://tracker.moodle.org/browse/MDL-34088
          Hide
          Damyon Wiese added a comment -

          Hi Raymond - this needs a little fixing

          assign_reset_userdata in mod/assign/lib.php is not correct - you are attempting to take a course and map it to a single assignment - there will be 0, 1 or many assignments per course.

          This is the bit that is wrong:

          get_coursemodule_from_instance('assign', $data->courseid, 0, false, MUST_EXIST); 
          

          This also needs rebasing to fix a conflict in the language file.

          Thanks, Damyon

          Show
          Damyon Wiese added a comment - Hi Raymond - this needs a little fixing assign_reset_userdata in mod/assign/lib.php is not correct - you are attempting to take a course and map it to a single assignment - there will be 0, 1 or many assignments per course. This is the bit that is wrong: get_coursemodule_from_instance('assign', $data->courseid, 0, false , MUST_EXIST); This also needs rebasing to fix a conflict in the language file. Thanks, Damyon
          Hide
          Raymond Antonio added a comment - - edited

          Hi Damyon,

          Thank you for your comment. It has been fixed now and the details are as follows:
          1. This the repo
          git://github.com/raymondAntonio/moodle.git
          2. This is the branch
          MDL-34794
          And this is the updated diff :
          https://github.com/raymondAntonio/moodle/commit/72429a10dac3d65cfa2d7c5923038de3e43ccac9
          Cheers

          Show
          Raymond Antonio added a comment - - edited Hi Damyon, Thank you for your comment. It has been fixed now and the details are as follows: 1. This the repo git://github.com/raymondAntonio/moodle.git 2. This is the branch MDL-34794 And this is the updated diff : https://github.com/raymondAntonio/moodle/commit/72429a10dac3d65cfa2d7c5923038de3e43ccac9 Cheers
          Hide
          Damyon Wiese added a comment -

          This looks ready for integration, thanks Raymond.

          Show
          Damyon Wiese added a comment - This looks ready for integration, thanks Raymond.
          Hide
          Sam Hemelryk added a comment -

          Thanks Raymond + Damyon, this has been integrated now onto master, soon to be 2.4.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Raymond + Damyon, this has been integrated now onto master, soon to be 2.4. Cheers Sam
          Hide
          Tim Barker added a comment -

          No testing instructions. Does it need them? If not can we have unit tests?

          Show
          Tim Barker added a comment - No testing instructions. Does it need them? If not can we have unit tests?
          Hide
          Damyon Wiese added a comment -

          Sorry Tim, instructions added.

          Show
          Damyon Wiese added a comment - Sorry Tim, instructions added.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Re-sending to testing.

          I think this is not a good candidate for unit testing (as far as the preparation is quite complex).

          Offtopic reflexion:

          Perhaps we could think about some way to have a general and simple generator for each module able to centralize all the preparation. So later testing of features (phpunit, selenium...) against would be easier/quicker.

          Uhm... individual activity backups may be a good way to provide preparation nearly instantly (for activity module testing). And, at the same time, restore (of that activity module) will be test-convered automatically.

          And those exemplary backup files could be used also by selenium/friends to start with a nice environment already created.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Re-sending to testing. I think this is not a good candidate for unit testing (as far as the preparation is quite complex). Offtopic reflexion: Perhaps we could think about some way to have a general and simple generator for each module able to centralize all the preparation. So later testing of features (phpunit, selenium...) against would be easier/quicker. Uhm... individual activity backups may be a good way to provide preparation nearly instantly (for activity module testing). And, at the same time, restore (of that activity module) will be test-convered automatically. And those exemplary backup files could be used also by selenium/friends to start with a nice environment already created. Ciao
          Hide
          Rajesh Taneja added a comment -

          Sorry guys failing it because I am getting following exception
          Coding error detected, it must be fixed by a programmer: The property "context" is not set.

          Debug info: 
          Error code: codingerror
          Stack trace:
          line 668 of /grade/grading/lib.php: coding_exception thrown
          line 331 of /grade/grading/lib.php: call to grading_manager->ensure_isset()
          line 530 of /grade/grading/lib.php: call to grading_manager->get_active_method()
          line 370 of /mod/assign/feedback/offline/locallib.php: call to grading_manager->get_active_controller()
          line 4394 of /mod/assign/locallib.php: call to assign_feedback_offline->is_enabled()
          line 858 of /mod/assign/lib.php: call to assign->get_user_grades_for_gradebook()
          line 875 of /mod/assign/lib.php: call to assign_get_user_grades()
          line 1190 of /lib/gradelib.php: call to assign_update_grades()
          line 1160 of /lib/gradelib.php: call to grade_update_mod_grades()
          line 1398 of /lib/gradelib.php: call to grade_grab_course_grades()
          line 5060 of /lib/moodlelib.php: call to grade_course_reset()
          line 73 of /course/reset.php: call to reset_course_userdata()
          

          Steps to replicate:
          Follow test instructions about reset course as admin, and click "select default". This will select "Delete all grades" in gradebook. Now click Reset course and boom.. You see this error. Also start date for assignment is changed to 5 June, 1970 and due and cut-off are 1 and 2 weeks apart respectively (which is fine).

          FYI:
          Reset course with only Change start date" and *Delete all submissions works as described.

          Failing as this fix is causing exception with other settings on same page.

          Show
          Rajesh Taneja added a comment - Sorry guys failing it because I am getting following exception Coding error detected, it must be fixed by a programmer: The property "context" is not set. Debug info: Error code: codingerror Stack trace: line 668 of /grade/grading/lib.php: coding_exception thrown line 331 of /grade/grading/lib.php: call to grading_manager->ensure_isset() line 530 of /grade/grading/lib.php: call to grading_manager->get_active_method() line 370 of /mod/assign/feedback/offline/locallib.php: call to grading_manager->get_active_controller() line 4394 of /mod/assign/locallib.php: call to assign_feedback_offline->is_enabled() line 858 of /mod/assign/lib.php: call to assign->get_user_grades_for_gradebook() line 875 of /mod/assign/lib.php: call to assign_get_user_grades() line 1190 of /lib/gradelib.php: call to assign_update_grades() line 1160 of /lib/gradelib.php: call to grade_update_mod_grades() line 1398 of /lib/gradelib.php: call to grade_grab_course_grades() line 5060 of /lib/moodlelib.php: call to grade_course_reset() line 73 of /course/reset.php: call to reset_course_userdata() Steps to replicate: Follow test instructions about reset course as admin, and click "select default". This will select "Delete all grades" in gradebook. Now click Reset course and boom.. You see this error. Also start date for assignment is changed to 5 June, 1970 and due and cut-off are 1 and 2 weeks apart respectively (which is fine). FYI: Reset course with only Change start date" and *Delete all submissions works as described. Failing as this fix is causing exception with other settings on same page.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has bee reverted (master only), reopening...

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This has bee reverted (master only), reopening... Ciao
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Raymond Antonio added a comment -

          IMO, this error is caused by assign_get_user_grades() callback whose the patch is in MDL-35389 (MDL-35389 is blocking this issue and MDL-34794 (could be more other issues related to the grade book and the assignment).

          Show
          Raymond Antonio added a comment - IMO, this error is caused by assign_get_user_grades() callback whose the patch is in MDL-35389 ( MDL-35389 is blocking this issue and MDL-34794 (could be more other issues related to the grade book and the assignment).
          Hide
          Raymond Antonio added a comment -

          Just rebased and reapplied the patch after this blocker (MDL-35389) triggering this error http://tracker.moodle.org/browse/MDL-34794?focusedCommentId=178682&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-178682
          has been solved . It is now ready for review and retesting again. Cheers

          Show
          Raymond Antonio added a comment - Just rebased and reapplied the patch after this blocker ( MDL-35389 ) triggering this error http://tracker.moodle.org/browse/MDL-34794?focusedCommentId=178682&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-178682 has been solved . It is now ready for review and retesting again. Cheers
          Hide
          Raymond Antonio added a comment -

          Forgot to add pending this blocker in 2.3 MDL-35665

          Show
          Raymond Antonio added a comment - Forgot to add pending this blocker in 2.3 MDL-35665
          Hide
          Damyon Wiese added a comment -

          I added some comments on the github diff Raymond - also this needs backporting for 2.3.

          Show
          Damyon Wiese added a comment - I added some comments on the github diff Raymond - also this needs backporting for 2.3.
          Hide
          Raymond Antonio added a comment -

          Hi Damyon,

          Thank you for your feedbacks. It has been fixed now and the backport for 2.3 Branch will be added soon. Cheers

          Show
          Raymond Antonio added a comment - Hi Damyon, Thank you for your feedbacks. It has been fixed now and the backport for 2.3 Branch will be added soon. Cheers
          Hide
          Raymond Antonio added a comment -

          2.3 Branch has been added. Cheers

          Show
          Raymond Antonio added a comment - 2.3 Branch has been added. Cheers
          Hide
          Damyon Wiese added a comment -

          Thanks Raymond, this looks ready to me (I just tested this on 2.3 and 2.4 with no errors).

          Show
          Damyon Wiese added a comment - Thanks Raymond, this looks ready to me (I just tested this on 2.3 and 2.4 with no errors).
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, 23 and master, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, 23 and master, thanks!
          Hide
          Rajesh Taneja added a comment -

          Works Great,
          Thanks for fixing this, Raymond.

          Show
          Rajesh Taneja added a comment - Works Great, Thanks for fixing this, Raymond.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          From somewhere within the clouds...

          Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - From somewhere within the clouds... Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration! Ciao
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as I don't think something that now works as expected should need extra docs but please say if you disagree.

          Show
          Mary Cooch added a comment - Removing docs_required label as I don't think something that now works as expected should need extra docs but please say if you disagree.
          Hide
          Damyon Wiese added a comment -

          I agree Mary

          Show
          Damyon Wiese added a comment - I agree Mary

            People

            • Votes:
              10 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: