Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] cancelOnFail not working properly since 3.0.4 #13149

Closed
Frost007 opened this issue Nov 8, 2017 · 14 comments
Closed

[bug] cancelOnFail not working properly since 3.0.4 #13149

Frost007 opened this issue Nov 8, 2017 · 14 comments
Assignees
Labels
breaks bc Functionality that breaks Backwards Compatibility

Comments

@Frost007
Copy link

Frost007 commented Nov 8, 2017

Expected And Actual Behavior

I try to add validation to a form field with "cancelOnFail" attribute.
In 3.0.0 it worked as expected. it stopped the field's validation it self, but in 3.0.4 somehow it stops the whole validation. So i don't get any validation messages from the fields that are placed after the field that has cancelOnFail.

My field validation looks like this.

        $element = new Text('lastName');
        $element->setLabel('user.lastName');
        $element->setFilters([
            "string",
            "striptags",
            "trim",
        ]);
        $element->addValidators([
            new PresenceOf([
                'message'        => 'user.lastName.presenceOf'
                , 'cancelOnFail' => true
            ])
            , new StringLength([
                'min'               => 3,
                'max'               => 255,
                'messageMaximum'    => 'user.lastName.max',
                'messageMinimum'    => 'user.lastName.min'
            ])
        ]);
        $this->add($element);

        $element = new Text('firstName');
        $element->setLabel('user.firstName');
        $element->setFilters([
            "string",
            "striptags",
            "trim",
        ]);
        $element->addValidators([
            new PresenceOf([
                'message'        => 'user.firstName.presenceOf'
                , 'cancelOnFail' => true
            ])
            , new StringLength([
                'min'               => 3,
                'max'               => 255,
                'messageMaximum'    => 'user.firstName.max',
                'messageMinimum'    => 'user.firstName.min'
            ])
        ]);
        $this->add($element);

Lets say i only have these two field in the form. If i don't fill any of them, i should get 2 PresenceOf validation messages. One for 'firstName' and one for 'lastName'. But it's not the case i only get one message for 'lastName' and the cancelOnFail stop the whole validation.

Details

  • Phalcon version: 3.0.4
  • PHP Version: 7.0
  • Operating System: ubuntu 16.04
  • Installation type: Compiling from source
  • Zephir version (if any):
  • Server: Apache
  • Other related info (Database, table schema):
@laffentaller
Copy link

Same issue here:

