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

Use _.isNaN for integer required validation instead of _.isEmpty #410

Merged

Conversation

eclarizio
Copy link
Member

When determining if a field has a value filled out, when the data_type is integer, we're still attempting to do a _.isEmpty check, which will always fail for integers since they are not an enumerable type. Instead, we should use _.isNaN for integers. Alternatively we could refactor this into our own _.isBlank and use that instead, but for now this fixes the problem.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1740899

@miq-bot assign @h-kataria

@h-kataria @d-m-u Please Review since @himdel is on PTO.

@eclarizio eclarizio force-pushed the BZ1740899-fix-required-integer-value branch 2 times, most recently from 0fa6710 to 16fab43 Compare August 28, 2019 18:42
@d-m-u
Copy link
Contributor

d-m-u commented Aug 28, 2019

Cool, I think we need this to go back to hammer?

@eclarizio eclarizio force-pushed the BZ1740899-fix-required-integer-value branch from 16fab43 to f7ff2cb Compare August 28, 2019 18:47
@eclarizio
Copy link
Member Author

@miq-bot add_label hammer/yes

@d-m-u
Copy link
Contributor

d-m-u commented Aug 28, 2019

@miq-bot add_label ivanchuk/yes, bug

@miq-bot
Copy link
Member

miq-bot commented Aug 28, 2019

@d-m-u Cannot apply the following label because they are not recognized: dialog_user

@eclarizio eclarizio force-pushed the BZ1740899-fix-required-integer-value branch from f7ff2cb to 7977073 Compare August 28, 2019 20:06
@miq-bot
Copy link
Member

miq-bot commented Aug 28, 2019

Checked commit eclarizio@7977073 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Contributor

@h-kataria h-kataria left a comment

Choose a reason for hiding this comment

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

verified fix in UI.

@h-kataria h-kataria added this to the Sprint 119 Ending Sep 2, 2019 milestone Aug 28, 2019
@h-kataria h-kataria merged commit 485efda into ManageIQ:master Aug 28, 2019
simaishi pushed a commit that referenced this pull request Sep 3, 2019
…-value

Use _.isNaN for integer required validation instead of _.isEmpty

(cherry picked from commit 485efda)

https://bugzilla.redhat.com/show_bug.cgi?id=1748455
@simaishi
Copy link

simaishi commented Sep 3, 2019

Hammer backport details:

$ git log -1
commit 747827139bd7adb37b326ce45218ff00be1acc14
Author: Harpreet Kataria <[email protected]>
Date:   Wed Aug 28 16:23:54 2019 -0400

    Merge pull request #410 from eclarizio/BZ1740899-fix-required-integer-value
    
    Use _.isNaN for integer required validation instead of _.isEmpty
    
    (cherry picked from commit 485efda2323d23b349794ae1ad550c4d6cb2c58e)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1748455

simaishi pushed a commit that referenced this pull request Sep 3, 2019
…-value

Use _.isNaN for integer required validation instead of _.isEmpty

(cherry picked from commit 485efda)

https://bugzilla.redhat.com/show_bug.cgi?id=1740899
@simaishi
Copy link

simaishi commented Sep 3, 2019

Ivanchuk backport details:

$ git log -1
commit 62638231238ba9934ecc3fb792481a65abb7b11d
Author: Harpreet Kataria <[email protected]>
Date:   Wed Aug 28 16:23:54 2019 -0400

    Merge pull request #410 from eclarizio/BZ1740899-fix-required-integer-value
    
    Use _.isNaN for integer required validation instead of _.isEmpty
    
    (cherry picked from commit 485efda2323d23b349794ae1ad550c4d6cb2c58e)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1740899

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants