From 825609ae082caf763131d282d736cb45232e36b5 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Tue, 13 Dec 2022 10:20:16 +0800 Subject: [PATCH] MDL-75898 assignfeedback_editpdf: Improve page count mismatch logic MDL-45580 introduced the readonlypages filearea, and when loading page images for an attempt, the code would check if the pages existed , creating them if not. The code inside this block also contained a guard clause for the case where no readonly pages existed - which is a situation that should not happen. Whenever readonly pages are requested, they should exist. MDL-66626 introduced a situation where page counts not matching would also retrigger page generation. However this led to a situation where the guard clause could be entered when requesting readonly pages. This patch refactors the guard clause, and improves the logic to regenerate pages. --- .../editpdf/classes/document_services.php | 26 +++++--- .../editpdf/tests/behat/annotate_pdf.feature | 61 +++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/mod/assign/feedback/editpdf/classes/document_services.php b/mod/assign/feedback/editpdf/classes/document_services.php index 739ac40d795..86e0119b806 100644 --- a/mod/assign/feedback/editpdf/classes/document_services.php +++ b/mod/assign/feedback/editpdf/classes/document_services.php @@ -580,14 +580,26 @@ EOD; } } + // This should never happen, there should be a version of the pages available + // whenever we are requesting the readonly version. + if (empty($pages) && $readonly) { + throw new \moodle_exception('Could not find readonly pages for grade ' . $grade->id); + } + + // There are two situations where the number of page images generated does not + // match the number of pages in the PDF: + // + // 1. The document conversion adhoc task was interrupted somehow (node died, solar flare, etc) + // 2. The submission has been updated by the student + // + // In the case of 1. we need to regenerate the pages, see MDL-66626. + // In the case of 2. we should do nothing, see MDL-45580. + // + // To differentiate between 1. and 2. we can check if the submission has been modified since the + // pages were generated. If it has, then we're in situation 2. $totalpagesforattempt = self::page_number_for_attempt($assignment, $userid, $attemptnumber, false); - // Here we are comparing the total number of images against the total number of pages from the combined PDF. - if (empty($pages) || count($pages) != $totalpagesforattempt) { - if ($readonly) { - // This should never happen, there should be a version of the pages available - // whenever we are requesting the readonly version. - throw new \moodle_exception('Could not find readonly pages for grade ' . $grade->id); - } + $submissionmodified = isset($pagemodified) && $submission->timemodified > $pagemodified; + if (empty($pages) || (count($pages) != $totalpagesforattempt && !$submissionmodified)) { $pages = self::generate_page_images_for_attempt($assignment, $userid, $attemptnumber, $resetrotation); } diff --git a/mod/assign/feedback/editpdf/tests/behat/annotate_pdf.feature b/mod/assign/feedback/editpdf/tests/behat/annotate_pdf.feature index 762be8cc47d..468db0d79d0 100644 --- a/mod/assign/feedback/editpdf/tests/behat/annotate_pdf.feature +++ b/mod/assign/feedback/editpdf/tests/behat/annotate_pdf.feature @@ -4,6 +4,67 @@ Feature: In an assignment, teacher can annotate PDF files during grading As a teacher I need to use the PDF editor + @javascript + Scenario: Submit a PDF file as a student and annotate the PDF as a teacher then overwrite the submission as a student + Given ghostscript is installed + And 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 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And the following "activity" exists: + | activity | assign | + | course | C1 | + | name | Test assignment name | + | assignfeedback_editpdf_enabled | 1 | + | assignfeedback_comments_enabled | 1 | + | assignsubmission_file_enabled | 1 | + | assignsubmission_file_maxfiles | 2 | + | assignsubmission_file_maxsizebytes | 102400 | + | maxfilessubmission | 2 | + | submissiondrafts | 0 | + And the following "mod_assign > submission" exists: + | assign | Test assignment name | + | user | student1 | + | file | mod/assign/feedback/editpdf/tests/fixtures/testgs.pdf | + + When I am on the "Test assignment name" Activity page logged in as teacher1 + And I follow "View all submissions" + And I click on "Grade" "link" in the "Submitted for grading" "table_row" + Then I should see "Page 1 of 1" + And I wait for the complete PDF to load + And I click on ".linebutton" "css_element" + And I draw on the pdf + And I press "Save changes" + And I should see "The changes to the grade and feedback were saved" + And I am on the "Test assignment name" Activity page logged in as student1 + And I follow "View annotated PDF..." + Then I should see "Page 1 of 1" + And I click on ".closebutton" "css_element" + And I press "Edit submission" + And I upload "mod/assign/feedback/editpdf/tests/fixtures/submission.pdf" file to "File submissions" filemanager + And I press "Save changes" + And I follow "View annotated PDF..." + Then I should see "Page 1 of 1" + And I am on the "Test assignment name" Activity page logged in as teacher1 + And I follow "View all submissions" + And I click on "Grade" "link" in the "Submitted for grading" "table_row" + Then I should see "Page 1 of 3" + And I wait for the complete PDF to load + And I click on ".linebutton" "css_element" + And I draw on the pdf + And I press "Save changes" + And I should see "The changes to the grade and feedback were saved" + And I am on the "Test assignment name" Activity page logged in as student1 + And I follow "View annotated PDF..." + Then I should see "Page 1 of 3" + @javascript Scenario: Submit a PDF file as a student and annotate the PDF as a teacher Given ghostscript is installed