From 9dfe458967b6c6f4968d783e1ba0df27d8d23ff5 Mon Sep 17 00:00:00 2001 From: Dmitrii Metelkin Date: Mon, 25 Mar 2024 14:23:07 +1100 Subject: [PATCH] issue #52: implement a logical operator for rules --- README.md | 5 +- classes/condition_manager.php | 17 +++- .../local/systemreports/matching_users.php | 2 +- .../local/systemreports/rules.php | 9 +- classes/rule.php | 4 + classes/rule_form.php | 7 ++ classes/rule_manager.php | 25 ++++- db/install.xml | 1 + db/upgrade.php | 48 ++++++++++ lang/en/tool_dynamic_cohorts.php | 3 + tests/condition_manager_test.php | 65 +++++++++++++ tests/rule_manager_test.php | 92 +++++++++++++++---- version.php | 4 +- 13 files changed, 247 insertions(+), 35 deletions(-) create mode 100644 db/upgrade.php diff --git a/README.md b/README.md index cc8af91..07e7b1a 100644 --- a/README.md +++ b/README.md @@ -54,12 +54,13 @@ Conditions are simple predicates which assert something about a user in the syst ## Rules -Rules are what determine if a user will be added or removed from a cohort. A rule is defined by two things: +Rules are what determine if a user will be added or removed from a cohort. A rule is defined by few things: 1. A cohort 2. A set of conditions +3. A logical operator to be applied for conditions (OR/AND) -For a user to be added to the cohort specified by a rule, they must match all of the rule's conditions. +For users to be added to the cohort specified by a rule, they must match all of rule's conditions (logical operator AND) or any of rule's conditions (logical operator OR). **NB:** A cohort can be managed by _one and only one_ rule. This is to prevent rules competing over users in a cohort (e.g., to avoid situations where Rule A wants users a, b, c to be in a cohort, but Rule B wants to remove user c from the same cohort). diff --git a/classes/condition_manager.php b/classes/condition_manager.php index e2797b8..eb1e478 100644 --- a/classes/condition_manager.php +++ b/classes/condition_manager.php @@ -211,12 +211,14 @@ public static function get_conditions_with_event(base $event): array { * A helper function to build sql data for given list of conditions. * * @param \tool_dynamic_cohorts\condition[] $conditions A list of conditions. - * @param int|null $userid + * @param string $operator Logical operator. + * @param int|null $userid Optional user id in case we need to build SQL for one user. For example when event is triggered. * * @return \tool_dynamic_cohorts\condition_sql */ - public static function build_sql_data(array $conditions, ?int $userid = null): condition_sql { - $where = ' u.deleted = 0 '; + public static function build_sql_data(array $conditions, string $operator = rule_manager::CONDITIONS_OPERATOR_AND, + ?int $userid = null): condition_sql { + $where = ''; $join = ''; $params = []; @@ -239,7 +241,10 @@ public static function build_sql_data(array $conditions, ?int $userid = null): c } if (!empty($sqldata->get_where())) { - $where .= ' AND (' . $sqldata->get_where() . ')'; + if (!empty($where)) { + $where .= ' ' . rule_manager::get_logical_operator_text($operator); + } + $where .= ' (' . $sqldata->get_where() . ')'; } if (!empty($sqldata->get_params())) { @@ -249,10 +254,12 @@ public static function build_sql_data(array $conditions, ?int $userid = null): c if ($userid) { $userparam = condition_sql::generate_param_alias(); - $where .= " AND u.id = :{$userparam} "; + $where .= " AND ( u.id = :{$userparam}) "; $params += [$userparam => $userid]; } + $where .= ' AND ( u.deleted = 0) '; + return new condition_sql($join, $where, $params); } } diff --git a/classes/reportbuilder/local/systemreports/matching_users.php b/classes/reportbuilder/local/systemreports/matching_users.php index 5b720f9..449d6f0 100644 --- a/classes/reportbuilder/local/systemreports/matching_users.php +++ b/classes/reportbuilder/local/systemreports/matching_users.php @@ -52,7 +52,7 @@ protected function initialise(): void { $this->add_base_condition_sql(' true = false'); } - $sql = condition_manager::build_sql_data($conditions); + $sql = condition_manager::build_sql_data($conditions, $rule->get('operator')); $this->add_base_fields('DISTINCT u.id'); $this->add_join($sql->get_join()); diff --git a/classes/reportbuilder/local/systemreports/rules.php b/classes/reportbuilder/local/systemreports/rules.php index fb187bb..c415ca1 100644 --- a/classes/reportbuilder/local/systemreports/rules.php +++ b/classes/reportbuilder/local/systemreports/rules.php @@ -28,6 +28,7 @@ use tool_dynamic_cohorts\rule; use pix_icon; use html_writer; +use tool_dynamic_cohorts\rule_manager; /** * Rules admin table. @@ -88,7 +89,7 @@ protected function initialise(): void { )) ->set_type(column::TYPE_TEXT) ->set_is_sortable(false) - ->add_fields("{$rulealias}.id") + ->add_fields("{$rulealias}.id, {$rulealias}.operator") ->add_callback(static function($id, $row): string { $rule = new rule(0, $row); $conditions = count($rule->get_condition_records()); @@ -99,8 +100,10 @@ protected function initialise(): void { 'data-ruleid' => $rule->get('id'), ]); } - - return $conditions; + return get_string('conditionstext', 'tool_dynamic_cohorts', (object)[ + 'conditions' => $conditions, + 'operator' => rule_manager::get_logical_operator_text($rule->get('operator')), + ]); }); $this->add_column_from_entity('rule_entity:status'); diff --git a/classes/rule.php b/classes/rule.php index 0c375bf..b0862a2 100644 --- a/classes/rule.php +++ b/classes/rule.php @@ -65,6 +65,10 @@ protected static function define_properties() { 'type' => PARAM_INT, 'default' => 0, ], + 'operator' => [ + 'type' => PARAM_INT, + 'default' => 0, + ], ]; } diff --git a/classes/rule_form.php b/classes/rule_form.php index 9017f78..d7651a2 100644 --- a/classes/rule_form.php +++ b/classes/rule_form.php @@ -99,6 +99,13 @@ protected function definition() { ); $mform->addHelpButton('bulkprocessing', 'bulkprocessing', 'tool_dynamic_cohorts'); + $mform->addElement('select', + 'operator', + get_string('logical_operator', 'tool_dynamic_cohorts'), + [rule_manager::CONDITIONS_OPERATOR_AND => 'AND', rule_manager::CONDITIONS_OPERATOR_OR => 'OR']); + $mform->addHelpButton('operator', 'logical_operator', 'tool_dynamic_cohorts'); + $mform->setType('operator', PARAM_INT); + $this->add_action_buttons(); } diff --git a/classes/rule_manager.php b/classes/rule_manager.php index ec1919e..201321c 100644 --- a/classes/rule_manager.php +++ b/classes/rule_manager.php @@ -42,6 +42,16 @@ class rule_manager { */ const BULK_PROCESSING_SIZE = 10000; + /** + * Conditions logical operator AND. + */ + const CONDITIONS_OPERATOR_AND = 0; + + /** + * Conditions logical operator OR. + */ + const CONDITIONS_OPERATOR_OR = 1; + /** * Builds rule edit URL. * @@ -121,6 +131,7 @@ public static function process_form(\stdClass $formdata): rule { 'cohortid' => $formdata->cohortid, 'description' => $formdata->description, 'bulkprocessing' => $formdata->bulkprocessing, + 'operator' => $formdata->operator, ]; $oldcohortid = 0; @@ -223,7 +234,7 @@ public static function get_matching_users(rule $rule, ?int $userid = null): arra $sql = "SELECT DISTINCT u.id FROM {user} u"; try { - $sqldata = condition_manager::build_sql_data($conditions, $userid); + $sqldata = condition_manager::build_sql_data($conditions, $rule->get('operator'), $userid); } catch (\Exception $exception ) { self::trigger_matching_failed_event($rule, $exception->getMessage()); $rule->mark_broken(); @@ -261,7 +272,7 @@ public static function get_matching_users_count(rule $rule, ?int $userid = null) $sql = "SELECT COUNT(DISTINCT u.id) cnt FROM {user} u"; try { - $sqldata = condition_manager::build_sql_data($conditions, $userid); + $sqldata = condition_manager::build_sql_data($conditions, $rule->get('operator'), $userid); } catch (\Exception $exception ) { self::trigger_matching_failed_event($rule, $exception->getMessage()); $rule->mark_broken(); @@ -400,4 +411,14 @@ public static function get_rules_with_condition(condition_base $condition): arra return $rules; } + + /** + * Returns logical operator text based on value. + * + * @param int $operator Operator value. + * @return string + */ + public static function get_logical_operator_text(int $operator): string { + return $operator == self::CONDITIONS_OPERATOR_AND ? 'AND' : 'OR'; + } } diff --git a/db/install.xml b/db/install.xml index 7132aae..aa7e2b9 100644 --- a/db/install.xml +++ b/db/install.xml @@ -13,6 +13,7 @@ + diff --git a/db/upgrade.php b/db/upgrade.php new file mode 100644 index 0000000..e78e999 --- /dev/null +++ b/db/upgrade.php @@ -0,0 +1,48 @@ +. + +/** + * Upgrade hook. + * + * @package tool_dynamic_cohorts + * @copyright 2024 Catalyst IT + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +/** + * Upgrade the plugin. + * + * @param int $oldversion The old version of the plugin + * @return bool + */ +function xmldb_tool_dynamic_cohorts_upgrade($oldversion): bool { + global $DB; + + $dbman = $DB->get_manager(); + + if ($oldversion < 2024032501) { + $table = new xmldb_table('tool_dynamic_cohorts'); + $field = new xmldb_field('operator', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, 0); + + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + upgrade_plugin_savepoint(true, 2024032501, 'tool', 'dynamic_cohorts'); + } + + return true; +} diff --git a/lang/en/tool_dynamic_cohorts.php b/lang/en/tool_dynamic_cohorts.php index 407187e..46e0283 100644 --- a/lang/en/tool_dynamic_cohorts.php +++ b/lang/en/tool_dynamic_cohorts.php @@ -40,6 +40,7 @@ $string['cohortid_help'] = 'A cohort to manage as part of this rule. Only cohorts that are not managed by other plugins are displayed in this list.'; $string['condition'] = 'Condition'; $string['conditions'] = 'Conditions'; +$string['conditionstext'] = '{$a->conditions} ( logical {$a->operator} )'; $string['conditionsformtitle'] = 'Rule conditions'; $string['conditionchnagesnotapplied'] = 'Condition changes are not applied until you save the rule form'; $string['conditionformtitle'] = 'Rule condition'; @@ -81,6 +82,8 @@ $string['ismemberof'] = 'is member of'; $string['isnotmemberof'] = 'is not member of'; $string['isnotempty'] = 'is not empty'; +$string['logical_operator'] = 'Logical operator'; +$string['logical_operator_help'] = 'A logical operator to be applied to conditions for this rule. Operator "AND" means a user has to match all conditions to be added to a cohort. "OR" means a user has to match any of conditions to be added to a cohort.'; $string['managerules'] = 'Manage rules'; $string['managecohorts'] = 'Manage cohorts'; $string['matchingusers'] = 'Matching users'; diff --git a/tests/condition_manager_test.php b/tests/condition_manager_test.php index 283bee7..ba2d581 100644 --- a/tests/condition_manager_test.php +++ b/tests/condition_manager_test.php @@ -20,6 +20,7 @@ use tool_dynamic_cohorts\event\condition_created; use tool_dynamic_cohorts\event\condition_deleted; use tool_dynamic_cohorts\event\condition_updated; +use tool_dynamic_cohorts\local\tool_dynamic_cohorts\condition\user_profile; /** * Tests for condition manager class. @@ -210,4 +211,68 @@ public function test_get_conditions_with_event() { $this->assertArrayHasKey('tool_dynamic_cohorts\local\tool_dynamic_cohorts\condition\user_custom_profile', $conditions); $this->assertArrayHasKey('tool_dynamic_cohorts\local\tool_dynamic_cohorts\condition\user_profile', $conditions); } + + /** + * Basic test of building SQL data. + */ + public function test_build_sql_data() { + $this->resetAfterTest(); + + $this->getDataGenerator()->create_user(['username' => 'user1username']); + $this->getDataGenerator()->create_user(['username' => 'user2username']); + $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_IS_EQUAL_TO, + 'username_value' => 'user1username', + ]); + $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' => 'username', + 'username_operator' => condition_base::TEXT_IS_EQUAL_TO, + 'username_value' => 'user2username', + ]); + $condition->get_record()->save(); + $conditions[] = $condition->get_record(); + + $sql = condition_manager::build_sql_data($conditions); + $this->assertEquals('', $sql->get_join()); + $this->assertStringNotContainsString('OR', $sql->get_where()); + $this->assertStringContainsString('AND ( u.deleted = 0)', $sql->get_where()); + $this->assertTrue(in_array('user1username', $sql->get_params())); + $this->assertTrue(in_array('user2username', $sql->get_params())); + $this->assertStringNotContainsString('AND ( u.id = ', $sql->get_where()); + $this->assertFalse(in_array(777, $sql->get_params())); + + $sql = condition_manager::build_sql_data($conditions, rule_manager::CONDITIONS_OPERATOR_OR); + $this->assertEquals('', $sql->get_join()); + $this->assertStringContainsString('OR', $sql->get_where()); + $this->assertStringContainsString('AND ( u.deleted = 0)', $sql->get_where()); + $this->assertTrue(in_array('user1username', $sql->get_params())); + $this->assertTrue(in_array('user2username', $sql->get_params())); + $this->assertStringNotContainsString('AND ( u.id = ', $sql->get_where()); + $this->assertFalse(in_array(777, $sql->get_params())); + + $sql = condition_manager::build_sql_data($conditions, rule_manager::CONDITIONS_OPERATOR_OR, 777); + $this->assertEquals('', $sql->get_join()); + $this->assertStringContainsString('OR', $sql->get_where()); + $this->assertStringContainsString('AND ( u.deleted = 0)', $sql->get_where()); + $this->assertStringContainsString('AND ( u.id = ', $sql->get_where()); + $this->assertTrue(in_array('user1username', $sql->get_params())); + $this->assertTrue(in_array('user2username', $sql->get_params())); + $this->assertTrue(in_array(777, $sql->get_params())); + } } diff --git a/tests/rule_manager_test.php b/tests/rule_manager_test.php index 93c678a..c32dc79 100644 --- a/tests/rule_manager_test.php +++ b/tests/rule_manager_test.php @@ -98,7 +98,7 @@ public function test_build_rule_data_for_form() { 'tool_dynamic_cohorts\local\tool_dynamic_cohorts\condition\user_profile', [ 'profilefield' => 'username', - 'username_operator' => user_profile::TEXT_IS_EQUAL_TO, + 'username_operator' => condition_base::TEXT_IS_EQUAL_TO, 'username_value' => 'user1', ] ); @@ -120,6 +120,7 @@ public function test_build_rule_data_for_form() { 'enabled' => 0, 'bulkprocessing' => 0, 'broken' => 0, + 'operator' => rule_manager::CONDITIONS_OPERATOR_AND, 'id' => 0, 'timecreated' => 0, 'timemodified' => 0, @@ -178,7 +179,7 @@ public function test_process_rule_form_new_rule() { $cohort3 = $this->getDataGenerator()->create_cohort(); $formdata = ['name' => 'Test', 'cohortid' => $cohort1->id, 'description' => '', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); @@ -190,7 +191,7 @@ public function test_process_rule_form_new_rule() { } $formdata = ['name' => 'Test', 'cohortid' => $cohort2->id, 'description' => '', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(2, $DB->count_records(rule::TABLE)); @@ -202,7 +203,7 @@ public function test_process_rule_form_new_rule() { $cohort = $this->getDataGenerator()->create_cohort(); $formdata = ['name' => 'Test1', 'cohortid' => $cohort3->id, 'description' => '', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(3, $DB->count_records(rule::TABLE)); @@ -235,7 +236,8 @@ public function test_process_rule_form_existing_rule() { $cohort = $this->getDataGenerator()->create_cohort(); $formdata = ['id' => $rule->get('id'), 'name' => 'Test1', 'cohortid' => $cohort->id, - 'description' => 'D', 'conditionjson' => '', 'bulkprocessing' => 1]; + 'description' => 'D', 'conditionjson' => '', 'bulkprocessing' => 1, + 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); @@ -253,7 +255,8 @@ public function test_process_rule_form_with_not_existing_cohort() { $this->expectException(moodle_exception::class); $this->expectExceptionMessage('Invalid rule data. Cohort is invalid: 999'); - $formdata = ['name' => 'Test', 'cohortid' => 999, 'description' => '', 'conditionjson' => '', 'bulkprocessing' => 1]; + $formdata = ['name' => 'Test', 'cohortid' => 999, 'description' => '', 'conditionjson' => '', + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); } @@ -268,7 +271,7 @@ public function test_process_rule_form_with_cohort_managed_by_other_component() $this->expectExceptionMessage('Invalid rule data. Cohort is invalid: ' . $cohort->id); $formdata = ['name' => 'Test', 'cohortid' => $cohort->id, 'description' => '', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); } @@ -285,13 +288,13 @@ public function test_process_rule_form_with_cohort_managed_by_another_rule() { $this->expectExceptionMessage('Cohort ' . $cohort->id . ' is already managed by tool_dynamic_cohorts'); $formdata = ['name' => 'Test1', 'cohortid' => $cohort->id, 'description' => 'D', 'conditionjson' => '', - 'bulkprocessing' => 1]; + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); $this->assertEquals('tool_dynamic_cohorts', $DB->get_field('cohort', 'component', ['id' => $cohort->id])); // Trying to make a new rule with a cohort that is already taken. Should throw exception. $formdata = ['name' => 'Test2', 'cohortid' => $cohort->id, 'description' => 'D', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); } @@ -306,13 +309,14 @@ public function test_process_rule_form_update_rule_form_keeping_cohort() { $cohort = $this->getDataGenerator()->create_cohort(); $formdata = ['name' => 'Test1', 'cohortid' => $cohort->id, 'description' => 'D', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals('tool_dynamic_cohorts', $DB->get_field('cohort', 'component', ['id' => $cohort->id])); // Update the rule, changing the name. Should work as cohort is the same. $formdata = ['id' => $rule->get('id'), 'name' => 'Test1', - 'cohortid' => $cohort->id, 'description' => 'D', 'conditionjson' => '', 'bulkprocessing' => 1]; + 'cohortid' => $cohort->id, 'description' => 'D', 'conditionjson' => '', + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); $this->assertEquals('tool_dynamic_cohorts', $DB->get_field('cohort', 'component', ['id' => $cohort->id])); @@ -328,7 +332,7 @@ public function test_process_rule_form_triggers_events() { $eventsink = $this->redirectEvents(); $formdata = ['name' => 'Test1', 'cohortid' => $cohort->id, 'description' => 'D', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object) $formdata); $events = array_filter($eventsink->get_events(), function ($event) { @@ -341,7 +345,8 @@ public function test_process_rule_form_triggers_events() { // Update the rule, changing the name. Should work as cohort is the same. $formdata = ['id' => $rule->get('id'), 'name' => 'Test1', - 'cohortid' => $cohort->id, 'description' => 'D', 'conditionjson' => '', 'bulkprocessing' => 1]; + 'cohortid' => $cohort->id, 'description' => 'D', 'conditionjson' => '', 'bulkprocessing' => 1, + 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object) $formdata); $events = array_filter($eventsink->get_events(), function ($event) { @@ -363,7 +368,8 @@ public function test_process_rule_form_without_condition_data() { $this->expectException(moodle_exception::class); $this->expectExceptionMessage('Invalid rule data. Missing condition data.'); - $formdata = ['name' => 'Test', 'cohortid' => $cohort->id, 'description' => '', 'bulkprocessing' => 1]; + $formdata = ['name' => 'Test', 'cohortid' => $cohort->id, 'description' => '', 'bulkprocessing' => 1, + 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; rule_manager::process_form((object)$formdata); } @@ -380,7 +386,7 @@ public function test_process_rule_form_with_conditions() { // Creating rule without conditions. $formdata = ['name' => 'Test', 'cohortid' => $cohort->id, 'description' => '', - 'conditionjson' => '', 'bulkprocessing' => 1]; + 'conditionjson' => '', 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); // No conditions yet. Rule should be ok. @@ -399,7 +405,8 @@ public function test_process_rule_form_with_conditions() { ]); $formdata = ['id' => $rule->get('id'), 'name' => 'Test', 'enabled' => 1, 'cohortid' => $cohort->id, - 'description' => '', 'conditionjson' => $conditionjson, 'bulkprocessing' => 1]; + 'description' => '', 'conditionjson' => $conditionjson, 'bulkprocessing' => 1, + 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); $this->assertCount(0, $rule->get_condition_records()); @@ -411,7 +418,8 @@ public function test_process_rule_form_with_conditions() { // Updating the rule with 3 new conditions. Expecting 3 new conditions to be created. $formdata = ['id' => $rule->get('id'), 'name' => 'Test', 'enabled' => 1, 'cohortid' => $cohort->id, - 'description' => '', 'conditionjson' => $conditionjson, 'isconditionschanged' => true, 'bulkprocessing' => 1]; + 'description' => '', 'conditionjson' => $conditionjson, 'isconditionschanged' => true, + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); $this->assertCount(3, $rule->get_condition_records()); @@ -444,7 +452,8 @@ public function test_process_rule_form_with_conditions() { $conditionjson = json_encode($conditionjson); $formdata = ['id' => $rule->get('id'), 'name' => 'Test', 'enabled' => 1, 'cohortid' => $cohort->id, - 'description' => '', 'conditionjson' => $conditionjson, 'isconditionschanged' => true, 'bulkprocessing' => 1]; + 'description' => '', 'conditionjson' => $conditionjson, 'isconditionschanged' => true, + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); $this->assertCount(3, $rule->get_condition_records()); @@ -457,7 +466,8 @@ public function test_process_rule_form_with_conditions() { $this->assertTrue(condition::record_exists_select('classname = ? AND ruleid = ?', ['class4', $rule->get('id')])); $formdata = ['id' => $rule->get('id'), 'name' => 'Test', 'enabled' => 1, 'cohortid' => $cohort->id, - 'description' => '', 'conditionjson' => '', 'isconditionschanged' => true, 'bulkprocessing' => 1]; + 'description' => '', 'conditionjson' => '', 'isconditionschanged' => true, + 'bulkprocessing' => 1, 'operator' => rule_manager::CONDITIONS_OPERATOR_AND]; $rule = rule_manager::process_form((object)$formdata); $this->assertEquals(1, $DB->count_records(rule::TABLE)); $this->assertCount(0, $rule->get_condition_records()); @@ -568,7 +578,7 @@ public function test_get_matching_users() { $condition = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]); $condition->set_config_data([ 'profilefield' => 'username', - 'username_operator' => user_profile::TEXT_IS_EQUAL_TO, + 'username_operator' => condition_base::TEXT_IS_EQUAL_TO, 'username_value' => 'user1username', ]); $condition->get_record()->save(); @@ -582,6 +592,40 @@ public function test_get_matching_users() { $this->assertEquals(1, rule_manager::get_matching_users_count($rule)); } + /** + * Basic test for get matching users with or logic to make sure it all works. + */ + public function test_get_matching_users_or_logic() { + $this->resetAfterTest(); + + $this->getDataGenerator()->create_user(['username' => 'user1username']); + $this->getDataGenerator()->create_user(['username' => 'user2username']); + $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(); + + $condition = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]); + $condition->set_config_data([ + 'profilefield' => 'username', + 'username_operator' => condition_base::TEXT_IS_EQUAL_TO, + 'username_value' => 'user1username', + ]); + $condition->get_record()->save(); + + $condition = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]); + $condition->set_config_data([ + 'profilefield' => 'username', + 'username_operator' => condition_base::TEXT_IS_EQUAL_TO, + 'username_value' => 'user2username', + ]); + $condition->get_record()->save(); + $this->assertEquals(2, rule_manager::get_matching_users_count($rule)); + } + /** * Test rule processing. */ @@ -889,4 +933,12 @@ public function test_get_rules_with_condition() { $rule4->save(); $this->assertFalse($cache->get($key)); } + + /** + * Testing logical operator text. + */ + public function test_get_logical_operator_text() { + $this->assertSame('AND', rule_manager::get_logical_operator_text(0)); + $this->assertSame('OR', rule_manager::get_logical_operator_text(rand(1, 1000))); + } } diff --git a/version.php b/version.php index 7ecb9ae..636b18e 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'tool_dynamic_cohorts'; -$plugin->release = 2024032200; -$plugin->version = 2024032200; +$plugin->release = 2024032501; +$plugin->version = 2024032501; $plugin->requires = 2022112800; $plugin->supported = [401, 403]; $plugin->maturity = MATURITY_ALPHA;