MDL-51652 Availability: OR conditions fail when filtering user lists

When filtering a user list, if there is an OR tree where one of the
conditions does not restrict user lists such as a date condition,
then the overall result should include all users (not be filtered).
This was not working correctly.

diff --git a/availability/classes/tree.php b/availability/classes/tree.php
index ef15c58..d88cf3a 100644 (file)
--- a/availability/classes/tree.php
+++ b/availability/classes/tree.php
@@ -328,7 +328,14 @@ class tree extends tree_node {
         // Loop through all valid children.
         foreach ($this->children as $index => $child) {
             if (!$child->is_applied_to_user_lists()) {
-                continue;
+                if ($andoperator) {
+                    continue;
+                } else {
+                    // OR condition with one option that doesn't restrict user
+                    // lists = everyone is allowed.
+                    $anyconditions = false;
+                    break;
+                }
             }
             $childresult = $child->filter_user_list($users, $innernot, $info, $checker);
             if ($andoperator) {
@@ -359,7 +366,14 @@ class tree extends tree_node {
         $childresults = array();
         foreach ($this->children as $index => $child) {
             if (!$child->is_applied_to_user_lists()) {
-                continue;
+                if ($andoperator) {
+                    continue;
+                } else {
+                    // OR condition with one option that doesn't restrict user
+                    // lists = everyone is allowed.
+                    $childresults = array();
+                    break;
+                }
             }
             $childresult = $child->get_user_list_sql($innernot, $info, $onlyactive);
             if ($childresult[0]) {

diff --git a/availability/tests/tree_test.php b/availability/tests/tree_test.php
index ebabc08..ab6c30c 100644 (file)
--- a/availability/tests/tree_test.php
+++ b/availability/tests/tree_test.php
@@ -608,6 +608,15 @@ class tree_testcase extends \advanced_testcase {
         ksort($result);
         $this->assertEquals(array(1, 3), array_keys($result));
 
+        // Tree with OR condition one of which doesn't filter.
+        $structure = tree::get_root_json(array(
+                self::mock(array('filter' => array(3))),
+                self::mock(array())), tree::OP_OR);
+        $tree = new tree($structure);
+        $result = $tree->filter_user_list($users, false, $info, $checker);
+        ksort($result);
+        $this->assertEquals(array(1, 2, 3), array_keys($result));
+
         // Tree with two condition that both filter (&).
         $structure = tree::get_root_json(array(
                 self::mock(array('filter' => array(2, 3))),
@@ -726,6 +735,16 @@ class tree_testcase extends \advanced_testcase {
         $result = $DB->get_fieldset_sql($sql, $params);
         sort($result);
         $this->assertEquals(array($userinneither->id), $result);
+
+        // Tree with 'OR' of group conditions and a non-filtering condition.
+        // The non-filtering condition should mean that ALL users are included.
+        $tree = new tree(tree::get_root_json(array(
+            \availability_group\condition::get_json($group1->id),
+            \availability_date\condition::get_json(\availability_date\condition::DIRECTION_UNTIL, 3)
+        ), tree::OP_OR));
+        list($sql, $params) = $tree->get_user_list_sql(false, $info, false);
+        $this->assertEquals('', $sql);
+        $this->assertEquals(array(), $params);
     }
 
     /**