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 diff --git a/mod/assign/feedback/editpdf/tests/behat/behat_assignfeedback_editpdf.php b/mod/assign/feedback/editpdf/tests/behat/behat_assignfeedback_editpdf.php index c6c69d686c5..a6bf12b0490 100644 --- a/mod/assign/feedback/editpdf/tests/behat/behat_assignfeedback_editpdf.php +++ b/mod/assign/feedback/editpdf/tests/behat/behat_assignfeedback_editpdf.php @@ -56,23 +56,47 @@ class behat_assignfeedback_editpdf extends behat_base { * @When /^I draw on the pdf$/ */ public function i_draw_on_the_pdf() { + // There appears to be a bug with detecting changes to + // annotations. If a PDF is annotated, then the student + // updates the submission, if the teacher then draws the + // exact same annotations on the new PDF, the readonly + // pages are not updated and the student's view of the + // annotated PDF still shows the old PDF. So we add some + // randomness in this step to ensure the annotations are + // different every time. + // + // This was added to test MDL-75898. See MDL-76659 for + // more details about the bug. + // + // Note - the start, move and end locations must all be different. + // If they are the same, it's possible the PDF tool selected does not activate. + $startx = 100 + rand(0, 50); + $starty = 250 + rand(0, 50); $js = ' (function() { var instance = M.assignfeedback_editpdf.instance; - var event = { clientX: 100, clientY: 250, preventDefault: function() {} }; + var event = { clientX: ' . $startx . ', clientY: ' . $starty . ', preventDefault: function() {} }; instance.edit_start(event); }()); '; $this->execute_script($js); sleep(1); + + // Move slightly in one direction. + $movex = $startx + 50; + $movey = $starty + 30; $js = ' (function() { var instance = M.assignfeedback_editpdf.instance; - var event = { clientX: 150, clientY: 275, preventDefault: function() {} }; + var event = { clientX: ' . $movex . ', clientY: ' . $movey . ', preventDefault: function() {} }; instance.edit_move(event); }()); '; $this->execute_script($js); sleep(1); + + // Move a little further to stop. + $endx = $movex + 15; + $endy = $movey + 15; $js = ' (function() { var instance = M.assignfeedback_editpdf.instance; - var event = { clientX: 200, clientY: 300, preventDefault: function() {} }; + var event = { clientX: ' . $endx . ', clientY: ' . $endy . ', preventDefault: function() {} }; instance.edit_end(event); }()); '; $this->execute_script($js); diff --git a/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-debug.js b/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-debug.js index a5728757953..bbca05bc937 100644 --- a/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-debug.js +++ b/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-debug.js @@ -3780,6 +3780,24 @@ EDITOR.prototype = { var data = this.handle_response_data(response), poll = false; if (data) { + // When we are requesting the readonly version of the pages, they should + // always be available (see document_services::get_page_images_for_attempt) + // so we can just serve them immediately without triggering any document + // conversion or polling. + // + // This is necessary to prevent situations where the student has updated + // their submission and the teacher has annotated a previous version of + // the submission in the assignment grader. In this situation if a student + // views the online version of the annotated PDF ("View annotated PDF" link) + // the readonly pages here and the updated pages (awaiting conversion) will + // never match, and the code endlessly polls. + // + // See also: MDL-45580, MDL-66626, MDL-75898. + if (this.get('readonly') === true) { + this.prepare_pages_for_display(data); + return; + } + this.documentstatus = data.status; if (data.status === 0) { // The combined document is still waiting for input to be ready. diff --git a/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-min.js b/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-min.js index 78f7e469e95..525b9391fc3 100644 --- a/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-min.js +++ b/mod/assign/feedback/editpdf/yui/build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-min.js @@ -1,9 +1,9 @@ -YUI.add("moodle-assignfeedback_editpdf-editor",function(u,e){var c,i,s,n,a,h,l,p,f,_,m,b,w,o=M.cfg.wwwroot+"/mod/assign/feedback/editpdf/ajax.php",k=M.cfg.wwwroot+"/mod/assign/feedback/editpdf/ajax_progress.php",y="assignfeedback_editpdf_widget",v=".navigate-previous-button",x=" .navigate-next-button",A=".searchcommentsbutton",S=".expcolcommentsbutton",N=".assignfeedback_editpdf_commentsearch input",D=".assignfeedback_editpdf_commentsearch ul",T=".navigate-page-select",I=".loading",C=".progress-info.progress-striped",r=".drawingregion",g=".drawingcanvas",q=".commentcolourbutton",E=".annotationcolourbutton",Y=".warningmessages",z=".infoicon",R='input[name="assignfeedback_editpdf_haschanges"]',X=".currentstampbutton",L='[data-region="user-info"]',P=".rotateleftbutton",j=".rotaterightbutton",O={white:"rgb(255,255,255)",yellow:"rgb(255,236,174)",red:"rgb(249,181,179)",green:"rgb(214,234,178)",blue:"rgb(203,217,237)",clear:"rgba(255,255,255, 0)"},d={white:"rgb(255,255,255)",yellow:"rgb(255,207,53)",red:"rgb(239,69,64)",green:"rgb(152,202,62)",blue:"rgb(125,159,211)",black:"rgb(51,51,51)"},B={comment:".commentbutton",pen:".penbutton",line:".linebutton",rectangle:".rectanglebutton",oval:".ovalbutton",stamp:".stampbutton",select:".selectbutton",drag:".dragbutton",highlight:".highlightbutton"},t=function(t,e){this.x=parseInt(t,10),this.y=parseInt(e,10),this.clip=function(t){return this.xt.x+t.width&&(this.x=t.x+t.width),this.yt.y+t.height&&(this.y=t.y+t.height),this}};M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.point=t,t=function(t,e,i,s){this.x=t,this.y=e,this.width=i,this.height=s,this.bound=function(t){for(var e,i=0,s=0,n=0,a=0,o=0,o=0;os||0===o)&&(s=e.x),(e.ya||0===o)&&(a=e.y);return this.x=i,this.y=n,this.width=s-i,this.height=a-n,this},this.has_min_width=function(){return 5<=this.width},this.has_min_height=function(){return 5<=this.height},this.set_min_width=function(){this.width=5},this.set_min_height=function(){this.height=5}},M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.rect=t,t=function(){this.start=!1,this.end=!1,this.starttime=0,this.annotationstart=!1,this.tool="drag",this.commentcolour="yellow",this.annotationcolour="red",this.stamp="",this.path=[]},M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.edit=t,t=function(t){this.editor=t,this.shapes=[],this.nodes=[],this.erase=function(){if(this.shapes)for(;0'),i=u.Node.create(''),e.setAttrs({alt:M.util.get_string("deleteannotation","assignfeedback_editpdf")}),e.setStyles({backgroundColor:"white"}),i.addClass("deleteannotationbutton"),i.append(e),s.append(i),i.setData("annotation",this),i.on("click",this.remove,this),i.on("key",this.remove,"space,enter",this),i.setX(n[0]+t.x+t.width-18),i.setY(n[1]+t.y+6),this.drawable.nodes.push(i)),this.drawable},draw:function(){return this.draw_highlight(),this.drawable},remove:function(t){var e,i;for(t.preventDefault(),e=this.editor.pages[this.editor.currentpage].annotations,i=0;it.x+t.width&&(this.x=t.x+t.width),this.yt.y+t.height&&(this.y=t.y+t.height),this}};M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.point=t,t=function(t,e,i,s){this.x=t,this.y=e,this.width=i,this.height=s,this.bound=function(t){for(var e,i=0,s=0,n=0,a=0,o=0,o=0;os||0===o)&&(s=e.x),(e.ya||0===o)&&(a=e.y);return this.x=i,this.y=n,this.width=s-i,this.height=a-n,this},this.has_min_width=function(){return 5<=this.width},this.has_min_height=function(){return 5<=this.height},this.set_min_width=function(){this.width=5},this.set_min_height=function(){this.height=5}},M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.rect=t,t=function(){this.start=!1,this.end=!1,this.starttime=0,this.annotationstart=!1,this.tool="drag",this.commentcolour="yellow",this.annotationcolour="red",this.stamp="",this.path=[]},M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.edit=t,t=function(t){this.editor=t,this.shapes=[],this.nodes=[],this.erase=function(){if(this.shapes)for(;0'),i=u.Node.create(''),e.setAttrs({alt:M.util.get_string("deleteannotation","assignfeedback_editpdf")}),e.setStyles({backgroundColor:"white"}),i.addClass("deleteannotationbutton"),i.append(e),s.append(i),i.setData("annotation",this),i.on("click",this.remove,this),i.on("key",this.remove,"space,enter",this),i.setX(n[0]+t.x+t.width-18),i.setY(n[1]+t.y+6),this.drawable.nodes.push(i)),this.drawable},draw:function(){return this.draw_highlight(),this.drawable},remove:function(t){var e,i;for(t.preventDefault(),e=this.editor.pages[this.editor.currentpage].annotations,i=0;i");return s.addClass("annotation"),s.addClass("stamp"),s.setStyles({position:"absolute",display:"inline-block",backgroundImage:"url("+this.editor.get_stamp_image_url(this.path)+")",width:this.endx-this.x,height:this.endy-this.y,backgroundSize:"100% 100%"}),e.append(s),s.setX(i.x),s.setY(i.y),t.store_position(s,i.x,i.y),this.editor.get("readonly")||(s.on("gesturemovestart",this.editor.edit_start,null,this.editor),s.on("gesturemove",this.editor.edit_move,null,this.editor),s.on("gesturemoveend",this.editor.edit_end,null,this.editor)),t.nodes.push(s),this.drawable=t,l.superclass.draw.apply(this)},draw_current_edit:function(t){var e,i,s=new M.assignfeedback_editpdf.rect,n=new M.assignfeedback_editpdf.drawable(this.editor),a=this.editor.get_dialogue_element(r);return s.bound([t.start,t.end]),i=this.editor.get_window_coordinates(new M.assignfeedback_editpdf.point(s.x,s.y)),(e=u.Node.create("
")).addClass("annotation"),e.addClass("stamp"),e.setStyles({position:"absolute",display:"inline-block",backgroundImage:"url("+this.editor.get_stamp_image_url(t.stamp)+")",width:s.width,height:s.height,backgroundSize:"100% 100%"}),a.append(e),e.setX(i.x),e.setY(i.y),n.store_position(e,i.x,i.y),n.nodes.push(e),n},init_from_edit:function(t){var e=new M.assignfeedback_editpdf.rect;return e.bound([t.start,t.end]),e.width<40&&(e.width=40),e.height<40&&(e.height=40),this.gradeid=this.editor.get("gradeid"),this.pageno=this.editor.currentpage,this.x=e.x,this.y=e.y,this.endx=e.x+e.width,this.endy=e.y+e.height,this.colour=t.annotationcolour,this.path=t.stamp,!0},move:function(t,e){t-=this.x,e-=this.y;this.x+=t,this.y+=e,this.endx+=t,this.endy+=e,this.drawable&&this.drawable.erase(),this.editor.drawables.push(this.draw())}}),M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.annotationstamp=l,u.extend(p=function(t){t.draggable=!1,t.centered=!1,t.width="auto",t.visible=!1,t.footerContent="",p.superclass.constructor.apply(this,[t])},M.core.dialogue,{initializer:function(t){var e,i;p.superclass.initializer.call(this,t),this.get("boundingBox").addClass("assignfeedback_editpdf_dropdown"),e=this.get("buttonNode"),t=this.bodyNode,(i=u.Node.create("

")).addClass("accesshide"),i.setHTML(this.get("headerText")),t.prepend(i),t.on("clickoutside",function(t){this.get("visible")&&t.target.get("id")!==e.get("id")&&t.target.ancestor().get("id")!==e.get("id")&&(t.preventDefault(),this.hide())},this),e.on("click",function(t){t.preventDefault(),this.show()},this),e.on("key",this.show,"enter,space",this)},show:function(){var t=this.get("buttonNode"),e=p.superclass.show.call(this);return this.align(t,[u.WidgetPositionAlign.TL,u.WidgetPositionAlign.BL]),e}},{NAME:"Dropdown menu",ATTRS:{headerText:{value:""},buttonNode:{value:null}}}),u.Base.modifyAttrs(p,{modal:{getter:function(){return!1}}}),M.assignfeedback_editpdf=M.assignfeedback_editpdf||{},M.assignfeedback_editpdf.dropdown=p,u.extend(f=function(t){f.superclass.constructor.apply(this,[t])},M.assignfeedback_editpdf.dropdown,{initializer:function(t){var e,n=u.Node.create('