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.

Sign up for GitHub

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

Jump to bottom

T13149 cancel on fail #14083

Merged
merged 4 commits into from
May 16, 2019
Merged

T13149 cancel on fail #14083

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da0c60d
[#13149] - Added test form and failing test
niden May 16, 2019
f06a02c
[#13149] - Adjusted the code to fail on the first `cancelOnFail` vali…
niden May 16, 2019
fa44a5c
[#13149] - Adjusted combined validators
niden May 16, 2019
26f6343
[#13149] - Updated changelog
niden May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[#13149] - Adjusted the code to fail on the first cancelOnFail vali… 
…dator for an element but continue to the rest
  • Loading branch information
@niden
niden committed May 16, 2019
commit f06a02c9bd08cbd65fb6aa21eba0813c0ea42fc6
6 changes: 3 additions & 3 deletions phalcon/Forms/Form.zep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ class Form extends Injectable implements \Countable, \Iterator, AttributesInterf
*/
public function getValue(string! name) -> var | null
{
var entity, value, data, $internal, element;
var entity, value, data, internalEntity, element;
array forbidden;
string method;

Expand Down Expand Up @@ -490,8 +490,8 @@ class Form extends Injectable implements \Countable, \Iterator, AttributesInterf
/**
* Check if the method is internal
*/
let $internal = strtolower(name);
if isset forbidden[$internal] {
let internalEntity = strtolower(name);
if isset forbidden[internalEntity] {
return null;
}

Expand Down
112 changes: 57 additions & 55 deletions phalcon/Validation.zep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ class Validation extends Injectable implements ValidationInterface
let this->combinedFieldsValidators[] = [field, validator];
} else {
for singleField in field {
let this->validators[] = [singleField, validator];
let this->validators[singleField][] = validator;
}
}
} elseif typeof field == "string" {
let this->validators[] = [field, validator];
let this->validators[field][] = validator;
} else {
throw new Exception(
"Field must be passed as array of fields or string"
Expand Down Expand Up @@ -440,13 +440,13 @@ class Validation extends Injectable implements ValidationInterface
*/
public function validate(var data = null, var entity = null) -> <Messages>
{
var validators, messages, scope, field, validator, status,
combinedFieldsValidators;
var combinedFieldsValidators, field, messages, status, validator,
validatorData, validators;

let validators = this->validators;
let combinedFieldsValidators = this->combinedFieldsValidators;
let validatorData = this->validators,
combinedFieldsValidators = this->combinedFieldsValidators;

if unlikely typeof validators != "array" {
if unlikely typeof validatorData != "array" {
throw new Exception("There are no validators to validate");
}

Expand Down Expand Up @@ -485,62 +485,64 @@ class Validation extends Injectable implements ValidationInterface
let this->data = data;
}

for scope in validators {
if unlikely typeof scope != "array" {
throw new Exception("The validator scope is not valid");
}

let field = scope[0],
validator = scope[1];

if unlikely typeof validator != "object" {
throw new Exception("One of the validators is not valid");
}
for field, validators in validatorData {
// if unlikely typeof scope != "array" {
// throw new Exception("The validator scope is not valid");
// }
//
// let field = scope[0],
// validator = scope[1];
for validator in validators {
if unlikely typeof validator != "object" {
throw new Exception("One of the validators is not valid");
}

/**
* Call internal validations, if it returns true, then skip the
* current validator
*/
if this->preChecking(field, validator) {
continue;
}
/**
* Call internal validations, if it returns true, then skip the
* current validator
*/
if this->preChecking(field, validator) {
continue;
}

/**
* Check if the validation must be canceled if this validator fails
*/
if validator->validate(this, field) === false {
if validator->getOption("cancelOnFail") {
break;
/**
* Check if the validation must be canceled if this validator fails
*/
if validator->validate(this, field) === false {
if validator->getOption("cancelOnFail") {
break;
}
}
}
}

for scope in combinedFieldsValidators {
if unlikely typeof scope != "array" {
throw new Exception("The validator scope is not valid");
}

let field = scope[0],
validator = scope[1];

if unlikely typeof validator != "object" {
throw new Exception("One of the validators is not valid");
}
for validator in combinedFieldsValidators {
// if unlikely typeof scope != "array" {
// throw new Exception("The validator scope is not valid");
// }
//
// let field = scope[0],
// validator = scope[1];
for validator in validators {
if unlikely typeof validator != "object" {
throw new Exception("One of the validators is not valid");
}

/**
* Call internal validations, if it returns true, then skip the
* current validator
*/
if this->preChecking(field, validator) {
continue;
}
/**
* Call internal validations, if it returns true, then skip the
* current validator
*/
if this->preChecking(field, validator) {
continue;
}

/**
* Check if the validation must be canceled if this validator fails
*/
if validator->validate(this, field) === false {
if validator->getOption("cancelOnFail") {
break;
/**
* Check if the validation must be canceled if this validator fails
*/
if validator->validate(this, field) === false {
if validator->getOption("cancelOnFail") {
break;
}
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions tests/_data/fixtures/Forms/ValidationForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ public function initialize($entity = null, $options = null)
]
),
];
$field = new Text(
'fullname',
[
'placeholder' => 'First & Last Name',
]
);
$field = new Text('fullname');

$field->addValidators($validators);

Expand All @@ -61,7 +56,6 @@ public function initialize($entity = null, $options = null)
new PresenceOf(
[
'message' => 'valid :field is required',
'cancelOnFail' => true,
]
),
];
Expand All @@ -76,7 +70,6 @@ public function initialize($entity = null, $options = null)
new PresenceOf(
[
'message' => 'your :field is required',
'cancelOnFail' => true,
]
),
];
Expand All @@ -100,7 +93,6 @@ public function initialize($entity = null, $options = null)
'messageMaximum' => "your message can't be longer than 300 characters",
'min' => 4,
'messageMinimum' => 'Tell us what we can do for you',
'cancelOnFail' => true,
]
),
];
Expand Down
30 changes: 29 additions & 1 deletion tests/integration/Forms/Form/IsValidCest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,35 @@ public function formsFormRemoveIsValidCancelOnFail(IntegrationTester $I)
$actual = $form->isValid($data);
$I->assertFalse($actual);

/**
* 6 validators in total
*/
$messages = $form->getMessages();
$I->assertCount(6, $messages);
$I->assertCount(4, $messages);

$data = [
'fullname' => '',
'email' => 'team@phalconphp.com',
'subject' => 'Some subject',
'message' => 'Some message',
];

$actual = $form->isValid($data);
$I->assertFalse($actual);

$messages = $form->getMessages();
$I->assertCount(1, $messages);

$expected = new Messages(
[
new Message(
'your fullname is required',
'fullname',
'PresenceOf'
)
]
);

$I->assertEquals($expected, $messages);
}
}
8 changes: 6 additions & 2 deletions tests/integration/Forms/FormElementsCest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,12 @@ public function shouldCancelValidationOnFirstFail(IntegrationTester $I)
new Message(
'user.lastName.presenceOf',
'lastName',
'PresenceOf',
0
'PresenceOf'
),
new Message(
'user.firstName.presenceOf',
'firstName',
'PresenceOf'
),
]
);
Expand Down