newhck / php-form-builder-class

Automatically exported from code.google.com/p/php-form-builder-class
GNU General Public License v3.0
0 stars 0 forks source link

PHP Validation should be automatic/cleaner #19

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Hello,

At the moment i think the php validation code, at least the way its done in
the example is too messy, especially when you have the captcha verification
code as well.

It would be good if the php validation could be completely built in and
automated, perhaps when a form is submitting to itself, or an option to
turn on php validation that will automatically serialize the form for you
when you do the render method. The captcha verification should be automated
as well, since if you add a captcha you would expect it to check it. 

I have modified the form to do some automated checking, but at the moment I
don't have a copy to see exactly what I did. Tommorow ill post my
modifications and how creating a php validated form looks with those changes.

Also, thanks for your consistently fast responses, I hope you don't find my
suggestions attempts to be helpful annoying... 

Original issue reported on code.google.com by moncojhr@gmail.com on 25 Feb 2010 at 4:31

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Here is some code that i have working with some very horrible modification to 
the
class, it is already much neater... I've commented the lines on how i think it 
could
be made even nicer. Basically I've modified the render method to serialize
automatically for you under a fixed session id of reg_form_class. I don't 
completely
remember all the changes I made to get this to work.

<?php

// require the class
require 'classes/formbuilder/class.form.php';

$form = new form;

$cmd = $_POST['cmd'];

//There should be a single form method that returns if everything
// in the form was valid, with a single method to return the error_msg
// need a way to detect that form has been submitted without relying on them
// setting a hidden form with cmd=submit
if($cmd == 'submit') {

    $error_msg = $form->checkForm() ? $form->checkForm() . '<br />' : $validForm=TRUE ;
    $error_msg .= !$form->captchaIsValid() ? $form->captchaGetError() . '<br />' :
$captchavalid=TRUE;

}

//could be something like if($form->validated()==TRUE){}
if($captchavalid==TRUE && $validForm==TRUE && $_POST['cmd'] == 'submit') {

    //these should be unset when the method that checks all the parts in the 
    //form verifys that they are all valid
    unset($_SESSION["reg_form_class"]);
    unset($_SESSION["captchavalid"]);

    echo 'Form Valid!';

} else {

    //by default there could be a disablephpvalidation attribute that is off
    // by default and the user can turn it on...
    $form->setAttributes(array(
            "tableAttributes" => array("width" => "500")
    ));

    //perhaps this hidden field can be automatically added
    $form->addHidden("cmd", "submit");

    //a textbox that needs validation
    $form->addTextbox("Name:", "name", "", array("required" => 1));

    //if the captcha has been answered correctly once but there was an error
    // in some other part of the form the captcha should not be displayed
    // again until they have submitted the form correctly at least once or after
    // a timeout
    $form->addCaptcha();

    $form->addButton();

    $main .= '
       ' . $error_msg . ' Please fill out this form: ' . $form->render(TRUE);

}

echo $main;

?>

Original comment by moncojhr@gmail.com on 25 Feb 2010 at 5:31

GoogleCodeExporter commented 8 years ago
Thanks for your suggestions.  I will review the code you provided above and let 
you 
know what I think.

- Andrew

Original comment by ajporterfield@gmail.com on 25 Feb 2010 at 10:46

GoogleCodeExporter commented 8 years ago
Hi,

The changes I made to the class to get the code above to work was:

