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

[V2V] Fix allowing address to be blank for the ConversionHost model, and update spec. #18690

Merged
merged 1 commit into from
Apr 26, 2019
Merged

[V2V] Fix allowing address to be blank for the ConversionHost model, and update spec. #18690

merged 1 commit into from
Apr 26, 2019

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Apr 25, 2019

Apparently adding :presence => false to the address validation for the ConversionHost model wasn't good enough due to some AR precedence issue, and my spec didn't catch it because it already skipped the validation if the underlying resource didn't return an address.

So, this explicitly skips the check if the address is blank, and updates the spec to make sure that it will return at least one address.

I validated that without this change, the spec will now fail.

Followup to: #18610
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1695797

@djberg96
Copy link
Contributor Author

@miq-bot add_label hammer/yes

@djberg96
Copy link
Contributor Author

@miq-bot add_label bug

@djberg96 djberg96 changed the title Fix allowing address to be blank for the ConversionHost model, and update spec. [V2V] Fix allowing address to be blank for the ConversionHost model, and update spec. Apr 25, 2019
@djberg96
Copy link
Contributor Author

@miq-bot add_label transformation

@miq-bot miq-bot added the v2v label Apr 25, 2019
@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2019

@djberg96 unrecognized command 'assign_reviewer', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, remove_reviewer, set_milestone

@djberg96
Copy link
Contributor Author

@miq-bot add_reviewer @agrare

@miq-bot miq-bot requested a review from agrare April 25, 2019 15:44
@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2019

Checked commit https://github.com/djberg96/manageiq/commit/d89d83a4a414b08b0f0e19f47efe56ead0020c47 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@agrare agrare merged commit afe8714 into ManageIQ:master Apr 26, 2019
@agrare agrare added this to the Sprint 110 Ending Apr 29, 2019 milestone Apr 26, 2019
simaishi pushed a commit that referenced this pull request Apr 26, 2019
…alidation

[V2V] Fix allowing address to be blank for the ConversionHost model, and update spec.

(cherry picked from commit afe8714)

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

Hammer backport details:

$ git log -1
commit 716845cb5a0b57fe9ea29c80f8083657a2a892c9
Author: Adam Grare <[email protected]>
Date:   Fri Apr 26 08:00:14 2019 -0400

    Merge pull request #18690 from djberg96/fix_conversion_host_address_validation
    
    [V2V] Fix allowing address to be blank for the ConversionHost model, and update spec.
    
    (cherry picked from commit afe8714e54164b619dbd3b8436f9e67b73f3a06f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1696437

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.

4 participants