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

Add Back button to policy simulation page for non-explorer #6099

Merged

Conversation

hstastna
Copy link

@hstastna hstastna commented Aug 26, 2019

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

Back button was missing in policy simulation screen while checking details of selected item. This is related only to non-explorer screens, for example for list of VMs displayed thru provider's details page. For explorer ones, the button was there (see this).

I am also adding another change session[:edit] = @edit. First, @edit with details of policy simulation is set in policy_sim_build_screen method and then forgotten, for non-explorer screens. This caused problems with rendering the selected item's info later, also with clicking on a selected item (quadicon). session[:edit] = @edit needs to be set after policy_sim_build_screen is called.

The fix works also for nested list of Instances.


After:
back-after

@hstastna
Copy link
Author

@miq-bot add_label bug, ivanchuk/yes, changelog/yes

@hstastna hstastna force-pushed the No_Back_btn_policy_sim_nested_VMs branch from c76b7ed to 663761c Compare August 26, 2019 17:24
@miq-bot
Copy link
Member

miq-bot commented Aug 26, 2019

Checked commit hstastna@663761c with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@skateman
Copy link
Member

@hstastna is this a regression? Because I think this thing used to work some time ago and we might have to look into what caused the problem than adding a new button. cc @h-kataria

@hstastna
Copy link
Author

hstastna commented Aug 27, 2019

I think this is not a regression but I may be wrong. I will try to check this in older versions.

@hstastna
Copy link
Author

hstastna commented Aug 27, 2019

So I've tested in 5.10 and 5.9 appliance and in both cases, this was the result (after clicking on quadicon):
policy_sim5 10
It looks like this haven't worked at all, in the previous versions. But let's wait for the reporter of the bug and his appliances.

@hstastna
Copy link
Author

hstastna commented Sep 2, 2019

Update: reporter of the bug cannot test if this is a regression due to https://bugzilla.redhat.com/show_bug.cgi?id=1686619 in 5.9 and also 5.10.

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@h-kataria h-kataria self-assigned this Sep 5, 2019
@h-kataria h-kataria added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 5, 2019
@h-kataria h-kataria merged commit fc2600c into ManageIQ:master Sep 5, 2019
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