From ce3d0ed4077ceb492bdb6081d7769abadce19902 Mon Sep 17 00:00:00 2001
From: Gunnar Wrobel <p@rdus.de>
Date: Tue, 9 Mar 2010 18:52:08 +0100
Subject: [PATCH] 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().
---
 .../LoginTasks/lib/Horde/LoginTasks/Tasklist.php   | 15 +---
 .../test/Horde/LoginTasks/LoginTasksTest.php       | 88 ++++++++++++++++++++++
 2 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php b/framework/LoginTasks/lib/Horde/LoginTasks/Tasklist.php
index ecdcbbca8..0c54e686d 100644
--- 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 1643c7b3c..1f126bed5 100644
--- 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(
-- 
2.11.0