Page 1 of 1

Organized Form Handling

Posted: Thu May 31, 2007 8:07 pm
by superdezign
This is something that I've been trying for the life of me to make look pretty.

Classes look pretty.
Objects running functions look pretty.
Small echoes based on ternary operators here and there look pretty.
Includes and requires look pretty.

But long lists of nested ifs and elses checking for isset(), !empty(), and whether or not something was properly updated, then creating an error message for later display looks ugly.

Once, what I did was separate the form handling into the class that it affected. The class would have a function Update() which would take the $_POST array as a parameter, and return constants (or an array of constants) to give me the results.

Code: Select all

function Update($post)
{
    if(!isset($post['username']) || !isset($post['password']))
    {
        return ERROR_FILLED;
    }
    
    if(!$this->Valid($post['username'], $post['password']))
    {
        return ERROR_INVALIDUSER;
    }
    
    return SUCCESS;
}
Then, I'd sort through the results in the page through a switch statement (or foreach if it was an array), and output the appropriate error messages.

Code: Select all

// Attempt to update
if(!empty($_POST))
    $result = $pUser->Update($_POST);
if($result == SUCCESS) header('Location: somewhere else');

...

switch($result)
{
    case ERROR_FILLED: echo 'Fill it up, man.'; break;
    case ERROR_INVALIDUSER: echo 'Who do you think you are? Scram.'; break;
    case SUCCESS: /* If header redirect didn't work */ echo 'Go <a href="somewhere else">here</a>'; break;
}
That is the neatest that I've ever made it and I liked being able to call a function return instead of nested ifs. However, it's definitely the less efficient of the two options. Also, I don't feel that it separates functionality from the form because the Update() function ends up being extremely application specific.


I was just curious to know how you guys are doing yours. I tried to get away from the method I just displayed, but I'm seriously considering going back to it.

Posted: Fri Jun 01, 2007 5:20 pm
by Ollie Saunders
Classes look pretty.
Objects running functions look pretty.
Small echoes based on ternary operators here and there look pretty.
Includes and requires look pretty.

But long lists of nested ifs and elses checking for isset(), !empty(), and whether or not something was properly updated, then creating an error message for later display looks ugly.
Pick up a copy of refactoring and learn more about ugly and pretty things and why they are so. Alternatively you might benefit from attempting explain to me why you think those things are pretty/ugly.

If you are feeling adventurous you can check out my solution to form validation

Another cool thing to so is let a superclass handle some of the real grass roots logic of the form. Oh go on I'll just give you mine of particular interest is the run() method

Code: Select all

<?php
require_once 'Ole/Entity/MsgSet/Factory.php';
/**
 * Implementation of template method on run() means you can just extend these
 * filling in the hooks and abstract methods to create well organised form
 * functionality.
 *
 * Dependent on Ole_Entity_MsgSet_*
 */
abstract class Ole_Entity_Form_Abstract
{
    const SESSION_EXPIRATION_THRESHOLD_IN_MINUTES = 18;
    protected $_controllerAction;
    /**
     * @var Ole_View_Abstract
     */
    private $_view;
    /**
     * @var Ole_Entity_MsgSet_Multi
     */
    protected $_errors;
    /**
     * @var Ole_Entity_MsgSet_Multi
     */
    protected $_notifications;
    /**
     * @var Ole_Entity_MsgSet_Single
     */
    protected $_formErrors;
    /**
     * @var Ole_Entity_MsgSet_Single
     */
    protected $_formNotifications;
    /**
     * @param Ole_Controller_Action $controllerAction
     */
    public function __construct(Ole_Controller_Action $controllerAction)
    {
        $this->_controllerAction = $controllerAction;

        $msgSetFactory = new Ole_Entity_MsgSet_Factory();
        $this->_errors            = $msgSetFactory->mkMulti('error');
        $this->_notifications     = $msgSetFactory->mkMulti('notification');
        $this->_formErrors        = $msgSetFactory->mkSingle('error');
        $this->_formNotifications = $msgSetFactory->mkSingle('notification');
    }
    public function getView()
    {
        return $this->_view;
    }
    public function setView(Ole_View_Abstract $view)
    {
        $view->assign(array(
            'errors'            => $this->_errors,
            'notifications'     => $this->_notifications,
            'formErrors'        => $this->_formErrors,
            'formNotifications' => $this->_formNotifications,
        ));
        $this->_view = $view;
    }
    public function run()
    {
        $this->_onInitialize();
    	if ($isSubmitted = $this->isSubmitted()) {
    	    if ($this->_checkToken()) {
        	    if ($this->_onSubmit()) {
        	        $this->_onValid();
        	    } else {
        	        $this->_onInvalid();
        	    }
    	    } else {
    	        $this->_formErrors->add('You session has expired. Click submit if you wish to submit this form.');
    	    }
    	} else {
        	$this->_setToken();
        	$this->_onPopulate();
    	}
    	return $isSubmitted;
    }
    /**
     * Prevents CSRF
     */
    private function _setToken()
    {
    	@session_start();
    	if (!isset($_SESSION['formTokens'])) {
    	    $_SESSION['formTokens'] = array();
    	}
    	$_SESSION['formTokens'][get_class($this)] = time();
    }
    /**
     * Prevents CSRF
     */
    private function _checkToken()
    {
        @session_start();
        if (!isset($_SESSION['formTokens'][get_class($this)])) {
            return false;
        }
        $expire = self::SESSION_EXPIRATION_THRESHOLD_IN_MINUTES * 60;
        if (time() - $_SESSION['formTokens'][get_class($this)] > $expire) {
            return false;
        }
        return true;
    }
    abstract public function isSubmitted();
    public function render()
    {
        echo $this->getView()->render();
    }
    protected function _onPopulate() {}
    protected function _onInitialize() {}
    protected function _onSubmit() {}
    protected function _onValid() {}
    protected function _onInvalid() {}
}