PHP Version 7.0.22-0ubuntu0.16.04.1
Phalcon: 3.2.4
Installation type: from deb source (https://packagecloud.io/install/repositories/phalcon/stable/script.deb.sh)
Zephir version: Version 0.10.4-11e39849b0
Server: Apache/2.4.18 (Ubuntu)

@trix87
Copy link

trix87 commented Nov 16, 2017

Hello,

I am having the same issue but with the latest version 3.2.4.
The structure is as in the first post.

Details

Phalcon version: 3.2.4
PHP Version: 7.0
Operating System: Ubuntu 16.04
Installation type: Compiling from source
Zephir version (if any):
Server: Apache/2.4.18 (Ubuntu)
Other related info (Database, table schema):

@sergeyklay
Copy link
Contributor

Could you guys provide a minimal script, with which I would reproduce the error.

@sergeyklay sergeyklay added the need script to reproduce Script is required to reproduce the issue label Nov 19, 2017
@Frost007
Copy link
Author

Frost007 commented Nov 24, 2017

For example Auth controller has an index action where the user can log in.

public function indexAction(){
        
        $form = new LoginForm();

        if ($this->request->isPost() && $form->isValid($this->request->getPost())) {
       
        }

       $this->view->form = $form;
}

In the auth/index.phtml file:

<form method="post" action="">

    <?php $form->renderDecorated(); ?>

    <input type="submit" value="login">

</form>

The form could be the one that i wrote above. If you leave both of the fields empty than the validation fails and you get the first field validated not both.

@sergeyklay
Copy link
Contributor

@Frost007 Did you see #13167

@Frost007
Copy link
Author

Frost007 commented Jan 7, 2018

Yes, i saw that. what exactly dou you want me to provide?

@sergeyklay
Copy link
Contributor

As you can see the test passed. All works as expected. Could you please create a PR with test which will failed?

@sergeyklay
Copy link
Contributor

I'm closing this issue due to the lack of any reaction. I'll open it again if the need arises

chilimatic pushed a commit to chilimatic/cphalcon that referenced this issue Jan 15, 2018
@marcojetson
Copy link

marcojetson commented Mar 16, 2018

test expectation is wrong, issue creator says he is expecting 2 PresenceOf, one for firstName and one for lastName

before 3.0.3 Form::isValid() was running Validation::validate() one element at a time so cancelOnFail was cancelling the validators per element
since 3.0.3 this behaviour changed since Form::isValid() runs the validation once for all fields

@laffentaller
Copy link

laffentaller commented Apr 19, 2018

@sergeyklay : aggree with @marcojetson we should get two 'PresenceOf' messages in case firstName and lastName both leaved empty.

@sergeyklay sergeyklay reopened this Apr 19, 2018
@sergeyklay
Copy link
Contributor

@laffentaller Could you please create a PR with failed tests? Feel free use #13167 as example

@magosla
Copy link

magosla commented Apr 19, 2018

The script below shows how cancelOnFail=true cancels the validation of the remaining unvalidated fields
in the form

Phalcon version: 3.3.2
Server Software: Apache/2.4.6 (CentOS) PHP/7.0.28
Zephir version: Version 0.10.7-2917ebe8ae

use Phalcon\Forms\Element\TextArea,
    Phalcon\Forms\Element\Text,
    Phalcon\Forms\Element\Email,
    Phalcon\Validation\Validator\PresenceOf,
    Phalcon\Validation\Validator\StringLength;

/**
 * Description of ContactForm
 *
 * @author Magnus Lator
 */
class ContactForm extends \Phalcon\Forms\Form {

    public function initialize($entity = null, $options = null)
    {
        $this->fullname();
        $this->email();
        $this->subject();
        $this->message();
    }

    private function email()
    {
        $this->add((new Email('email', []))->setLabel('Email')
                        ->addValidators([
                            new PresenceOf(['message' => 'valid :field is required', 'cancelOnFail' => true]),
        ]));
    }

    private function fullname()
    {
        $this->add((new Text('fullname', ['placeholder' => 'First & Last Name',]))
                        ->setLabel('Full name')
                        ->addValidators([
                            new PresenceOf(['message' => 'your :field is required',
                                'cancelOnFail' => true]),
                            new StringLength(
                                    [
                                'max' => 20, 'messageMaximum' => ':field too long',
                                'min' => 2, 'messageMinimum' => ':field too short'
                                    ]
                            )
        ]));
    }

    private function subject()
    {
        $this->add((new Text('subject'))
                        ->addValidators([new PresenceOf(['message' => 'your :field is required',
                                'cancelOnFail' => true]),])
                        ->setLabel('Subject:')
        );
    }

    private function message()
    {
        $this->add((new TextArea('message'))
                        ->addValidators([
                            new PresenceOf(['message' => 'Your message is required', 'cancelOnFail' => true]),
                            new StringLength(
                                    [
                                'max' => 300, 'messageMaximum' => sprintf('your message can\'t be longer the %s characters', 300),
                                'min' => 4, 'messageMinimum' => 'Tell us what we can do for you', 'cancelOnFail' => true
                                    ]
                            )
                        ])
                        ->setLabel('Message?')
        );
    }

}

$form = new ContactForm();

if (!$form->isValid(['fullname' => '', 'email' => "", 'subject' => '', 'message' => ''])) {
    print_r($form->getMessages());
} else {
    echo "valid data";
}

@stale stale bot added the stale Stale issue - automatically closed label Jul 18, 2018
@stale stale bot closed this as completed Jul 19, 2018
@sergeyklay sergeyklay reopened this Jul 19, 2018
@stale stale bot removed the stale Stale issue - automatically closed label Jul 19, 2018
@sergeyklay sergeyklay removed need script to reproduce Script is required to reproduce the issue Not verified labels Aug 12, 2018
@sergeyklay sergeyklay added this to the unplanned milestone Aug 12, 2018
@sergeyklay sergeyklay assigned Jurigag and unassigned sergeyklay Aug 12, 2018
@niden
Copy link
Member

niden commented Feb 22, 2019

Also note suggestions in #13540

@niden niden removed this from the unplanned milestone May 16, 2019
@niden niden mentioned this issue May 16, 2019
4 tasks
@niden niden added the breaks bc Functionality that breaks Backwards Compatibility label May 16, 2019
niden added a commit that referenced this issue May 16, 2019
…dator for an element but continue to the rest
niden added a commit that referenced this issue May 16, 2019
niden added a commit that referenced this issue May 16, 2019
@niden
Copy link
Member

niden commented May 16, 2019

Resolved in #14083

@niden niden closed this as completed May 16, 2019
@niden niden added the 4.0 label Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks bc Functionality that breaks Backwards Compatibility
Projects
None yet
Development

No branches or pull requests

8 participants