Skip to content

Commit

Permalink
Form validation some fixes (#12466)
Browse files Browse the repository at this point in the history 
* fix form isValid method

* fix zephir syntax erros

* fix iterator - fix getMessageFor - unused getMessages byItemName

* changelog + add unit tests

* add test validation messages equals to form message

* prevent exception in getMessagesFor

* add test getMessagesFor for non existing element
  • Loading branch information
@sergeyklay
Mohamad authored and sergeyklay committed Dec 20, 2016
1 parent 924326e commit c505c68
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Fixed `Phalcon\Http\Request::getHeaders` to handle auth headers correctly [#12480](https://github.com/phalcon/cphalcon/issues/12480)
- Fixed `Phalcon\Http\Request::getMethod` to handle `X-HTTP-Method-Override` header correctly [#12478](https://github.com/phalcon/cphalcon/issues/12478)
- Fixed `Phalcon\Mvc\Model\Criteria::limit` and `Phalcon\Mvc\Model\Query\Builder::limit` to work with `limit` and `offset` properly [#12419](https://github.com/phalcon/cphalcon/issues/12419)
- Fixed `Phalcon\Forms\Form` to correct form validation and set messages for elements [#12465](https://github.com/phalcon/cphalcon/issues/12465), [#11500](https://github.com/phalcon/cphalcon/issues/11500), [#11135](https://github.com/phalcon/cphalcon/issues/11135), [#3167](https://github.com/phalcon/cphalcon/issues/3167), [#12395](https://github.com/phalcon/cphalcon/issues/12395)

# [3.0.2](https://github.com/phalcon/cphalcon/releases/tag/v3.0.2) (2016-11-26)
- Fixed saving snapshot data while caching model [#12170](https://github.com/phalcon/cphalcon/issues/12170), [#12000](https://github.com/phalcon/cphalcon/issues/12000)
Expand Down
111 changes: 45 additions & 66 deletions phalcon/forms/form.zep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
namespace Phalcon\Forms;

use Phalcon\Validation;
use Phalcon\ValidationInterface;
use Phalcon\DiInterface;
use Phalcon\FilterInterface;
use Phalcon\Di\Injectable;
Expand Down Expand Up @@ -255,9 +256,9 @@ class Form extends Injectable implements \Countable, \Iterator
*/
public function isValid(var data = null, var entity = null) -> boolean
{
var notFailed, messages, element,
validators, name, preparedValidators, filters,
validator, validation, elementMessages;
var validationStatus, messages, element,
validators, name, filters,
validator, validation, elementMessage;

if empty this->_elements {
return true;
Expand Down Expand Up @@ -290,17 +291,19 @@ class Form extends Injectable implements \Countable, \Iterator
}
}

let notFailed = true,
messages = [];
let validationStatus = true;

let validation = this->getValidation();

if typeof validation != "object" || !(validation instanceof ValidationInterface) {
// Create an implicit validation
let validation = new Validation();
}

for element in this->_elements {

let validators = element->getValidators();
if typeof validators != "array" {
continue;
}

if count(validators) == 0 {
if typeof validators != "array" || count(validators) == 0 {
continue;
}

Expand All @@ -309,25 +312,11 @@ class Form extends Injectable implements \Countable, \Iterator
*/
let name = element->getName();

/**
* Prepare the validators
*/
let preparedValidators = [];

/**
* Append (not overriding) element validators to validation class
*/
for validator in validators {
let preparedValidators[] = [name, validator];
}

let validation = this->getValidation();

if typeof validation == "object" {
if validation instanceof Validation {
// Set the validators to be validated
validation->setValidators(preparedValidators);
}
} else {
// Create an implicit validation
let validation = new Validation(preparedValidators);
validation->add(name, validator);
}

/**
Expand All @@ -341,21 +330,25 @@ class Form extends Injectable implements \Countable, \Iterator
if typeof filters == "array" {
validation->setFilters(name, filters);
}

/**
* Perform the validation
*/
let elementMessages = validation->validate(data, entity);
if count(elementMessages) {
let messages[name] = elementMessages;
let notFailed = false;
}
}

/**
* Perform the validation
*/
let messages = validation->validate(data, entity);
if messages->count() {
// Add validation messages to relevant elements
for elementMessage in iterator(messages) {
this->get(elementMessage->getField())->appendMessage(elementMessage);
}
messages->rewind();
let validationStatus = false;
}

/**
* If the validation fails update the messages
*/
if !notFailed {
if !validationStatus {
let this->_messages = messages;
}

Expand All @@ -369,51 +362,33 @@ class Form extends Injectable implements \Countable, \Iterator
/**
* Return the validation status
*/
return notFailed;
return validationStatus;
}

/**
* Returns the messages generated in the validation
*/
public function getMessages(boolean byItemName = false) -> <Group>
{
var messages, group, elementMessages;
var messages;

let messages = this->_messages;
if byItemName {
if typeof messages != "array" {
return new Group();
}
return messages;
}

let group = new Group();

if typeof messages == "array" {
for elementMessages in messages {
group->appendMessages(elementMessages);
}
if typeof messages == "object" && messages instanceof Group {
return messages;
}

return group;
return new Group();
}

/**
* Returns the messages generated for a specific element
*/
public function getMessagesFor(string! name) -> <Group>
{
var messages, elementMessages, group;

let messages = this->_messages;
if fetch elementMessages, messages[name] {
return elementMessages;
}

let group = new Group(),
this->_messages[name] = group;

return group;
if this->has(name) {
return this->get(name)->getMessages();
}
return new Group();
}

/**
Expand Down Expand Up @@ -712,7 +687,11 @@ class Form extends Injectable implements \Countable, \Iterator
public function rewind() -> void
{
let this->_position = 0;
let this->_elementsIndexed = array_values(this->_elements);
if typeof this->_elements == "array" {
let this->_elementsIndexed = array_values(this->_elements);
} else {
let this->_elementsIndexed = [];
}
}

/**
Expand Down
142 changes: 142 additions & 0 deletions tests/unit/Forms/FormTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Phalcon\Validation\Validator\PresenceOf;
use Phalcon\Validation\Validator\Regex;
use Phalcon\Validation\Validator\StringLength;
use Phalcon\Validation;

/**
* \Phalcon\Test\Unit\Forms\FormTest
Expand Down Expand Up @@ -398,4 +399,145 @@ function () {
}
);
}

/**
* Tests Element::hasMessages() Element::getMessages()
*
* @author Mohamad Rostami <[email protected]>
* @issue 11135, 3167
*/
public function testElementMessages()
{
$this->specify('Element messages are empty if form validation fails', function () {
// First element
$telephone = new Text('telephone');

$telephone->addValidators([
new Regex([
'pattern' => '/\+44 [0-9]+ [0-9]+/',
'message' => 'The telephone has an invalid format'
])
]);

// Second element
$address = new Text('address');
$form = new Form();

$form->add($telephone);
$form->add($address);

expect($form->isValid(['telephone' => '12345', 'address' => 'hello']))->false();
expect($form->get('telephone')->hasMessages())->true();
expect($form->get('address')->hasMessages())->false();

expect($form->get('telephone')->getMessages())->equals(
Group::__set_state([
'_messages' => [
Message::__set_state([
'_type' => 'Regex',
'_message' => 'The telephone has an invalid format',
'_field' => 'telephone',
'_code' => 0,
])
],
])
);
expect($form->get('telephone')->getMessages())->equals($form->getMessages());
expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []]));
expect($form->getMessagesFor('notelement'))->equals(Group::__set_state(['_messages' => []]));
});
}

/**
* Tests Form::setValidation()
*
* @author Mohamad Rostami <[email protected]>
* @issue 12465
*/
public function testCustomValidation()
{
$this->specify('Injecting custom validation to form doesn\'t validate correctly', function () {
// First element
$telephone = new Text('telephone');
$customValidation = new Validation();
$customValidation->add('telephone', new Regex([
'pattern' => '/\+44 [0-9]+ [0-9]+/',
'message' => 'The telephone has an invalid format'
]));
$form = new Form();
$address = new Text('address');
$form->add($telephone);
$form->add($address);
$form->setValidation($customValidation);
expect($form->isValid(['telephone' => '12345', 'address' => 'hello']))->false();
expect($form->get('telephone')->hasMessages())->true();
expect($form->get('address')->hasMessages())->false();
expect($form->get('telephone')->getMessages())->equals(
Group::__set_state([
'_messages' => [
Message::__set_state([
'_type' => 'Regex',
'_message' => 'The telephone has an invalid format',
'_field' => 'telephone',
'_code' => 0,
])
],
])
);
expect($form->get('telephone')->getMessages())->equals($form->getMessages());
expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []]));
});
}

/**
* Tests Form::isValid()
*
* @author Mohamad Rostami <[email protected]>
* @issue 11500
*/
public function testMergeValidators()
{
$this->specify('Injecting custom validation to form doesn\'t merge validators on isValid', function () {
// First element
$telephone = new Text('telephone');
$telephone->addValidators([
new PresenceOf([
'message' => 'The telephone is required'
])
]);
$customValidation = new Validation();
$customValidation->add('telephone', new Regex([
'pattern' => '/\+44 [0-9]+ [0-9]+/',
'message' => 'The telephone has an invalid format'
]));
$form = new Form();
$address = new Text('address');
$form->add($telephone);
$form->add($address);
$form->setValidation($customValidation);
expect($form->isValid(['address' => 'hello']))->false();
expect($form->get('telephone')->hasMessages())->true();
expect($form->get('address')->hasMessages())->false();
expect($form->get('telephone')->getMessages())->equals(
Group::__set_state([
'_messages' => [
Message::__set_state([
'_type' => 'Regex',
'_message' => 'The telephone has an invalid format',
'_field' => 'telephone',
'_code' => 0,
]),
Message::__set_state([
'_type' => 'PresenceOf',
'_message' => 'The telephone is required',
'_field' => 'telephone',
'_code' => 0,
])
],
])
);
expect($form->get('telephone')->getMessages())->equals($form->getMessages());
expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []]));
});
}
}

0 comments on commit c505c68

Please sign in to comment.