[dev] [commits] Horde branch Horde_LoginTasks-Refactor created. b70b84343d05538bad914a0738ae25e63ab31806
p at rdus.de
Tue Mar 9 16:04:08 UTC 2010
Quoting Michael M Slusarz <slusarz at horde.org>:
> Quoting Gunnar Wrobel <p at rdus.de>:
>>> For example, if you have the following tasks:
>>> There should be 3 confirmation screens generated: 1 with
>>> CONFIRM_NO and CONFIRM_YES tasks,
>> As CONFIRM_NO != CONFIRM_YES the check "$v['task']->display !=
>> $display" in needDisplay() should disallow displaying both on the
>> same screen or do I misunderstand that section?
> I think the code is correct. When adding the task with addTask(),
> CONFIRM_NO will have 'display' set to true and $_addFlag will be
> false. CONFIRM _YES will also have 'display set to true since it is
> not DISPLAY_NONE and $_addFlag is not set. Therefore, when you run
> needDisplay(), both tasks will skip the break statement (in our
> example, CONFIRM_NO will skip because 'displa'y is true and $display
> is null; CONFIRM_YES will skip because 'display' is true and
Everything correct ...
> $v['task']->display == $display).
... except for this statement. In the first round when handling
CONFIRM_NO $display will be set to CONFIRM_NO while
$v['task']->display is CONFIRM_YES in the second round.
Just to be certain I tested this outside of my test suite by adding
two dummy tasks to a horde base installation. I do indeed only get one
After confirming this first task I get redirected to the portal
instead of getting the remaining login tasks displayed. This is
something I also see in my unit tests. The redirection will transport
me to the URL initially provided to runTasks() instead of moving to
the login tasks page again.
I'm going to provide suggested fixes in the branch.
>> I'm done with the core part of the refactoring. Having the abstract
>> Horde_LoginTasks_Backend class should be sufficient for me to plug
>> the system into something other than Horde. Can you give me some
>> feedback if you think the modifications are okay and if I can merge?
> I will take a look. It would be best to move all Horde-specific
> backend stuff to Core - like I did with the Notification driver.
> That would remove all horde core-specific code from LoginTasks.
This would mean moving Horde_LoginTasks_Backend_Horde into
Horde_Core_LoginTasks_Backend, right? That would make sense to me. I'd
convert Horde_LoginTasks_Backend to an interface in that case (rather
than the abstract class it is now).
>> Two additional items...
>> One question concerning ONCE tasks: Why is the _once value set when
>> the task is being added to the tasklist? Shouldn't it be set once
>> the task was actually executed? I didn't test this but I assume the
>> user could login, visit the confirmation page, kill his session,
>> login again and thus avoid execution of the task.
> That's fine. To me this is an implicit decision by the user that
> they didn't want the task to be run.
Ah. I was thinking of tasks such as the TOS agreement that are
required to be confirmed. I'll have to think about this again.
>> The Horde_Util::getFormData() in runTasks() is still inconvenient
>> as it accesses $_POST directly so I guess I will add a workaround
>> there later.
> You could use Horde_Variables instead (from the Util package). The
> Util package is already marked as a dependency of LoginTasks.
Thanks for the hint, I'll look into that.
I noticed some additional things when I started using the package
outside of Horde now. I can't place the redirection feature into the
backend as I'm in a Zend MVC context. And redirection needs to be done
by the controller. So runTasks() must return the redirection address
so that the controller can perform the actual redirection. This is
something already supported as runTasks() returns the result of the
redirect call to the backend. Hope that is okay.
And I need to consider if it is possible to have tasks that redirect
to their own specific URLs rather than a global login task URL.
> Michael Slusarz [slusarz at horde.org]
> Horde developers mailing list - Join the hunt: http://horde.org/bounties/
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 197 bytes
Desc: Digitale PGP-Unterschrift
More information about the dev