Remove the _addFlag.

I still don't grasp its logic and it prevents the display sequence I
currently expect. Probably my expectations are wrong. Anyhow, I
wrapped my idea of how the display series should look like into a
lengthy test (that would need to be slimmed down at some point):

Assume you have a list of tasks with

DISPLAY_CONFIRM_NO
DISPLAY_CONFIRM_YES
DISPLAY_NONE
DISPLAY_NOTICE
DISPLAY_CONFIRM_YES
DISPLAY_NONE
DISPLAY_CONFIRM_YES
DISPLAY_NOTICE

then there would be five screens with

 - 1 with CONFIRM_NO and CONFIRM_YES tasks
 - 1 with NOTICE task
 - 1 with CONFIRM_YES task
 - 1 with CONFIRM_YES task
 - 1 with NOTICE task

Once the user posted to the login service page he is redirected to the
login service page again in case the tasks are not finished. Only in
case all tasks are completed he is beind redirected to the final
redirect URL provided on the first call of runTasks().

diff --git a/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php b/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
index ecdcbbc..0c54e68 100644 (file)
--- a/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
+++ b/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
@@ -41,13 +41,6 @@ class Horde_LoginTasks_Tasklist
     protected $_tasks = array();
 
     /**
-     * Internal flag for addTask().
-     *
-     * @var boolean
-     */
-    protected $_addFlag = false;
-
-    /**
      * Current task location pointer.
      *
      * @var integer
@@ -66,14 +59,8 @@ class Horde_LoginTasks_Tasklist
             'task' => $task
         );
 
-        if (($task->display == Horde_LoginTasks::DISPLAY_AGREE) ||
-            ($task->display == Horde_LoginTasks::DISPLAY_NOTICE)) {
-            $tmp['display'] = true;
-            $this->_addFlag = true;
-        } elseif (($task->display != Horde_LoginTasks::DISPLAY_NONE) &&
-                  !$this->_addFlag) {
+        if ($task->display != Horde_LoginTasks::DISPLAY_NONE) {
             $tmp['display'] = true;
-            $this->_addFlag = false;
         }
 
         switch ($task->priority) {

diff --git a/framework/LoginTasks/test/Horde/LoginTasks/LoginTasksTest.php b/framework/LoginTasks/test/Horde/LoginTasks/LoginTasksTest.php
index 1643c7b..1f126be 100644 (file)
--- a/framework/LoginTasks/test/Horde/LoginTasks/LoginTasksTest.php
+++ b/framework/LoginTasks/test/Horde/LoginTasks/LoginTasksTest.php
@@ -524,6 +524,94 @@ class Horde_LoginTasks_LoginTasksTest extends PHPUnit_Framework_TestCase
             ),
             Horde_LoginTasks_Stub_Task::$executed
         );
+        $tasklist = $tasks->displayTasks();
+        $classes = array();
+        foreach ($tasklist as $task) {
+            $classes[] = get_class($task);
+        }
+        asort($classes);
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_ConfirmTwo',
+            ),
+            $classes
+        );
+        $_POST['logintasks_confirm_0'] = true;
+        $this->assertContains(
+            'http:///services/logintasks.php?app=test',
+            (string) $tasks->runTasks(true)
+        );
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_ConfirmNo',
+                'Horde_LoginTasks_Stub_Confirm',
+                'Horde_LoginTasks_Stub_Task',
+                'Horde_LoginTasks_Stub_Notice',
+                'Horde_LoginTasks_Stub_ConfirmTwo',
+                'Horde_LoginTasks_Stub_TaskTwo',
+            ),
+            Horde_LoginTasks_Stub_Task::$executed
+        );
+        $_POST = array();
+        $tasklist = $tasks->displayTasks();
+        $classes = array();
+        foreach ($tasklist as $task) {
+            $classes[] = get_class($task);
+        }
+        asort($classes);
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_ConfirmThree',
+            ),
+            $classes
+        );
+        $_POST['logintasks_confirm_0'] = true;
+        $this->assertContains(
+            'http:///services/logintasks.php?app=test',
+            (string) $tasks->runTasks(true)
+        );
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_ConfirmNo',
+                'Horde_LoginTasks_Stub_Confirm',
+                'Horde_LoginTasks_Stub_Task',
+                'Horde_LoginTasks_Stub_Notice',
+                'Horde_LoginTasks_Stub_ConfirmTwo',
+                'Horde_LoginTasks_Stub_TaskTwo',
+                'Horde_LoginTasks_Stub_ConfirmThree',
+            ),
+            Horde_LoginTasks_Stub_Task::$executed
+        );
+        $_POST = array();
+        $tasklist = $tasks->displayTasks();
+        $classes = array();
+        foreach ($tasklist as $task) {
+            $classes[] = get_class($task);
+        }
+        asort($classes);
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_NoticeTwo',
+            ),
+            $classes
+        );
+        $this->assertContains(
+            'redirect',
+            (string) $tasks->runTasks(true)
+        );
+        $this->assertEquals(
+            array(
+                'Horde_LoginTasks_Stub_ConfirmNo',
+                'Horde_LoginTasks_Stub_Confirm',
+                'Horde_LoginTasks_Stub_Task',
+                'Horde_LoginTasks_Stub_Notice',
+                'Horde_LoginTasks_Stub_ConfirmTwo',
+                'Horde_LoginTasks_Stub_TaskTwo',
+                'Horde_LoginTasks_Stub_ConfirmThree',
+                'Horde_LoginTasks_Stub_NoticeTwo',
+            ),
+            Horde_LoginTasks_Stub_Task::$executed
+        );
     }
 
     private function _getLoginTasks(