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 miqObserveRequest to force dialog submit to wait for auto refresh #2397

Merged
merged 1 commit into from
Oct 16, 2017

Conversation

eclarizio
Copy link
Member

There was a small race condition before where while ordering a dialog, if the user had typed in something into a text box without tabbing away or clicking elsewhere, they could click the submit button and potentially submit the form before any auto refreshes were actually done processing.

This fix simply adds the auto refreshes to the observe queue which forces the submit to wait until they are finished.

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

/cc @gmcculloug

@miq-bot add_label bug, euwe/yes, fine/yes
@miq-bot assign @h-kataria

@himdel We talked about this a bit throwing around ideas, but...ends up the simplest idea worked the best, haha!
@AparnaKarve Think you can review? Would be nice to get this in asap so we can get a hotfix for the customer and I think @himdel is gone for the day.
@h-kataria Can you merge when everything is good to go?

@miq-bot
Copy link
Member

miq-bot commented Oct 13, 2017

Checked commit eclarizio@c2701cc with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@eclarizio
Copy link
Member Author

Spec failures seem unrelated, can someone restart it?

@AparnaKarve
Copy link
Contributor

AparnaKarve commented Oct 13, 2017

@eclarizio Using miqObserveRequest seems reasonable for the scenario you have mentioned above.

I just have one question - because of this change, do you foresee a situation where the autorefresh may take way too much time than actually necessary?

EDIT: Or maybe it does not matter, because the user has already submitted the form?

@eclarizio
Copy link
Member Author

It shouldn't take any longer than it did before this change, so that's not a worry. When we move to the new triggering of auto refreshes instead, this will be obsolete anyway as well.

@AparnaKarve
Copy link
Contributor

Ok, that sounds good. Thanks.

@h-kataria IMO, GTG.

When we move to the new triggering of auto refreshes instead,

As in, a refactored auto refresh?

@eclarizio
Copy link
Member Author

@AparnaKarve Yeah, sort of. Basically instead of using the auto refresh/trigger auto refresh checkboxes, users will be able to select specific fields they would like that field to refresh, and this will happen simultaneously instead of right now which is just like a cascading one after another.

@h-kataria h-kataria added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 16, 2017
@h-kataria h-kataria merged commit 8dd216a into ManageIQ:master Oct 16, 2017
simaishi pushed a commit that referenced this pull request Oct 16, 2017
Use miqObserveRequest to force dialog submit to wait for auto refresh
(cherry picked from commit 8dd216a)

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

Fine backport details:

$ git log -1
commit 35beb5e894ab2232b24b1e3083f26b23a827ba10
Author: Harpreet Kataria <[email protected]>
Date:   Mon Oct 16 10:25:19 2017 -0400

    Merge pull request #2397 from eclarizio/BZ1499589
    
    Use miqObserveRequest to force dialog submit to wait for auto refresh
    (cherry picked from commit 8dd216a26aec3117bfc6e6f18f0bd0dab48573b7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1502738

@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit d54693403f7d55cda97ad31ff88d4629b72439fd
Author: Harpreet Kataria <[email protected]>
Date:   Mon Oct 16 10:25:19 2017 -0400

    Merge pull request #2397 from eclarizio/BZ1499589
    
    Use miqObserveRequest to force dialog submit to wait for auto refresh
    (cherry picked from commit 8dd216a26aec3117bfc6e6f18f0bd0dab48573b7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1502739

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