Posted: Sat Jun 02, 2007 9:28 am
by superdezign
Okay, so I read this class and realized that I wouldn't even know where to start creating one such as this.

Reading through, it seems that this abstract is meant to be extended for each form. So every form would have it's own class, each of which defines the conditions for isSubmitted().

_onInitialize creates the form
_onSubmit validates the form.
_onValid defines actions to be taken upon success.
_onInvalid defines actions to be taken on failure.
_onPopulate... I'm clueless as to what it does.

I also have no idea what CSRF is, but it looks at though you give the user 18 minutes to complete the form and, if they don't do it within that time limit, they receive the error. However, I don't see where this expiration value is reset.

What confuses me the most is making it work in an organized fashion. Is the form's format meant to be defined in a Ole_View class? Thinking about how that class takes the errors and such, I can see the errors being easily placed both at the top of a form and beside the form element that it applies to, but does the Ole_View class also take a FormElement abstract class to define the different types of form elements, as well as maybe, adding captions?

You know.... The more I ask these questions, the more it makes sense :-p
The only thing I'm still a little worried about is database interaction.



Any way you could show me an example implementation of this onto a simple... login form, maybe? And, just for kicks, throw in a "remember username" checkbox?

Posted: Sat Jun 02, 2007 1:02 pm
by Ollie Saunders
it seems that this abstract is meant to be extended for each form. So every form would have it's own class, each of which defines the conditions for isSubmitted().

_onInitialize creates the form
_onSubmit validates the form.
_onValid defines actions to be taken upon success.
_onInvalid defines actions to be taken on failure.
Correct.
_onPopulate... I'm clueless as to what it does.
Where you set the initial values for the form elements usually these are obtained from a database. Not all forms need this.
I also have no idea what CSRF is, but it looks at though you give the user 18 minutes to complete the form and, if they don't do it within that time limit, they receive the error.
A typical CSRF exploit involves a hacker using a XSS vulnerability to generate an img tag with a src attribute of a form page and have an unknowing accomplice request the image and send a form submission. Forms that are sent via GET are particularly vulnerable. I must admit I can't completely remember how it works but I have implemented the recommended defence here. The time limit can be adjusted to taste. 18 minutes has been enough for my needs because my forms are relatively short but for a long survey form you might want to remove the time limit entirely.
However, I don't see where this expiration value is reset.
I think if the value was reset it wouldn't work. The token should only be set on the initial request.
Ole_View class also take a FormElement abstract class to define the different types of form elements, as well as maybe, adding captions?
Most people find view helpers are fine for this. I actually use an Entity Renderer pattern but I'm still in the process of defining exactly what that is.
Any way you could show me an example implementation of this onto a simple... login form, maybe? And, just for kicks, throw in a "remember username" checkbox?
Why don't you have a try and I'll point you in the right direction with any problems you have. One day I might write a tutorial on it.