From 4edd8e773d379707fb07098e3f79f3e7d1dc9ac7 Mon Sep 17 00:00:00 2001
From: David Monllao <davidm@moodle.com>
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.36.1