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

Check that reconfigure is supported before we try it #18636

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Apr 8, 2019

I would expect GET /api/services/:id?attributes=reconfigure_dialog to check that the reconfigure is valid before running it rather than getting an internal_server_error from undefined method `dialog_tabs' for nil:NilClass because the reconfigure was called on a dialog without the endpoint set up (or without fields set to be reconfigureable). We have the supports_feature mixin on the reconfigure so we should use it.

" services shouldn't behave different than anything else with virtual associations", the man says.

Fix for https://bugzilla.redhat.com/show_bug.cgi?id=1663972

@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 8, 2019

@miq-bot assign @jrafanie

@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 8, 2019

@miq-bot add_label bug

@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 8, 2019

@miq-bot add_label hammer/yes

@d-m-u d-m-u changed the title Check that reconfigure is supported before we try it [WIP] Check that reconfigure is supported before we try it Apr 8, 2019
@miq-bot miq-bot added the wip label Apr 8, 2019
@d-m-u d-m-u force-pushed the catch_attribute_error_better branch 2 times, most recently from df0bfb1 to 682bc05 Compare April 8, 2019 17:02
app/models/service.rb Outdated Show resolved Hide resolved
@d-m-u d-m-u force-pushed the catch_attribute_error_better branch from 682bc05 to 441ad79 Compare April 9, 2019 13:44
@miq-bot
Copy link
Member

miq-bot commented Apr 9, 2019

Checked commit d-m-u@441ad79 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. ⭐

@d-m-u d-m-u changed the title [WIP] Check that reconfigure is supported before we try it Check that reconfigure is supported before we try it Apr 9, 2019
@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 9, 2019

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Apr 9, 2019
@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 11, 2019

can i get anyone to look at this please?

@abellotti
Copy link
Member

This fix works perfectly and keeps the API happy. Thanks @d-m-u for taking care of this. 👍

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

:shipit:

@jrafanie jrafanie merged commit dd686c8 into ManageIQ:master Apr 11, 2019
@jrafanie jrafanie added this to the Sprint 109 Ending Apr 15, 2019 milestone Apr 11, 2019
@d-m-u d-m-u deleted the catch_attribute_error_better branch April 11, 2019 14:28
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.

5 participants