Coding style

classes/manager/workflow_manager.php

@@ -72,9 +72,9 @@ class workflow_manager {
      */
     public static function disable($workflowid) {
         $workflow = self::get_workflow($workflowid);
-        if ($workflow && self::is_disableable($workflowid)) { // @TODO Notify user if not.
+        if ($workflow && self::is_disableable($workflowid)) {
             $workflow->active = false;
-            // $workflow->timeactive = null; @TODO Necessary? Why do we have active and timeactive?
+            $workflow->timeactive = null;
             $workflow->sortindex = null;
             $workflow->timedeactive = time();
             self::insert_or_update($workflow);
@@ -268,14 +268,15 @@ class workflow_manager {
                 self::remove($workflowid);
             } else {
                 echo $OUTPUT->notification(get_string('workflow_not_removeable', 'tool_lifecycle')
-                    , 'warning'); // @todo these notifications aren't shown properly currently
+                    , 'warning'); // TODO these notifications aren't shown properly currently.
             }
         }
     }
 
     private static function render_demand_confirm($action, $workflowid, $message) {
         global $OUTPUT, $PAGE;
-        $yesurl = new \moodle_url($PAGE->url, array('workflowid' => $workflowid, 'action' => $action, 'sesskey' => sesskey(), 'confirm' => 1));
+        $yesurl = new \moodle_url($PAGE->url, array('workflowid' => $workflowid, 'action' => $action,
+            'sesskey' => sesskey(), 'confirm' => 1));
         $nourl = new \moodle_url('/admin/tool/lifecycle/adminsettings.php');
         $output = $OUTPUT->header();
         $output .= $OUTPUT->confirm($message, $yesurl, $nourl);

db/upgrade.php

@@ -298,7 +298,6 @@ function xmldb_tool_lifecycle_upgrade($oldversion) {
         upgrade_plugin_savepoint(true, 2018101000, 'tool', 'lifecycle');
     }
 
-    // @todo set right version
     if ($oldversion < 2019010102) {
         // Define field timedeactive to be added to tool_lifecycle_workflow.
         $table = new xmldb_table('tool_lifecycle_workflow');
@@ -310,7 +309,6 @@ function xmldb_tool_lifecycle_upgrade($oldversion) {
         }
 
         // Lifecycle savepoint reached.
-        // @TODO Set right version.
         upgrade_plugin_savepoint(true, 2019010102, 'tool', 'lifecycle');
     }
 

tests/behat/behat_tool_lifecycle.php

@@ -117,9 +117,8 @@ class behat_tool_lifecycle extends behat_base {
      * @throws Exception
      */
     public function i_should_not_see_the_table($tablename) {
-        // @todo solve without relaying on exceptions
         try {
-            $this->get_xpath_of_table($tablename);
+            $this->find_table($tablename);
         } catch (ExpectationException $e) {
             return;
         }
@@ -136,10 +135,10 @@ class behat_tool_lifecycle extends behat_base {
      * @throws Exception
      */
     public function i_should_see_the_row($rowname, $tablename) {
-        // @todo solve without relaying on exceptions
+        $this->find_table($tablename);
         try {
-            $this->get_xpath_of_row($rowname, $tablename);
-        } catch (ExpectationException $e) { // gets also threw on not existing table!
+            $this->find_row($rowname, $tablename);
+        } catch (ExpectationException $e) {
             throw new ExpectationException('"The row "' . $tablename . '"  was found."', $this->getSession());
         }
 
@@ -155,10 +154,11 @@ class behat_tool_lifecycle extends behat_base {
      * @throws Exception
      */
     public function i_should_not_see_the_row($rowname, $tablename) {
-        // @todo solve without relaying on exceptions
+        $this->find_table($tablename);
+
         try {
-            $this->get_xpath_of_row($rowname, $tablename);
-        } catch (ExpectationException $e) { // gets also threw on not existing table!
+            $this->find_row($rowname, $tablename);
+        } catch (ExpectationException $e) {
             return;
         }
         throw new ExpectationException('"The row "' . $tablename . '"  was found."', $this->getSession());
@@ -193,7 +193,7 @@ class behat_tool_lifecycle extends behat_base {
      * @return string xpath of the table
      * @throws ExpectationException
      */
-    private function get_xpath_of_table($tablename) {
+    private function find_table($tablename) {
         $xpathelement = "//table/tbody/tr[contains(@id, '$tablename')]";
 
         try {
@@ -212,15 +212,9 @@ class behat_tool_lifecycle extends behat_base {
      * @return string xpath of the table
      * @throws ExpectationException
      */
-    private function get_xpath_of_row($rowname, $tablename) {
+    private function find_row($rowname, $tablename) {
         $xpathelement = "//table/tbody/tr[contains(@id, '$tablename')]";
 
-        try {
-            $this->find('xpath', $xpathelement);
-        } catch (ElementNotFoundException $e) {
-            throw new ExpectationException('"The table ' . $tablename . ' was not found.', $this->getSession());
-        }
-
         $xpathelement = $xpathelement . "//*[contains(text(),'$rowname')]/ancestor::tr";
 
         try {

tests/behat/disable_workflow.feature

@@ -75,5 +75,6 @@ Feature: Disable a workflow
     Then I should see the row "My Workflow" in the "tool_lifecycle_workflow_definitions" table
     And I should not see the row "My Workflow" in the "tool_lifecycle_active_automatic_workflows" table
     When I press "Activate"
-    Then I should not see the row "My Workflow" in the "tool_lifecycle_workflow_definitions" table
+    # Since no element is left, the table is not displayed anymore.
+    Then I should not see the table "tool_lifecycle_workflow_definitions"
     And I should see the row "My Workflow" in the "tool_lifecycle_active_automatic_workflows" table
\ No newline at end of file

tests/workflow_actions_testcase.php

@@ -42,7 +42,8 @@ abstract class workflow_actions_testcase extends \advanced_testcase {
         // Remove preset workflows.
         $workflows = workflow_manager::get_active_automatic_workflows();
         foreach ($workflows as $workflow) {
-            workflow_manager::remove($workflow->id, true); // remove() hasn't removed unremovable workflows (like presets) anymore…
+            workflow_manager::remove($workflow->id, true);
+            // Function remove() hasn't removed unremovable workflows (like presets) anymore.
         }
 
         $this->workflow1 = $generator->create_workflow();

tests/workflow_activate_disable_duplicate_test.php

@@ -73,7 +73,7 @@ class tool_lifecycle_workflow_activate_disable_duplicate_testcase extends workfl
      * Test to deactivate the first workflow.
      */
     public function test_deactivate_first() {
-        // @TODO
+        // TODO to be implemented.
     }
 
 }

trigger/categories/lib.php

@@ -100,7 +100,9 @@ class categories extends base_automatic {
             'multiple' => true,
             'noselectionstring' => get_string('categories_noselection', 'lifecycletrigger_categories'),
         );
-        $mform->addElement('autocomplete', 'categories', get_string('categories', 'lifecycletrigger_categories'), $categorynames, $options);
+        $mform->addElement('autocomplete', 'categories',
+            get_string('categories', 'lifecycletrigger_categories'),
+            $categorynames, $options);
         $mform->setType('categories', PARAM_SEQUENCE);
 
         $mform->addElement('advcheckbox', 'exclude', get_string('exclude', 'lifecycletrigger_categories'));