From 181c20bc526867108c9d5803ec9eb394aa2299d0 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Mon, 2 May 2016 15:45:32 +0800 Subject: [PATCH 1/2] MDL-53601 mod_assign: Feedback files are sent to all group members. --- mod/assign/feedback/editpdf/locallib.php | 86 +++++++++++++----------- mod/assign/feedback/file/locallib.php | 2 +- mod/assign/feedback/offline/locallib.php | 11 +++ mod/assign/feedbackplugin.php | 4 +- 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/mod/assign/feedback/editpdf/locallib.php b/mod/assign/feedback/editpdf/locallib.php index b58757a5e21..a9a74e5e027 100644 --- a/mod/assign/feedback/editpdf/locallib.php +++ b/mod/assign/feedback/editpdf/locallib.php @@ -188,53 +188,61 @@ class assign_feedback_editpdf extends assign_feedback_plugin { * @return boolean True if the pdf has been modified, else false. */ public function is_feedback_modified(stdClass $grade, stdClass $data) { - global $USER; - $pagenumbercount = document_services::page_number_for_attempt($this->assignment, $grade->userid, $grade->attemptnumber); - for ($i = 0; $i < $pagenumbercount; $i++) { - // Select all annotations. - $draftannotations = page_editor::get_annotations($grade->id, $i, true); - $nondraftannotations = page_editor::get_annotations($grade->id, $i, false); - // Check to see if the count is the same. - if (count($draftannotations) != count($nondraftannotations)) { - // The count is different so we have a modification. - return true; - } else { - $matches = 0; - // Have a closer look and see if the draft files match all the non draft files. - foreach ($nondraftannotations as $ndannotation) { - foreach ($draftannotations as $dannotation) { - foreach ($ndannotation as $key => $value) { - if ($key != 'id' && $value != $dannotation->{$key}) { - continue 2; + // We only need to know if the source user's PDF has changed. If so then all + // following users will have the same status. If it's only an individual annotation + // then only one user will come through this method. + // Source user id is only added to the form if there was a pdf. + if (!empty($data->editpdf_source_userid)) { + $sourceuserid = $data->editpdf_source_userid; + // Retrieve the grade information for the source user. + $sourcegrade = $this->assignment->get_user_grade($sourceuserid, true, $grade->attemptnumber); + $pagenumbercount = document_services::page_number_for_attempt($this->assignment, $sourceuserid, $sourcegrade->attemptnumber); + for ($i = 0; $i < $pagenumbercount; $i++) { + // Select all annotations. + $draftannotations = page_editor::get_annotations($sourcegrade->id, $i, true); + $nondraftannotations = page_editor::get_annotations($grade->id, $i, false); + // Check to see if the count is the same. + if (count($draftannotations) != count($nondraftannotations)) { + // The count is different so we have a modification. + return true; + } else { + $matches = 0; + // Have a closer look and see if the draft files match all the non draft files. + foreach ($nondraftannotations as $ndannotation) { + foreach ($draftannotations as $dannotation) { + foreach ($ndannotation as $key => $value) { + if ($key != 'id' && $value != $dannotation->{$key}) { + continue 2; + } } + $matches++; } - $matches++; + } + if ($matches !== count($nondraftannotations)) { + return true; } } - if ($matches !== count($nondraftannotations)) { + // Select all comments. + $draftcomments = page_editor::get_comments($sourcegrade->id, $i, true); + $nondraftcomments = page_editor::get_comments($grade->id, $i, false); + if (count($draftcomments) != count($nondraftcomments)) { return true; - } - } - // Select all comments. - $draftcomments = page_editor::get_comments($grade->id, $i, true); - $nondraftcomments = page_editor::get_comments($grade->id, $i, false); - if (count($draftcomments) != count($nondraftcomments)) { - return true; - } else { - // Go for a closer inspection. - $matches = 0; - foreach ($nondraftcomments as $ndcomment) { - foreach ($draftcomments as $dcomment) { - foreach ($ndcomment as $key => $value) { - if ($key != 'id' && $value != $dcomment->{$key}) { - continue 2; + } else { + // Go for a closer inspection. + $matches = 0; + foreach ($nondraftcomments as $ndcomment) { + foreach ($draftcomments as $dcomment) { + foreach ($ndcomment as $key => $value) { + if ($key != 'id' && $value != $dcomment->{$key}) { + continue 2; + } } + $matches++; } - $matches++; } - } - if ($matches !== count($nondraftcomments)) { - return true; + if ($matches !== count($nondraftcomments)) { + return true; + } } } } diff --git a/mod/assign/feedback/file/locallib.php b/mod/assign/feedback/file/locallib.php index 859701141d9..41e0c6c058d 100644 --- a/mod/assign/feedback/file/locallib.php +++ b/mod/assign/feedback/file/locallib.php @@ -90,7 +90,7 @@ class assign_feedback_file extends assign_feedback_plugin { $filekey = null; $draftareainfo = null; foreach ($data as $key => $value) { - if (strpos($key, 'files_') === 0) { + if (strpos($key, 'files_') === 0 && strpos($key, '_filemanager')) { $filekey = $key; } } diff --git a/mod/assign/feedback/offline/locallib.php b/mod/assign/feedback/offline/locallib.php index 798fd3e0b9d..e5ab1111ba9 100644 --- a/mod/assign/feedback/offline/locallib.php +++ b/mod/assign/feedback/offline/locallib.php @@ -67,6 +67,17 @@ class assign_feedback_offline extends assign_feedback_plugin { return true; } + /** + * This plugin does not save through the normal interface so this returns false. + * + * @param stdClass $grade The grade. + * @param stdClass $data Form data from the feedback form. + * @return boolean - False + */ + public function is_feedback_modified(stdClass $grade, stdClass $data) { + return false; + } + /** * Loop through uploaded grades and update the grades for this assignment * diff --git a/mod/assign/feedbackplugin.php b/mod/assign/feedbackplugin.php index 5c083e1b9f4..340b4352933 100644 --- a/mod/assign/feedbackplugin.php +++ b/mod/assign/feedbackplugin.php @@ -120,8 +120,8 @@ abstract class assign_feedback_plugin extends assign_plugin { * @return boolean - True if the form element has been modified. */ public function is_feedback_modified(stdClass $grade, stdClass $data) { - debugging('This plugin has not overwritten the is_feedback_modified() method. Please add this method to your plugin', - DEBUG_DEVELOPER); + debugging('This plugin (' . $this->get_name() . ') has not overwritten the is_feedback_modified() method. + Please add this method to your plugin', DEBUG_DEVELOPER); return true; } From 1beb95af48b32b0dd756dfe2e5dfc7e84651d649 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Thu, 7 Apr 2016 13:29:35 +0800 Subject: [PATCH 2/2] MDL-53601 mod_assign: Behat test for group feedback. --- .../tests/behat/group_annotations.feature | 28 +++++- .../file/tests/behat/feedback_file.feature | 89 +++++++++++++++++++ .../feedback/file/tests/fixtures/feedback.txt | 1 + .../file/tests/fixtures/submission.txt | 1 + 4 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 mod/assign/feedback/file/tests/behat/feedback_file.feature create mode 100644 mod/assign/feedback/file/tests/fixtures/feedback.txt create mode 100644 mod/assign/feedback/file/tests/fixtures/submission.txt diff --git a/mod/assign/feedback/editpdf/tests/behat/group_annotations.feature b/mod/assign/feedback/editpdf/tests/behat/group_annotations.feature index f397c61d231..4292b101855 100644 --- a/mod/assign/feedback/editpdf/tests/behat/group_annotations.feature +++ b/mod/assign/feedback/editpdf/tests/behat/group_annotations.feature @@ -4,8 +4,7 @@ Feature: In a group assignment, teacher can annotate PDF files for all users As a teacher I need to use the PDF editor for a group assignment - @javascript - Scenario: Submit a PDF file as a student and annotate the PDF as a teacher + Background: Given ghostscript is installed And the following "courses" exist: | fullname | shortname | category | groupmode | @@ -56,6 +55,10 @@ Feature: In a group assignment, teacher can annotate PDF files for all users And I click on ".stampbutton" "css_element" And I draw on the pdf And I wait until the page is ready + + @javascript + Scenario: Submit a PDF file as a student and annotate the PDF as a teacher + Given I set the field "applytoall" to "0" And I press "Save changes" And I should see "The changes to the grade and feedback were saved" And I press "Ok" @@ -73,3 +76,24 @@ Feature: In a group assignment, teacher can annotate PDF files for all users And I follow "Course 1" And I follow "Test assignment name" And I should not see "View annotated PDF..." + + @javascript + Scenario: Submit a PDF file as a student and annotate the PDF as a teacher and all students in the group get a copy of the annotated PDF. + Given I press "Save changes" + And I click on "Ok" "button" + And I follow "Course 1" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + When I follow "View annotated PDF..." + And I change window size to "large" + Then I should see "Annotate PDF" + And I change window size to "medium" + And I wait until the page is ready + And I click on "Close" "button" + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + And I should see "View annotated PDF..." diff --git a/mod/assign/feedback/file/tests/behat/feedback_file.feature b/mod/assign/feedback/file/tests/behat/feedback_file.feature new file mode 100644 index 00000000000..78abd6c2fda --- /dev/null +++ b/mod/assign/feedback/file/tests/behat/feedback_file.feature @@ -0,0 +1,89 @@ +@mod @mod_assign @assignfeedback @assignfeedback_file @_file_upload +Feature: In an assignment, teacher can submit feedback files during grading + In order to provide a feedback file + As a teacher + I need to submit a feedback file. + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "groups" exist: + | name | course | idnumber | + | G1 | C1 | G1 | + And the following "group members" exist: + | user | group | + | student1 | G1 | + | student2 | G1 | + And I log in as "teacher1" + And I follow "Course 1" + And I turn editing mode on + And I add a "Assignment" to section "1" and I fill the form with: + | Assignment name | Test assignment name | + | Description | Submit your PDF file | + | Maximum number of uploaded files | 2 | + | Students submit in groups | Yes | + And I follow "Test assignment name" + And I follow "Edit settings" + And I follow "Expand all" + And I set the field "assignfeedback_file_enabled" to "1" + And I press "Save and display" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I press "Add submission" + And I upload "mod/assign/feedback/file/tests/fixtures/submission.txt" file to "File submissions" filemanager + And I press "Save changes" + And I should see "Submitted for grading" + And I should see "submission.txt" + And I should see "Not graded" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Test assignment name" + And I click on "Grade" + And I upload "mod/assign/feedback/file/tests/fixtures/feedback.txt" file to "Feedback files" filemanager + + @javascript + Scenario: A teacher can provide a feedback file when grading an assignment. + Given I set the field "applytoall" to "0" + And I press "Save changes" + And I click "Ok" "button" + And I follow "Course 1" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I should see "feedback.txt" + And I log out + When I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + Then I should not see "feedback.txt" + + @javascript + Scenario: A teacher can provide a feedback file when grading an assignment and all students in the group will receive the file. + Given I press "Save changes" + And I click "Ok" "button" + And I follow "Course 1" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I should see "feedback.txt" + And I log out + When I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + Then I should see "feedback.txt" \ No newline at end of file diff --git a/mod/assign/feedback/file/tests/fixtures/feedback.txt b/mod/assign/feedback/file/tests/fixtures/feedback.txt new file mode 100644 index 00000000000..bbbe0011f87 --- /dev/null +++ b/mod/assign/feedback/file/tests/fixtures/feedback.txt @@ -0,0 +1 @@ +feedback.txt \ No newline at end of file diff --git a/mod/assign/feedback/file/tests/fixtures/submission.txt b/mod/assign/feedback/file/tests/fixtures/submission.txt new file mode 100644 index 00000000000..e31c4569647 --- /dev/null +++ b/mod/assign/feedback/file/tests/fixtures/submission.txt @@ -0,0 +1 @@ +submission.txt \ No newline at end of file