Merge pull request #103 from catalyst/issue102-MOODLE_401_STABLE

issue #102: exclude deleted users when using OR operator

Author: dmitriim

classes/condition_manager.php

@@ -252,6 +252,11 @@ public static function build_sql_data(array $conditions, string $operator = rule
             }
         }
 
+        // If we have conditions let's wrap them all in one big condition.
+        if (!empty($where)) {
+            $where = ' ( ' . $where . ' ) ';
+        }
+
         if ($userid) {
             $userparam = condition_sql::generate_param_alias();
             $where .= " AND ( u.id = :{$userparam}) ";

tests/condition_manager_test.php

@@ -275,4 +275,62 @@ public function test_build_sql_data() {
         $this->assertTrue(in_array('user2username', $sql->get_params()));
         $this->assertTrue(in_array(777, $sql->get_params()));
     }
+
+    /**
+     * Test that generated SQL should exclude deleted users.
+     */
+    public function test_should_exclude_deleted_users() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $this->getDataGenerator()->create_user(['username' => 'user1username', 'auth' => 'lti']);
+        $this->getDataGenerator()->create_user(['username' => 'user2username', 'auth' => 'lti']);
+        $usertodeleted = $this->getDataGenerator()->create_user(['username' => 'user3username', 'auth' => 'lti']);
+        $this->getDataGenerator()->create_user(['username' => 'test']);
+
+        $cohort = $this->getDataGenerator()->create_cohort();
+
+        $rule = new rule(0, (object)['name' => 'Test rule 1', 'cohortid' => $cohort->id,
+            'operator' => rule_manager::CONDITIONS_OPERATOR_OR]);
+        $rule->save();
+
+        $conditions = [];
+        $condition = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]);
+        $condition->set_config_data([
+            'profilefield' => 'username',
+            'username_operator' => condition_base::TEXT_CONTAINS,
+            'username_value' => 'username',
+        ]);
+        $condition->get_record()->save();
+        $conditions[] = $condition->get_record();
+
+        $condition = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]);
+        $condition->set_config_data([
+            'profilefield' => 'auth',
+            'auth_operator' => condition_base::TEXT_IS_EQUAL_TO,
+            'auth_value' => 'lti',
+        ]);
+        $condition->get_record()->save();
+        $conditions[] = $condition->get_record();
+
+        $sqldataand = condition_manager::build_sql_data($conditions);
+        $sqldataor = condition_manager::build_sql_data($conditions, rule_manager::CONDITIONS_OPERATOR_OR);
+
+        $basesql = "SELECT DISTINCT u.id FROM {user} u ";
+        $sqland = $basesql . $sqldataand->get_join() . ' WHERE ' . $sqldataand->get_where();
+        $sqlor = $basesql . $sqldataor->get_join() . ' WHERE ' . $sqldataor->get_where();
+
+        $this->assertCount(3, $DB->get_records_sql($sqland, $sqldataand->get_params()));
+        $this->assertCount(3, $DB->get_records_sql($sqlor, $sqldataor->get_params()));
+        $this->assertArrayHasKey($usertodeleted->id, $DB->get_records_sql($sqland, $sqldataand->get_params()));
+        $this->assertArrayHasKey($usertodeleted->id, $DB->get_records_sql($sqlor, $sqldataor->get_params()));
+
+        delete_user($usertodeleted);
+
+        $this->assertCount(2, $DB->get_records_sql($sqland, $sqldataand->get_params()));
+        $this->assertCount(2, $DB->get_records_sql($sqlor, $sqldataor->get_params()));
+        $this->assertArrayNotHasKey($usertodeleted->id, $DB->get_records_sql($sqland, $sqldataand->get_params()));
+        $this->assertArrayNotHasKey($usertodeleted->id, $DB->get_records_sql($sqlor, $sqldataor->get_params()));
+    }
 }

version.php

@@ -25,8 +25,8 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'tool_dynamic_cohorts';
-$plugin->release = 2024051000;
-$plugin->version = 2024051000;
+$plugin->release = 2024051001;
+$plugin->version = 2024051001;
 $plugin->requires = 2022112800;
 $plugin->supported = [401, 403];
 $plugin->maturity = MATURITY_STABLE;