Bug #8475: Better way to make sure system tasks are always run

diff --git a/framework/Core/lib/Horde/Registry.php b/framework/Core/lib/Horde/Registry.php
index 2f8c082..585b57d 100644 (file)
--- a/framework/Core/lib/Horde/Registry.php
+++ b/framework/Core/lib/Horde/Registry.php
@@ -916,7 +916,9 @@ class Horde_Registry
         /* Do login tasks. */
         if ($checkPerms) {
             $tasks = Horde_LoginTasks::singleton($app, Horde::selfUrl(true, true, true));
-            $tasks->runTasks(array('runtasks' => !empty($options['logintasks'])));
+            if (!empty($options['logintasks'])) {
+                $tasks->runTasks();
+            }
         }
 
         /* Include base.php file. */

diff --git a/framework/LoginTasks/lib/Horde/LoginTasks.php b/framework/LoginTasks/lib/Horde/LoginTasks.php
index f5a8335..ab48754 100644 (file)
--- a/framework/LoginTasks/lib/Horde/LoginTasks.php
+++ b/framework/LoginTasks/lib/Horde/LoginTasks.php
@@ -144,7 +144,7 @@ class Horde_LoginTasks
             array_unshift($app_list, 'horde');
         }
 
-        $systemtasks = $tasks = array();
+        $tasks = array();
 
         foreach ($app_list as $app) {
             $fileroot = $GLOBALS['registry']->get('fileroot', $app);
@@ -154,11 +154,7 @@ class Horde_LoginTasks
                         foreach (scandir($fileroot . '/lib/LoginTasks/' . $val) as $file) {
                             $classname = $app . '_LoginTasks_' . $val . '_' . basename($file, '.php');
                             if (class_exists($classname)) {
-                                if ($val == 'SystemTask') {
-                                    $systemtasks[$classname] = $app;
-                                } else {
-                                    $tasks[$classname] = $app;
-                                }
+                                $tasks[$classname] = $app;
                             }
                         }
                     }
@@ -166,10 +162,6 @@ class Horde_LoginTasks
             }
         }
 
-        /* Need to make sure all systemtasks are run before regular tasks
-         * are run. */
-        $tasks = array_merge($systemtasks, $tasks);
-
         if (empty($tasks)) {
             return;
         }
@@ -225,7 +217,11 @@ class Horde_LoginTasks
             }
 
             if ($addtask) {
-                $this->_tasklist->addTask($ob);
+                if ($ob instanceof Horde_LoginTasks_SystemTask) {
+                    $ob->execute();
+                } else {
+                    $this->_tasklist->addTask($ob);
+                }
             }
         }
     }
@@ -237,27 +233,19 @@ class Horde_LoginTasks
      * login and will redirect to the login tasks page if necessary.  This is
      * the function that should be called from the application upon login.
      *
-     * @param array $options  Options:
-     * <pre>
-     * 'confirmed' - (boolean) If true, indicates that any pending actions
-     *                         have been confirmed by the user.
-     * 'runtasks' - (boolean) If true, run all tasks. If false, only run
-     *                        system tasks.
-     * </pre>
+     * @param boolean $confirmed  If true, indicates that any pending actions
+     *                            have been confirmed by the user.
      */
-    public function runTasks($options = array())
+    public function runTasks($confirmed = false)
     {
         if (!isset($this->_tasklist) ||
             ($this->_tasklist === true)) {
             return;
         }
 
-        $options['advance'] = $this->_init || !empty($options['confirmed']);
-
         /* Perform ready tasks now. */
-        foreach ($this->_tasklist->ready($options) as $key => $val) {
-            if ($val instanceof Horde_LoginTasks_SystemTask ||
-                in_array($val->display, array(self::DISPLAY_AGREE, self::DISPLAY_NOTICE, self::DISPLAY_NONE)) ||
+        foreach ($this->_tasklist->ready($this->_init || $confirmed) as $key => $val) {
+            if (in_array($val->display, array(self::DISPLAY_AGREE, self::DISPLAY_NOTICE, self::DISPLAY_NONE)) ||
                 Horde_Util::getFormData('logintasks_confirm_' . $key)) {
                 $val->execute();
             }

diff --git a/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php b/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
index 99364ad..da11d2f 100644 (file)
--- a/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
+++ b/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
@@ -69,11 +69,6 @@ class Horde_LoginTasks_Tasklist
             'task' => $task
         );
 
-        if ($task instanceof Horde_LoginTasks_SystemTask) {
-            $this->_tasks[] = $tmp;
-            return;
-        }
-
         if (($task->display == Horde_LoginTasks::DISPLAY_AGREE) ||
             ($task->display == Horde_LoginTasks::DISPLAY_NOTICE)) {
             $tmp['display'] = true;
@@ -98,32 +93,23 @@ class Horde_LoginTasks_Tasklist
     /**
      * Returns the list of tasks to perform.
      *
-     * @param array $options  Options:
-     * <pre>
-     * 'advance' - (boolean) If true, mark ready tasks as completed.
-     * 'runtasks' - (boolean) If true, run all tasks. If false, only run
-     *              system tasks.
-     * </pre>
-     *
-     * @param boolean $complete  Mark ready tasks as completed?
+     * @param boolean $advance  If true, mark ready tasks as completed.
      *
      * @return array  The list of tasks to perform.
      */
-    public function ready($options = array())
+    public function ready($advance = false)
     {
         $tmp = array();
 
         reset($this->_tasks);
         while (list($k, $v) = each($this->_tasks)) {
-            if (($v['task'] instanceof Horde_LoginTasks_Task) &&
-                (empty($options['runtasks']) ||
-                 ($v['display'] && ($k >= $this->_ptr)))) {
+            if ($v['display'] && ($k >= $this->_ptr)) {
                 break;
             }
             $tmp[] = $v['task'];
         }
 
-        if (!empty($options['advance'])) {
+        if ($advance) {
             $this->_tasks = array_slice($this->_tasks, count($tmp) + $this->_ptr);
             $this->_ptr = 0;
         }