From dd49c01989c0d8032b5a934c63bfe394d0e70074 Mon Sep 17 00:00:00 2001
From: Dmitrii Metelkin <dmitriim@catalyst-au.net>
Date: Tue, 9 Jul 2024 16:46:55 +1000
Subject: [PATCH 1/2] issue #102: exclude deleted users when using OR operator

---
 classes/condition_manager.php    |  5 +++
 tests/condition_manager_test.php | 58 ++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/classes/condition_manager.php b/classes/condition_manager.php
index eb1e478..fd514d9 100644
--- a/classes/condition_manager.php
+++ b/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}) ";
diff --git a/tests/condition_manager_test.php b/tests/condition_manager_test.php
index ba2d581..71c53cd 100644
--- a/tests/condition_manager_test.php
+++ b/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()));
+    }
 }

From f89a89a0291657d905f72971174cafeaa3cc606e Mon Sep 17 00:00:00 2001
From: Dmitrii Metelkin <dmitriim@catalyst-au.net>
Date: Tue, 9 Jul 2024 16:47:14 +1000
Subject: [PATCH 2/2] Version bump

---
 version.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/version.php b/version.php
index b765a46..36ca5dc 100644
--- a/version.php
+++ b/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;