issue #35: indicate broken conditions in the list of conditions

catalyst · Mar 8, 2024 · 1c32cc4 · 1c32cc4
1 parent 3df13c7
commit 1c32cc4
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 5 deletions.
diff --git a/classes/external/rule_conditions.php b/classes/external/rule_conditions.php
@@ -73,10 +73,12 @@ public static function get_conditions(int $ruleid): array {
                 $name = $condition->get('classname');
                 $description = $condition->get('configdata');
                 $configdata = $condition->get('configdata');
+                $broken = false;
             } else {
                 $name = $instance->get_name();
                 $description = $instance->is_broken() ? $instance->get_broken_description() : $instance->get_config_description();
                 $configdata = json_encode($instance->get_config_data());
+                $broken = $instance->is_broken();
             }
 
             $conditions[$condition->get('id')] = [
@@ -86,6 +88,7 @@ public static function get_conditions(int $ruleid): array {
                 'configdata' => $configdata,
                 'description' => $description,
                 'name' => $name,
+                'broken' => $broken,
             ];
         }
 
@@ -106,6 +109,7 @@ public static function get_conditions_returns(): external_multiple_structure {
                 'configdata' => new external_value(PARAM_RAW, ''),
                 'description' => new external_value(PARAM_RAW, ''),
                 'name' => new external_value(PARAM_RAW, ''),
+                'broken' => new external_value(PARAM_BOOL, ''),
             ])
         );
     }

diff --git a/classes/rule_manager.php b/classes/rule_manager.php
@@ -76,16 +76,20 @@ public static function build_data_for_form(rule $rule): array {
             $instance = condition_base::get_instance(0, $condition->to_record());
 
             if (!$instance) {
+                $broken = false;
                 $name = $condition->get('classname');
                 $description = $condition->get('configdata');
             } else {
+                $broken = $instance->is_broken();
                 $name = $instance->get_name();
-                $description = $instance->is_broken() ? $instance->get_broken_description() : $instance->get_config_description();
+                $description = $broken ? $instance->get_broken_description() : $instance->get_config_description();
             }
 
             $conditions[] = (array)$condition->to_record() +
                 ['description' => $description] +
-                ['name' => $name];
+                ['name' => $name] +
+                ['broken' => $broken];
+            ;
         }
 
         if (!empty($conditions)) {

diff --git a/templates/conditions.mustache b/templates/conditions.mustache
@@ -51,7 +51,7 @@
         <tbody>
         {{#conditions}}
             <tr>
-                <td>{{{name}}}</td>
+                <td>{{#broken}}{{#pix}} i/invalid, core {{/pix}}{{/broken}} {{{name}}}</td>
                 <td>{{{description}}}</td>
                 {{^hidecontrols}}
                     <td>

diff --git a/tests/external/rule_conditions_test.php b/tests/external/rule_conditions_test.php
@@ -119,9 +119,18 @@ public function test_get_conditions_for_rule() {
         $condition3 = new condition(0, (object)['ruleid' => $rule->get('id'), 'classname' => 'test', 'sortorder' => 2]);
         $condition3->save();
 
+        // Broken condition.
+        $condition4 = user_profile::get_instance(0, (object)['ruleid' => $rule->get('id'), 'sortorder' => 1]);
+        $condition4->set_config_data([
+            'profilefield' => 'username',
+            'username_operator' => user_profile::TEXT_IS_EQUAL_TO,
+            'username_value' => '',
+        ]);
+        $condition4->get_record()->save();
+
         $conditions = rule_conditions::get_conditions($rule->get('id'));
         $this->assertIsArray($conditions);
-        $this->assertCount(3, $conditions);
+        $this->assertCount(4, $conditions);
 
         $this->assertArrayHasKey($condition1->get_record()->get('id'), $conditions);
         $this->assertArrayHasKey($condition2->get_record()->get('id'), $conditions);
@@ -162,5 +171,23 @@ public function test_get_conditions_for_rule() {
         $this->assertSame('{}', $conditions[$condition3->get('id')]['configdata']);
         $this->assertSame('{}', $conditions[$condition3->get('id')]['description']);
         $this->assertSame('test', $conditions[$condition3->get('id')]['name']);
+
+        $this->assertSame($condition4->get_record()->get('id'), $conditions[$condition4->get_record()->get('id')]['id']);
+        $this->assertSame(1, $conditions[$condition4->get_record()->get('id')]['sortorder']);
+        $this->assertSame(
+            'tool_dynamic_cohorts\local\tool_dynamic_cohorts\condition\user_profile',
+            $conditions[$condition4->get_record()->get('id')]['classname']
+        );
+        $this->assertSame(
+            '{"profilefield":"username","username_operator":3,"username_value":""}',
+            $conditions[$condition4->get_record()->get('id')]['configdata']
+        );
+        $this->assertSame('Username is equal to user1username', $conditions[$condition2->get_record()->get('id')]['description']);
+        $this->assertSame('User standard profile field', $conditions[$condition2->get_record()->get('id')]['name']);
+
+        $this->assertSame(true, $conditions[$condition1->get_record()->get('id')]['broken']);
+        $this->assertSame(false, $conditions[$condition2->get_record()->get('id')]['broken']);
+        $this->assertSame(false, $conditions[$condition3->get('id')]['broken']);
+        $this->assertSame(true, $conditions[$condition4->get_record()->get('id')]['broken']);
     }
 }
diff --git a/tests/rule_manager_test.php b/tests/rule_manager_test.php
@@ -110,7 +110,8 @@ public function test_build_rule_data_for_form() {
         $condition = condition::get_record(['id' => $instance->get_record()->get('id')]);
         $conditions[] = (array) $condition->to_record() +
             ['description' => $instance->get_config_description()] +
-            ['name' => $instance->get_name()];
+            ['name' => $instance->get_name()] +
+            ['broken' => false];
 
         $expected = [
             'name' => 'Test rule',