Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fixes #6647 : Zend\I18n\Validator\Float set error message for NOT_FLOAT #6648

Closed
wants to merge 2 commits into from
Closed

Fixes #6647 : Zend\I18n\Validator\Float set error message for NOT_FLOAT #6648

wants to merge 2 commits into from

Conversation

samsonasik
Copy link
Contributor

Fixes #6647

@samsonasik samsonasik mentioned this pull request Sep 8, 2014
Closed
moderndeveloperllc
moderndeveloperllc reviewed Sep 8, 2014
View reviewed changes
tests/ZendTest/I18n/Validator/FloatTest.php
return array(
array(
'hello',
2.000,000
Copy link
Contributor

Choose a reason for hiding this comment

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

I would take out this second one, or specify that it's en-US locale. This is a valid float in many locales. Thanks for catching that I didn't set the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@moderndeveloperllc so the validator itself should be return true for 2.000,000 ? /cc @DASPRiD

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to have this test at all. We already have testValidationFailures on line 151 which covers the conditional. Adding the $this->error(self::NOT_FLOAT); in those two places is fine, but we don't need to touch the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@moderndeveloperllc Ok, I removed the additional tests, thank you.

@samsonasik
Copy link
Contributor Author

@moderndeveloperllc Ok, I removed the additional tests, thank you.

@samsonasik samsonasik force-pushed the patch-float-nonfloatmessage branch from 09c2d3d to be9e13b Compare September 22, 2014 18:51
This was referenced Sep 23, 2014
Closed
Closed
Closed
@samsonasik samsonasik changed the title Fixes #6647 Fixes #6647 : Zend\I18n\Validator\Float set error message for NOT_FLOAT Sep 27, 2014
@samsonasik samsonasik force-pushed the patch-float-nonfloatmessage branch from be9e13b to 0def83f Compare October 11, 2014 12:15
@coolmic
Copy link
Contributor

coolmic commented Oct 15, 2014

Still not merged?
It's not critical, but still an annoying bug.

@samsonasik
Copy link
Contributor Author

still waiting for merge...

@samsonasik samsonasik force-pushed the patch-float-nonfloatmessage branch from 0def83f to 1c6d909 Compare October 18, 2014 03:31
@Ocramius Ocramius mentioned this pull request Oct 19, 2014
Closed
@gkralik gkralik mentioned this pull request Nov 4, 2014
Closed
@Ocramius Ocramius self-assigned this Nov 22, 2014
@Ocramius
Copy link
Member

@samsonasik may I ask you to add a test here? This has been changed various times, and the bug came up as a regression AFAIK.

@Ocramius Ocramius added this to the 2.3.4 milestone Nov 22, 2014
@samsonasik
Copy link
Contributor Author

@Ocramius, @moderndeveloperllc said it doesnt' need new test. I can re-add this test samsonasik@dfd48e1 if needed...

@Ocramius
Copy link
Member

@samsonasik well, I'd need a test that fails if this PR is reverted :-)

@samsonasik samsonasik force-pushed the patch-float-nonfloatmessage branch from 1c6d909 to b23b547 Compare November 22, 2014 15:47
@samsonasik
Copy link
Contributor Author

@Ocramius done ;). I've added test case to fit catch self::NOT_FLOAT in that condition.

Ocramius added a commit that referenced this pull request Nov 27, 2014
@Ocramius
#6647 #6648 - minor CS fixes (spacing)
8e13b2f
Ocramius added a commit that referenced this pull request Nov 27, 2014
@Ocramius
#6647 #6648 - adding @group annotation referencing the open pull re…
d33827e 
…quest
@Ocramius Ocramius closed this in d714eca Nov 27, 2014
Ocramius added a commit that referenced this pull request Nov 27, 2014
@Ocramius
Merge branch 'hotfix/#6648-#6647-zend-i18n-validator-float-not-float-…
cd7d2a0 
…error-message' into develop

Close #6648
Close #6647
Forward port #6648
Forward port #6647
@Ocramius
Copy link
Member

@samsonasik merged, thanks!

master: d714eca
develop: cd7d2a0

gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@Ocramius
zendframework/zendframework#6647 zendframework/zendframework#6648 - m…
59254dc 
…inor CS fixes (spacing)
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@Ocramius
zendframework/zendframework#6647 zendframework/zendframework#6648 - a…
6eee880 
…dding `@group` annotation referencing the open pull request
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@Ocramius
Merge branch 'hotfix/zendframework/zendframework#6648-zendframework/z…
1f6c691 
…endframework#6647-zend-i18n-validator-float-not-float-error-message'

Close zendframework/zendframework#6648
Close zendframework/zendframework#6647
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@Ocramius
Merge branch 'hotfix/zendframework/zendframework#6648-zendframework/z…
5ec914f 
…endframework#6647-zend-i18n-validator-float-not-float-error-message' into develop

Close zendframework/zendframework#6648
Close zendframework/zendframework#6647
Forward port zendframework/zendframework#6648
Forward port zendframework/zendframework#6647
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@Ocramius Ocramius

Labels
bug I18n Validator
Projects
None yet
Milestone
2.3.4
Development

Successfully merging this pull request may close these issues.

Zend\I18n\Validator\Float does not set error message on NOT_FLOAT.
4 participants
@samsonasik @coolmic @Ocramius @moderndeveloperllc