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

Removes last call to automate from dialog_field serializer #17436

Merged
merged 1 commit into from
May 21, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented May 17, 2018

The field serializer shouldn't be calling automate, so any call to values needs to be removed. I think this was the last one. We've had multiple issues with automate running more times than it needs to and this is at least partially a fix for those type of issues.

Should fix #17446

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

@d-m-u
Copy link
Contributor Author

d-m-u commented May 17, 2018

@miq-bot add_label bug
@miq-bot assign @gmcculloug
@eclarizio can you 👀 for me please?
@tinaafitz can you also 👀 for me please?
@miq-bot add_label all_the_bugs_are_scared_of_brandon

@miq-bot miq-bot added the bug label May 17, 2018
@d-m-u d-m-u force-pushed the excluding_values_from_serializer branch from fef9113 to 7aded49 Compare May 17, 2018 22:15
Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

Needs a spec but otherwise 👍

As for how to test, maybe stub dialog_field.values when the field is dynamic and ensure that it isn't called.

@d-m-u d-m-u force-pushed the excluding_values_from_serializer branch from 7aded49 to 5415cc1 Compare May 21, 2018 17:36
@miq-bot
Copy link
Member

miq-bot commented May 21, 2018

Checked commit d-m-u@5415cc1 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. 👍

@gmcculloug
Copy link
Member

@eclarizio Specs added. Please take another look

Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

Cool, looks good. Do you think we will need this in gaprindashvili to complement the changes made in #17329 ?

@d-m-u
Copy link
Contributor Author

d-m-u commented May 21, 2018

@eclarizio yeah, i believe so

@jrafanie
Copy link
Member

This fixed my issue. Once @eclarizio signs off on the specs, I'm good to go. Thanks @d-m-u 👏

@gmcculloug
Copy link
Member

@eclarizio Let's associate this PR with BZ https://bugzilla.redhat.com/show_bug.cgi?id=1570298.

@gmcculloug gmcculloug merged commit 1009681 into ManageIQ:master May 21, 2018
@gmcculloug gmcculloug added this to the Sprint 86 Ending May 21, 2018 milestone May 21, 2018
@miq-bot
Copy link
Member

miq-bot commented May 21, 2018

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

simaishi pushed a commit that referenced this pull request Jun 11, 2018
Removes last call to automate from dialog_field serializer
(cherry picked from commit 1009681)

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

Gaprindashvili backport details:

$ git log -1
commit f984d7be552833dc5f03f5c5fad4982ca0a49626
Author: Greg McCullough <[email protected]>
Date:   Mon May 21 16:03:10 2018 -0400

    Merge pull request #17436 from d-m-u/excluding_values_from_serializer
    
    Removes last call to automate from dialog_field serializer
    (cherry picked from commit 1009681373bed6951f4e5bcc6a9daa9e50dcfcfd)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1589837

@d-m-u d-m-u deleted the excluding_values_from_serializer branch October 26, 2018 18:13
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.

6 participants