From 5a469543b0c19d7f5eac1451e94576e909bdf92d Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Wed, 16 Aug 2017 15:21:33 +0800 Subject: [PATCH 1/2] MDL-55849 mod_assign: Reopening assignments only increases by one. Group assignments when being reopened will only increase the attempt total by one. --- mod/assign/locallib.php | 57 ++++++++++++++++++++++++++++++++++++----- mod/assign/upgrade.txt | 4 +++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 76312d630a3..df9d54b86d9 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -167,6 +167,12 @@ class assign { /** @var array cached list of IDs of users who share group membership with the user. The cache key will be the user. */ private $sharedgroupmembers = array(); + /** + * @var stdClass The most recent team submission. Used to determine additional attempt numbers and whether + * to update the gradebook. + */ + private $mostrecentteamsubmission = null; + /** * Constructor for the base assign class. * @@ -2473,13 +2479,16 @@ class assign { $submission = null; if ($this->get_instance()->teamsubmission) { - $submission = $this->get_group_submission($grade->userid, 0, false); + if (isset($this->mostrecentteamsubmission)) { + $submission = $this->mostrecentteamsubmission; + } else { + $submission = $this->get_group_submission($grade->userid, 0, false); + } } else { $submission = $this->get_user_submission($grade->userid, false); } - // Only push to gradebook if the update is for the latest attempt. - // Not the latest attempt. + // Only push to gradebook if the update is for the most recent attempt. if ($submission && $submission->attemptnumber != $grade->attemptnumber) { return true; } @@ -7930,6 +7939,12 @@ class assign { $instance = $this->get_instance(); $submission = null; if ($instance->teamsubmission) { + // We need to know what the most recent group submission is. + // Specifically when determining if we are adding another attempt (we only want to add one attempt per team), + // and when deciding if we need to update the gradebook with an edited grade. + $mostrecentsubmission = $this->get_group_submission($userid, 0, false, -1); + $this->set_most_recent_team_submission($mostrecentsubmission); + // Get the submission that we are saving grades for. The data attempt number determines which submission attempt. $submission = $this->get_group_submission($userid, 0, false, $data->attemptnumber); } else { $submission = $this->get_user_submission($userid, false, $data->attemptnumber); @@ -7944,8 +7959,10 @@ class assign { } $members = $this->get_submission_group_members($groupid, true, $this->show_only_active_users()); foreach ($members as $member) { - // User may exist in multple groups (which should put them in the default group). - $this->apply_grade_to_user($data, $member->id, $data->attemptnumber); + // We only want to update the grade for this group submission attempt. The data attempt number could be + // -1 which may end up in additional attempts being created for each group member instead of just one + // additional attempt for the group. + $this->apply_grade_to_user($data, $member->id, $submission->attemptnumber); $this->process_outcomes($member->id, $data, $userid); } } else { @@ -8116,6 +8133,11 @@ class assign { } if (empty($groupsprocessed[$groupid])) { + // We need to know what the most recent group submission is. + // Specifically when determining if we are adding another attempt (we only want to add one attempt per team), + // and when deciding if we need to update the gradebook with an edited grade. + $currentsubmission = $this->get_group_submission($userid, 0, false, -1); + $this->set_most_recent_team_submission($currentsubmission); $result = $this->process_add_attempt($userid) && $result; $groupsprocessed[$groupid] = true; } @@ -8166,7 +8188,18 @@ class assign { // Create the new submission record for the group/user. if ($this->get_instance()->teamsubmission) { - $newsubmission = $this->get_group_submission($userid, 0, true, $oldsubmission->attemptnumber + 1); + if (isset($this->mostrecentteamsubmission)) { + // Team submissions can end up in this function for each user (via save_grade). We don't want to create + // more than one attempt for the whole team. + if ($this->mostrecentteamsubmission->attemptnumber == $oldsubmission->attemptnumber) { + $newsubmission = $this->get_group_submission($userid, 0, true, $oldsubmission->attemptnumber + 1); + } else { + $newsubmission = $this->get_group_submission($userid, 0, false, $oldsubmission->attemptnumber); + } + } else { + debugging('Please use set_most_recent_team_submission() before calling add_attempt', DEBUG_DEVELOPER); + $newsubmission = $this->get_group_submission($userid, 0, true, $oldsubmission->attemptnumber + 1); + } } else { $newsubmission = $this->get_user_submission($userid, true, $oldsubmission->attemptnumber + 1); } @@ -8630,6 +8663,18 @@ class assign { return $o; } + + /** + * Set the most recent submission for the team. + * The most recent team submission is used to determine if another attempt should be created when allowing another + * attempt on a group assignment, and whether the gradebook should be updated. + * + * @since Moodle 3.3.3 + * @param stdClass $submission The most recent submission of the group. + */ + public function set_most_recent_team_submission($submission) { + $this->mostrecentteamsubmission = $submission; + } } /** diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index 0d99f0776db..18611afefeb 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -1,5 +1,9 @@ This files describes API changes in the assign code. +=== 3.3.3 === +* assign::add_attempt requires that set_most_recent_team_submission() be called if attempting to use this function with a team + submission. + === 3.3.2 === * assign_refresh_events() Now takes two additional parameters to refine the update to a specific instance. This function now optionally takes the module instance object or ID, and the course module object or ID. Please try to send the full From 9a1d82de52e3689eb9e74ecf7d02e2f6d8db52df Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Thu, 31 Aug 2017 11:10:58 +0800 Subject: [PATCH 2/2] MDL-55849 mod_assign: Behat update for group submission reopening. --- .../tests/behat/allow_another_attempt.feature | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/mod/assign/tests/behat/allow_another_attempt.feature b/mod/assign/tests/behat/allow_another_attempt.feature index ba940f59925..29fdb6bbfad 100644 --- a/mod/assign/tests/behat/allow_another_attempt.feature +++ b/mod/assign/tests/behat/allow_another_attempt.feature @@ -126,42 +126,43 @@ Feature: In an assignment, students start a new attempt based on their previous | operation | Allow another attempt | And I click on "Go" "button" confirming the dialogue And I should not see "The grades were not saved because someone has modified one or more records more recently than when you loaded the page." -# Behat tests for the group submission, should be uncommented once the MDL-48216 is fixed. -# And I log out -# And I log in as "student3" -# And I am on "Course 1" course homepage -# And I follow "Test assignment name" -# #And I should see "This is attempt 1 ( 3 attempts allowed )." -# And I press "Add submission" -# And I set the following fields to these values: -# | Online text | I'm the student's 3 group 2 first attempt | -# And I press "Save changes" -# And I log out -# And I log in as "teacher1" -# And I am on "Course 1" course homepage -# And I follow "Test assignment name" -# And I navigate to "View all submissions" in current page administration -# And "Student 1" row "Status" column of "generaltable" table should contain "Reopened" -# And "Student 2" row "Status" column of "generaltable" table should contain "Reopened" -# And "Student 3" row "Status" column of "generaltable" table should contain "Submitted for grading" -# And "Student 4" row "Status" column of "generaltable" table should contain "Submitted for grading" -# And I click on "Grade " "link" in the "Student 3" "table_row" -# And I set the following fields to these values: -# | Allow another attempt | 1 | -# And I press "Save changes" -# And I log out -# And I log in as "student4" -# And I am on "Course 1" course homepage -# And I follow "Test assignment name" -# #And I should see "This is attempt 2 ( 3 attempts allowed )." -# And I press "Add submission" -# And I set the following fields to these values: -# | Online text | I'm the student's 4 group 2 second attempt | -# And I press "Save changes" -# And I log out -# And I log in as "teacher1" -# And I am on "Course 1" course homepage -# And I follow "Test assignment name" -# I navigate to "View all submissions" in current page administration -# And I click on "Grade" "link" in the "Student 1" "table_row" - #And I should see "This is attempt 2 (3 attempts allowed)" + And I log out + And I log in as "student3" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I should see "This is attempt 1 ( 3 attempts allowed )." + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student's 3 group 2 first attempt | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And "Student 1" row "Status" column of "generaltable" table should contain "Reopened" + And "Student 2" row "Status" column of "generaltable" table should contain "Reopened" + And "Student 3" row "Status" column of "generaltable" table should contain "Submitted for grading" + And "Student 4" row "Status" column of "generaltable" table should contain "Submitted for grading" + And I click on "Grade" "link" in the "Student 3" "table_row" + And I set the following fields to these values: + | Allow another attempt | 1 | + And I press "Save changes" + And I press "Ok" + And I follow "Assignment: Test assignment name" + And I log out + And I log in as "student4" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I should see "This is attempt 2 ( 3 attempts allowed )." + And I press "Add a new attempt" + And I set the following fields to these values: + | Online text | I'm the student's 4 group 2 second attempt | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I select "Group 2" from the "group" singleselect + And I click on "Grade" "link" in the ".submissionlinks" "css_element" + And I should see "2" in the "#id_attemptsettings" "css_element"