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

Fixes ProvisionWorkflow#available_vlans_and_hosts #269

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented May 19, 2018

When the super form of this method returns nil for hosts, the .map(&:id) call would no work properly.

In previous forms, hosts had to_miq_a, which will effectively run an Array.wrap around the object. I figured moving away from specialty code for this was a better option moving forward, so I skipped that an opted for an unless check instead.

The tests added in this PR mimic what is done in manageiq-ui-classic, which was the original "canary in the coal mine" that exposed this error.

Links

When the `super` form of this method returns `nil` for hosts, the
`.map(&:id)` call would no work properly.

In previous forms, `hosts` had `to_miq_a`, which will effectively run an
`Array.wrap` around the object.  I figured moving away from specialty
code for this was a better option moving forward, so I skipped that an
opted for an `unless` check instead.
@NickLaMuro
Copy link
Member Author

@miq-bot assign @agrare
@miq-bot add_label bug

@miq-bot
Copy link
Member

miq-bot commented May 19, 2018

Checked commit NickLaMuro@c0a5a9e 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. 🍰

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM

@agrare agrare merged commit 5b7c527 into ManageIQ:master May 19, 2018
@agrare
Copy link
Member

agrare commented May 19, 2018

Thanks @NickLaMuro

@NickLaMuro
Copy link
Member Author

Sorry for breaking it in the first place 😢

@agrare agrare added this to the Sprint 86 Ending May 21, 2018 milestone May 19, 2018
@NickLaMuro
Copy link
Member Author

@miq-bot add_label fine/yes, gaprindashvili/yes

Backport this after #249 .

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

Successfully merging this pull request may close these issues.

3 participants