From 4edd8e773d379707fb07098e3f79f3e7d1dc9ac7 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Tue, 1 Mar 2016 12:29:13 +0800 Subject: [PATCH] MDL-52397 editpdf: Fix draft vs nondraft comparison We can not rely on the db engine returning results in the same order if no order is specified in get_comments and get_annotations. Also fixing a related unit test. --- mod/assign/feedback/editpdf/locallib.php | 32 +++++++++++++------ .../feedback/editpdf/tests/editpdf_test.php | 4 +-- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/mod/assign/feedback/editpdf/locallib.php b/mod/assign/feedback/editpdf/locallib.php index 57e0e71fee9..f2c5b8d780a 100644 --- a/mod/assign/feedback/editpdf/locallib.php +++ b/mod/assign/feedback/editpdf/locallib.php @@ -204,14 +204,21 @@ class assign_feedback_editpdf extends assign_feedback_plugin { // The count is different so we have a modification. return true; } else { - // Have a closer look and see if the draft files match the non draft files. - foreach ($nondraftannotations as $index => $ndannotation) { - foreach ($ndannotation as $key => $value) { - if ($key != 'id' && $value != $draftannotations[$index]->$key) { - return true; + $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++; } } + if ($matches !== count($nondraftannotations)) { + return true; + } } // Select all comments. $draftcomments = page_editor::get_comments($grade->id, $i, true); @@ -220,13 +227,20 @@ class assign_feedback_editpdf extends assign_feedback_plugin { return true; } else { // Go for a closer inspection. - foreach ($nondraftcomments as $index => $ndcomment) { - foreach ($ndcomment as $key => $value) { - if ($key != 'id' && $value != $draftcomments[$index]->$key) { - return true; + $matches = 0; + foreach ($nondraftcomments as $ndcomment) { + foreach ($draftcomments as $dcomment) { + foreach ($ndcomment as $key => $value) { + if ($key != 'id' && $value != $dcomment->{$key}) { + continue 2; + } } + $matches++; } } + if ($matches !== count($nondraftcomments)) { + return true; + } } } return false; diff --git a/mod/assign/feedback/editpdf/tests/editpdf_test.php b/mod/assign/feedback/editpdf/tests/editpdf_test.php index 3a0ad840d2b..f8eb3e969d5 100644 --- a/mod/assign/feedback/editpdf/tests/editpdf_test.php +++ b/mod/assign/feedback/editpdf/tests/editpdf_test.php @@ -356,7 +356,7 @@ class assignfeedback_editpdf_testcase extends mod_assign_base_testcase { $annotation->type = 'rectangle'; $annotation->colour = 'yellow'; - page_editor::add_annotation($annotation); + $yellowannotationid = page_editor::add_annotation($annotation); // Add a comment as well. $comment = new comment(); @@ -391,7 +391,7 @@ class assignfeedback_editpdf_testcase extends mod_assign_base_testcase { page_editor::add_annotation($annotation); $annotations = page_editor::get_annotations($grade->id, 0, true); - page_editor::remove_annotation($annotations[1]->id); + page_editor::remove_annotation($yellowannotationid); $this->assertTrue($plugin->is_feedback_modified($grade, $data)); $plugin->save($grade, $data); -- 2.43.0