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

Fix broken form getMessages #13295

Merged
merged 7 commits into from
Feb 6, 2018

Conversation

mbrostami
Copy link
Contributor

Hello!

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

Thanks

@mbrostami
Copy link
Contributor Author

Will check it out soon.

@sergeyklay
Copy link
Contributor

@mbrostami Could you please update change log too.

@sergeyklay sergeyklay added this to the 3.3.x milestone Feb 5, 2018
@@ -370,10 +370,23 @@ class Form extends Injectable implements \Countable, \Iterator
*/
public function getMessages(boolean byItemName = false) -> <Group>
Copy link
Contributor

@sergeyklay sergeyklay Feb 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public function getMessages(boolean byItemName = false) -> <Group> | array

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please amend PHPDoc by adding examples from the #13294 issue

/**
* This part of code is for backward compatibility, it should be removed in next major version
*/
if byItemName {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if unlikely byItemName

For more see: https://stackoverflow.com/a/11227902/1661465

if byItemName {
let messagesByItem = [];
for elementMessage in messages {
if !isset messagesByItem[elementMessages->getField()] {
Copy link
Contributor

@sergeyklay sergeyklay Feb 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems to me if fetch will be faster

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyway here is typo elementMessage not elementMessages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find the way to use if fetch, refer to this link https://docs.zephir-lang.com/en/latest/operators.html#fetch I thought that isset is faster than fetch, please let me know if I'm wrong.

@mbrostami
Copy link
Contributor Author

@sergeyklay Done.

@sergeyklay sergeyklay merged commit 4f193ad into phalcon:3.3.x Feb 6, 2018
@sergeyklay
Copy link
Contributor

@mbrostami Thank you for quick reply and fixes

@mbrostami mbrostami deleted the 3.3.x-fix-broken-formmessages branch February 7, 2018 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants