From 5cf3db6e0ee3a036e19b438e6af53a0a0a4d4122 Mon Sep 17 00:00:00 2001 From: mbrostami Date: Mon, 5 Feb 2018 13:44:52 +0330 Subject: [PATCH 1/7] fix broken getMessages by itemName --- phalcon/forms/form.zep | 12 +++++++- tests/unit/Forms/FormTest.php | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index 1101b78fa58..658397ad082 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -370,10 +370,20 @@ class Form extends Injectable implements \Countable, \Iterator */ public function getMessages(boolean byItemName = false) -> { - var messages; + var messages, messagesByItem, elementMessage; let messages = this->_messages; if typeof messages == "object" && messages instanceof Group { + /** + * This part of code is for backward compatibility, it should be removed in next major version + */ + if byItemName { + let messagesByItem = []; + for elementMessage in messages { + messagesByItem[elementMessages->getField()][] = new Group([elementMessage]); + } + return messagesByItem; + } return messages; } diff --git a/tests/unit/Forms/FormTest.php b/tests/unit/Forms/FormTest.php index 004d89fb061..c7de764bdb6 100644 --- a/tests/unit/Forms/FormTest.php +++ b/tests/unit/Forms/FormTest.php @@ -540,4 +540,60 @@ public function testMergeValidators() expect($form->get('address')->getMessages())->equals(Group::__set_state(['_messages' => []])); }); } + + /** + * Tests Form::getMessages(true) + * @author Mohamad Rostami + * @issue 13294 + * This should be removed in next major version + * We should not return multiple type of result in a single method! (form->getMessages(true) vs form->getMessages()) + */ + public function testGetElementMessagesFromForm() + { + $this->specify('When form is not valid, iterate over messages by elements is not possible', 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->getMessages(true))->equals([ + 'telephone' => [ + Group::__set_state([ + '_messages' => [ + Message::__set_state([ + '_type' => 'Regex', + '_message' => 'The telephone has an invalid format', + '_field' => 'telephone', + '_code' => 0, + ]) + ] + ]), + Group::__set_state([ + '_messages' => [ + Message::__set_state([ + '_type' => 'PresenceOf', + '_message' => 'The telephone is required', + '_field' => 'telephone', + '_code' => 0, + ]) + ] + ]) + ] + ]); + expect($form->get('telephone')->getMessages())->equals($form->getMessages(true)['telephone']); + }); + } } From 86e40aee7db91b12e965d74e7913c53020095476 Mon Sep 17 00:00:00 2001 From: mbrostami Date: Mon, 5 Feb 2018 13:56:09 +0330 Subject: [PATCH 2/7] fix syntax error --- phalcon/forms/form.zep | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index 658397ad082..b185f872d0a 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -380,6 +380,9 @@ class Form extends Injectable implements \Countable, \Iterator if byItemName { let messagesByItem = []; for elementMessage in messages { + if !isset messagesByItem[elementMessages->getField()] { + let messagesByItem[elementMessages->getField()] = []; + } messagesByItem[elementMessages->getField()][] = new Group([elementMessage]); } return messagesByItem; From b24c43c30ade1863b0de641cd75da53989e1923b Mon Sep 17 00:00:00 2001 From: mbrostami Date: Mon, 5 Feb 2018 16:14:49 +0330 Subject: [PATCH 3/7] fix syntax error --- phalcon/forms/form.zep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index b185f872d0a..d7ebc11f337 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -383,7 +383,7 @@ class Form extends Injectable implements \Countable, \Iterator if !isset messagesByItem[elementMessages->getField()] { let messagesByItem[elementMessages->getField()] = []; } - messagesByItem[elementMessages->getField()][] = new Group([elementMessage]); + let messagesByItem[elementMessages->getField()][] = new Group([elementMessage]); } return messagesByItem; } From 146a436da01290c68e2a7e63d5396bc57cec17a4 Mon Sep 17 00:00:00 2001 From: mbrostami Date: Tue, 6 Feb 2018 11:01:21 +0330 Subject: [PATCH 4/7] update changelog --- CHANGELOG-3.3.md | 1 + phalcon/forms/form.zep | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG-3.3.md b/CHANGELOG-3.3.md index 7e58756caad..d66bb48e97f 100644 --- a/CHANGELOG-3.3.md +++ b/CHANGELOG-3.3.md @@ -1,5 +1,6 @@ # [3.3.2](https://github.com/phalcon/cphalcon/releases/tag/v3.3.2) (2018-XX-XX) - Fixed `Phalcon\Db\Dialect\Mysql::modifyColumn` to produce valid SQL for renaming the column [#13012](https://github.com/phalcon/cphalcon/issues/13012) +- Fixed `Phalcon\Forms\Form::getMessages` to fix return array of messages with element name as key [#13294](https://github.com/phalcon/cphalcon/issues/13294) # [3.3.1](https://github.com/phalcon/cphalcon/releases/tag/v3.3.1) (2018-01-08) - Fixed a boolean logic error in the CSS minifier and a corresponding unit test so that whitespace is stripped [#13200](https://github.com/phalcon/cphalcon/pull/13200) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index d7ebc11f337..85333191db8 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -367,8 +367,10 @@ class Form extends Injectable implements \Countable, \Iterator /** * Returns the messages generated in the validation + * @param byItemName bool + * @example foreach($form->getMessages(true) as $elementName => $arrayOfGroupMessages) */ - public function getMessages(boolean byItemName = false) -> + public function getMessages(boolean byItemName = false) -> | array { var messages, messagesByItem, elementMessage; @@ -377,13 +379,13 @@ class Form extends Injectable implements \Countable, \Iterator /** * This part of code is for backward compatibility, it should be removed in next major version */ - if byItemName { + if unlikely byItemName { let messagesByItem = []; for elementMessage in messages { - if !isset messagesByItem[elementMessages->getField()] { - let messagesByItem[elementMessages->getField()] = []; + if !isset messagesByItem[elementMessage->getField()] { + let messagesByItem[elementMessage->getField()] = []; } - let messagesByItem[elementMessages->getField()][] = new Group([elementMessage]); + let messagesByItem[elementMessage->getField()][] = new Group([elementMessage]); } return messagesByItem; } From 82f6041da36bbfb4c96b6ee3b319601920d4112a Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Tue, 6 Feb 2018 10:00:17 +0200 Subject: [PATCH 5/7] Update form.zep --- phalcon/forms/form.zep | 62 ++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/phalcon/forms/form.zep b/phalcon/forms/form.zep index 85333191db8..58fae42bdd7 100644 --- a/phalcon/forms/form.zep +++ b/phalcon/forms/form.zep @@ -366,30 +366,56 @@ class Form extends Injectable implements \Countable, \Iterator } /** - * Returns the messages generated in the validation - * @param byItemName bool - * @example foreach($form->getMessages(true) as $elementName => $arrayOfGroupMessages) + * Returns the messages generated in the validation. + * + * + * if ($form->isValid($_POST) == false) { + * // Get messages separated by the item name + * // $messages is an array of Group object + * $messages = $form->getMessages(true); + * + * foreach ($messages as $message) { + * echo $message, "
"; + * } + * + * // Default behavior. + * // $messages is a Group object + * $messages = $form->getMessages(); + * + * foreach ($messages as $message) { + * echo $message, "
"; + * } + * } + * */ public function getMessages(boolean byItemName = false) -> | array { - var messages, messagesByItem, elementMessage; + var messages, messagesByItem, elementMessage, fieldName; let messages = this->_messages; + if typeof messages == "object" && messages instanceof Group { - /** - * This part of code is for backward compatibility, it should be removed in next major version - */ - if unlikely byItemName { - let messagesByItem = []; - for elementMessage in messages { - if !isset messagesByItem[elementMessage->getField()] { - let messagesByItem[elementMessage->getField()] = []; - } - let messagesByItem[elementMessage->getField()][] = new Group([elementMessage]); - } - return messagesByItem; - } - return messages; + /** + * @deprecated This part of code is for backward compatibility, it should be removed in next major version + */ + if unlikely byItemName { + let messagesByItem = []; + messages->rewind(); + + while messages->valid() { + let elementMessage = messages->current(), + fieldName = elementMessage->getField(); + + if !isset messagesByItem[fieldName] { + let messagesByItem[fieldName] = []; + } + + let messagesByItem[fieldName][] = new Group([elementMessage]); + messages->next(); + } + return messagesByItem; + } + return messages; } return new Group(); From fd8b72801aec1c8ee8ee63b791dae64bc64547e4 Mon Sep 17 00:00:00 2001 From: mbrostami Date: Tue, 6 Feb 2018 18:44:38 +0330 Subject: [PATCH 6/7] remove wrong test code --- tests/unit/Forms/FormTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/Forms/FormTest.php b/tests/unit/Forms/FormTest.php index c7de764bdb6..6b1df9497f3 100644 --- a/tests/unit/Forms/FormTest.php +++ b/tests/unit/Forms/FormTest.php @@ -593,7 +593,6 @@ public function testGetElementMessagesFromForm() ]) ] ]); - expect($form->get('telephone')->getMessages())->equals($form->getMessages(true)['telephone']); }); } } From 2470a2ff3e2bf1cd79d617bfcd102796721be332 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 7 Feb 2018 00:04:07 +0200 Subject: [PATCH 7/7] Update CHANELOG [ci skip] --- CHANGELOG-3.3.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG-3.3.md b/CHANGELOG-3.3.md index d66bb48e97f..e08eae743c3 100644 --- a/CHANGELOG-3.3.md +++ b/CHANGELOG-3.3.md @@ -1,6 +1,6 @@ # [3.3.2](https://github.com/phalcon/cphalcon/releases/tag/v3.3.2) (2018-XX-XX) - Fixed `Phalcon\Db\Dialect\Mysql::modifyColumn` to produce valid SQL for renaming the column [#13012](https://github.com/phalcon/cphalcon/issues/13012) -- Fixed `Phalcon\Forms\Form::getMessages` to fix return array of messages with element name as key [#13294](https://github.com/phalcon/cphalcon/issues/13294) +- Fixed `Phalcon\Forms\Form::getMessages` to return back previous behaviour: return array of messages with element name as key [#13294](https://github.com/phalcon/cphalcon/issues/13294) # [3.3.1](https://github.com/phalcon/cphalcon/releases/tag/v3.3.1) (2018-01-08) - Fixed a boolean logic error in the CSS minifier and a corresponding unit test so that whitespace is stripped [#13200](https://github.com/phalcon/cphalcon/pull/13200)