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

Allow returning back to the previous controller, after clicking on Back button and to cancel policy sim properly #5839

Merged
merged 4 commits into from
Jul 25, 2019

Conversation

hstastna
Copy link

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

When clicking on quadion of a VM or Template while running policy simulation, the actual controller was changed to VmOrTemplate. After clicking on Back button, it remained the same. This was causing unexpected behavior later in the next steps, many variables not set properly, not able to cancel policy sim properly... To prevent this controller's 'change' #5580 was created.

The solution worked well except the case that user chose some items from the list which were both VMs AND Templates, for policy sim, in Compute > Infra > Virtual Machines > VMs & Templates accordion. The problem was missing quadicon(s) of Template in policy simulation.

So little bit different approach is good to implement: going 'back' to the previous controller after clicking on Back button (instead of the same controller all the time). So I specified the controller name for getting proper url for onclick for appropriate Back button. By using model_for_vm method we get a proper model of the @record, and with controller_for_vm we get a proper controller name.


Before:
polsim_before

After:
polsim_after

@hstastna
Copy link
Author

@miq-bot add_label hammer/yes, bug

@skateman
Copy link
Member

@miq-bot add_reviewer @skateman
Long story short, we're jumping from one controller to the another in one direction of the navigation workflow. However, the way back doesn't switch the controller, so we have to alter the URL to get back to the right place.

@miq-bot miq-bot requested a review from skateman July 22, 2019 08:37
@hstastna hstastna changed the title [WIP] Allow returning back to the previous controller, after clicking on Back button and to cancel policy sim properly Allow returning back to the previous controller, after clicking on Back button and to cancel policy sim properly Jul 22, 2019
@miq-bot miq-bot removed the wip label Jul 22, 2019
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

@miq-bot
Copy link
Member

miq-bot commented Jul 25, 2019

Checked commits hstastna/manageiq-ui-classic@331b320~...d4de8de with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🏆

@skateman
Copy link
Member

@miq-bot add_label ivanchuk/yes
@miq-bot assign @mzazrivec

@miq-bot miq-bot assigned mzazrivec and unassigned skateman Jul 25, 2019
@mzazrivec mzazrivec modified the milestone: Sprint 117 Ending Aug 5, 2019 Jul 25, 2019
@mzazrivec mzazrivec merged commit aa7b10b into ManageIQ:master Jul 25, 2019
simaishi pushed a commit that referenced this pull request Jul 25, 2019
Allow returning back to the previous controller, after clicking on Back button and to cancel policy sim properly

(cherry picked from commit aa7b10b)

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

Ivanchuk backport details:

$ git log -1
commit 25ec06b557c5424574c123ec800dd5c22b18912b
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Thu Jul 25 12:39:48 2019 +0200

    Merge pull request #5839 from hstastna/Fix_cancel_policy_sim
    
    Allow returning back to the previous controller, after clicking on Back button and to cancel policy sim properly
    
    (cherry picked from commit aa7b10b8eb3cc3213a8dc591ad3883fe31cd0931)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1717495

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