line 692:
    public function render($return=FALSE,$rendered=FALSE) {

        if(!empty($_SESSION["reg_form_class"]) && $rendered==FALSE) {
            //Instead of rebuilding the form, revive it from the stored shapshot.

            $form = unserialize($_SESSION["reg_form_class"]);
            return $form->render(TRUE,TRUE);
        }
            $_SESSION["reg_form_class"] = serialize($this);

line 2246:
    public function checkForm($rendered=FALSE) {

        if(!empty($_SESSION["reg_form_class"]) && $rendered==FALSE) {
            //Instead of rebuilding the form, revive it from the stored shapshot.

            $form = unserialize($_SESSION["reg_form_class"]);

            //Store reference values for pre-filling form fields.
            $form->setReferenceValues($_POST);
            $_SESSION["reg_form_class"] = serialize($form);
            return $form->checkForm(TRUE);

        }
        $_SESSION["reg_form_class"] = serialize($this);

I'm not suggesting we use this, just if you wanted to test my above code.

Original comment by moncojhr@gmail.com on 1 Mar 2010 at 12:10

GoogleCodeExporter commented 8 years ago
I started working on an updated solution for the php validation this evening.  
I will keep 
you posted on my progress.

- Andrew

Original comment by ajporterfield@gmail.com on 10 Mar 2010 at 3:23

GoogleCodeExporter commented 8 years ago
I have a solution in place on a development copy of the project that I think is 
going to 
turn out nicely. It's about 90% of the way there.  I still need to iron out a 
few 
things...one of which being how to handle the situation where multiple forms 
are 
generated on the same page.  I hope to get a copy to you by the end of the week 
for 
you to look over and get your feedback before I move forward and publish.

- Andrew

Original comment by ajporterfield@gmail.com on 10 Mar 2010 at 6:36

GoogleCodeExporter commented 8 years ago
Hi great, will be interesting to see how you tackled the problem.

Depending on how your doing it, it would make sense if each form got its own 
unique
session identifier, so when a new form is created it either gets a incremental 
id or
a user specified id.

Looking forward to seeing it :-)

Original comment by moncojhr@gmail.com on 11 Mar 2010 at 3:00

GoogleCodeExporter commented 8 years ago
Attached is a working copy of what I have so far.  There is an example file 
included 
that demos the new functionality.  

Some of the changes made are not backwards compatible with previous versions of 
this project.  For instance, the checkForm function and the old captcha 
validation 
function are no longer included.

Take a look and let me know what you think.

- Andrew

Original comment by ajporterfield@gmail.com on 11 Mar 2010 at 10:00

Attachments:

GoogleCodeExporter commented 8 years ago
Hi, i'll have a better look at it soon, definitely looking better though!

The first thing i thought though when i looked was, why dont you have
session_start(); in the class? 

Only had time to look at the example and not the changes in the class, ill post 
again
soon

Original comment by moncojhr@gmail.com on 12 Mar 2010 at 7:09

GoogleCodeExporter commented 8 years ago
session_start() is not included in the class b/c it must be invoked before any 
output 
to the browser.  If you do include session_start after output, you will get a 
warning 
similar to the line provided below.

Warning: session_start() [function.session-start]: Cannot send session cache 
limiter - 
headers already sent (output started at...

Also, calling session_start() when a session is already started will produce a 
notice 
similar to the line provided below.

Notice: A session had already been started - ignoring session_start() in...

Original comment by ajporterfield@gmail.com on 12 Mar 2010 at 2:40

GoogleCodeExporter commented 8 years ago
So i've given this a fairly good look, its great! I'll attach a copy of the 
example
how i would use it... but it seems fairly obvious that in your example you are
expecting people to be submitting to another file and validating there and 
returning,
rather then submitting to the same file and taking a different code path. Which 
is
what I would do.

The one idea I have left since each form now gets an identifier, is to have an
automatic hidden field that would basically be the same as...

$form->addHidden("validation_example_1", "submit");

That way you'll know which form was submitted, im sure there would be many 
other good
uses for that as well, you would also be able to use it instead of having to add
$form->addHidden("cmd", "submit"); which practically all the examples have.

Perhaps you should put up a TODO on the wiki? I would say creating more user
definable error checks than just "required" would be the next step.

I suppose it doesnt really matter but, If somebody submits the form without the
recaptcha_challenge_field or recaptcha_response_field you will generate a 
notice...

Notice: Undefined index: recaptcha_challenge_field in 
formbuilder\class.form.php on
line 2731

Notice: Undefined index: recaptcha_response_field in formbuilder\class.form.php 
on
line 2731

Original comment by moncojhr@gmail.com on 15 Mar 2010 at 2:46

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for your suggestions and for reviewing the changes made to the php 
validation 
process.  I hope to get a new version release out by the end of the week 
including these 
changes as well as updated example files.

Thanks,
Andrew  

Original comment by ajporterfield@gmail.com on 15 Mar 2010 at 2:08

GoogleCodeExporter commented 8 years ago
I added a new wiki page, http://code.google.com/p/php-form-builder-
class/wiki/NewFeatureRequests, for collecting new feature requests from 
developers 
using this project.

Also, I added you are a contributor for this project viewable from 
http://code.google.com/p/php-form-builder-class/people/list.

- Andrew

Original comment by ajporterfield@gmail.com on 16 Mar 2010 at 7:38