issue #116: add WS to delete and toggle rules

Author: dmitriim

classes/external/rules.php

@@ -58,28 +58,26 @@ public static function delete_rules_parameters(): external_function_parameters {
      * @return array
      */
     public static function delete_rules(array $ruleids): array {
-        global $DB;
-
         self::validate_parameters(self::delete_rules_parameters(), ['ruleids' => $ruleids]);
 
         $context = context_system::instance();
         self::validate_context($context);
         require_capability('tool/dynamic_cohorts:manage', $context);
 
         // We would like to treat deletion for multiple rules as one operation.
-        // If one failed we would like to fail whole call and roll back.
-        $transaction = $DB->start_delegated_transaction();
-        try {
-            foreach ($ruleids as $ruleid) {
-                $rule = rule::get_record(['id' => (int) $ruleid]);
-                if (empty($rule)) {
-                    throw new invalid_parameter_exception('Rule does not exist. ID: ' . $ruleid);
-                }
-                rule_manager::delete_rule($rule);
-                //$transaction->allow_commit();
+        // So let's check that all rules exist and then delete them.
+        // Otherwise throw an exception and fail whole WS call.
+        $rulestodelete = [];
+        foreach ($ruleids as $ruleid) {
+            $rule = rule::get_record(['id' => (int) $ruleid]);
+            if (empty($rule)) {
+                throw new invalid_parameter_exception('Rule does not exist. ID: ' . $ruleid);
             }
-        } catch (throwable $ex) {
-            $transaction->rollback($ex);
+            $rulestodelete[] = $rule;
+        }
+
+        foreach ($rulestodelete as $rule) {
+            rule_manager::delete_rule($rule);
         }
 
         return [];
@@ -112,14 +110,14 @@ public static function toggle_status_parameters(): external_function_parameters
      * @return array
      */
     public static function toggle_status(int $ruleid): array {
-        self::validate_parameters(self::delete_rules_parameters(), ['ruleid' => $ruleid]);
+        self::validate_parameters(self::toggle_status_parameters(), ['ruleid' => $ruleid]);
 
         self::validate_context(context_system::instance());
         require_capability('tool/dynamic_cohorts:manage', context_system::instance());
 
         $rule = rule::get_record(['id' => $ruleid]);
         if (empty($rule)) {
-            throw new invalid_parameter_exception('Rule does not exist.');
+            throw new invalid_parameter_exception('Rule does not exist. ID: ' . $ruleid);
         }
 
         if ($rule->is_broken()) {
@@ -128,7 +126,7 @@ public static function toggle_status(int $ruleid): array {
             $rule->save();
             rule_updated::create(['other' => ['ruleid' => $rule->get('id')]])->trigger();
 
-            throw new invalid_parameter_exception('A broken rule can\'t be enabled');
+            throw new invalid_parameter_exception('A broken rule can\'t be enabled ID: ' . $ruleid);
         }
 
         $newvalue = (int) !$rule->is_enabled();

tests/external/rules_test.php

@@ -63,7 +63,7 @@ public function test_delete_rules_permissions() {
     }
 
     /**
-     * Test can get total.
+     * Test exception thrown on deleting invalid rule.
      */
     public function test_exception_delete_rules_when_one_is_invalid() {
         $this->resetAfterTest();
@@ -79,7 +79,7 @@ public function test_exception_delete_rules_when_one_is_invalid() {
     }
 
     /**
-     * Test can get total.
+     * Test rules are not deleted if one is invalid.
      */
     public function test_delete_rules_keep_rules_when_one_is_invalid() {
         $this->resetAfterTest();
@@ -93,9 +93,6 @@ public function test_delete_rules_keep_rules_when_one_is_invalid() {
 
         $this->assertCount(2, rule::get_records());
 
-        $this->expectException(\required_capability_exception::class);
-        $this->expectExceptionMessage('Rule does not exist. ID: 777');
-
         try {
             rules::delete_rules([$rule1->get('id'), $rule2->get('id'), 777]);
         } catch (\invalid_parameter_exception $exception) {
@@ -106,4 +103,69 @@ public function test_delete_rules_keep_rules_when_one_is_invalid() {
             $this->assertCount(2, rule::get_records());
         }
     }
+
+    /**
+     * Test exception if rule is not exist.
+     */
+    public function test_toggle_status_exception_on_invalid_rule() {
+        $this->resetAfterTest();
+
+        $this->setAdminUser();
+        $this->expectException(\invalid_parameter_exception::class);
+        $this->expectExceptionMessage('Rule does not exist. ID: 777');
+
+        rules::toggle_status(777);
+    }
+
+    /**
+     * Test required permissions.
+     */
+    public function test_toggle_status_permissions() {
+        $this->resetAfterTest();
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        $this->expectException(\required_capability_exception::class);
+        $this->expectExceptionMessage('Sorry, but you do not currently have permissions to do that (Manage rules).');
+
+        rules::toggle_status(777);
+    }
+
+    /**
+     * Test exception is thrown trying to toggle a broken rule
+     */
+    public function test_exception_toggle_status_on_broken_rule() {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $rule = new rule(0, (object)['name' => 'Test rule 1']);
+        $rule->set('broken', 1);
+        $rule->save();
+
+        $this->expectException(\invalid_parameter_exception::class);
+        $this->expectExceptionMessage('A broken rule can\'t be enabled ID: ' . $rule->get('id'));
+
+        rules::toggle_status($rule->get('id'));
+    }
+
+    /**
+     * Test toggling status on rule.
+     */
+    public function test_toggle_status_on_rule() {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $rule = new rule(0, (object)['name' => 'Test rule 1']);
+        $rule->save();
+
+        $this->assertFalse($rule->is_enabled());
+
+        rules::toggle_status($rule->get('id'));
+        $rule = rule::get_record(['id' => $rule->get('id')]);
+        $this->assertTrue($rule->is_enabled());
+
+        rules::toggle_status($rule->get('id'));
+        $rule = rule::get_record(['id' => $rule->get('id')]);
+        $this->assertFalse($rule->is_enabled());
+    }
 }