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

Fix Evacuate form #1676

Merged
merged 3 commits into from Jul 19, 2017
Merged

Fix Evacuate form #1676

merged 3 commits into from Jul 19, 2017

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Jul 12, 2017

Where:
Compute -> Clouds -> Instances -> click OpenStack one -> Lifecycle -> Evacuate Instance
Compute -> Clouds -> Instances -> check OpenStack one or more -> Lifecycle -> Evacuate Instance
Compute -> Clouds -> Providers -> click OpenStack one -> click Instances-> click one -> Lifecycle -> Evacuate Instance
Compute -> Clouds -> Providers -> click OpenStack one -> click Instances-> check one or more -> Lifecycle -> Evacuate Instance

Before:
Various error message on screen or in log. Like undefined method 'length' for nil:NilClass [vm/evacuate] or Error caught: [RuntimeError] Can't access selected records. Never displaying Evacuate form.

After:
It works :) With GTL issue that's not related to this PR. User can get to form for Evacuate and can save or cancel it.

Blockes #1233

@miq-bot add_label bug

@romanblanco please have a look as it's related to RBAC

@miq-bot miq-bot added the bug label Jul 12, 2017
@romanblanco
Copy link
Member

@miq-bot assign @romanblanco

Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

@ZitaNemeckova It works better, but I'm still getting Javascript error message:

Error: [$controller:ctrlreg] The controller with the name 'reportDataController' is not registered.
http://errors.angularjs.org/1.6.5/$controller/ctrlreg?p0=reportDataController

screencast from 2017-07-12 17-41-51

@himdel
Copy link
Contributor

himdel commented Jul 12, 2017

@romanblanco The reportDataController problem is unrelated, that happen sometimes on every screen with a sub-GTL, @karelhala is aware of it..

But I'm more concerned with what the screengrab seems to be saying...

You are on an instance detail screen, click evacuate, and suddenly the list contains 6 instances. That is definitely not desired ;)

@karelhala
Copy link
Contributor

The controller with the name 'reportDataController' is not registered. happens because you register gtl controller too late. Can you @ZitaNemeckova add = render :partial => "layouts/gtl" to the bottom of page above :javascript? This should fix the issue. However another issue which @himdel mentioned seemes kinda fishy. I will have to investigate further to see what causes this problem.

@ZitaNemeckova
Copy link
Contributor Author

@karelhala Thanks it really helped :)

Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

@karelhala @ZitaNemeckova Moving render before :javascript didn't help, I'm still seeing the error, but I checked with @himdel and the issue is unrelated to the fix in this PR.

Changes in PR are fixing the original error correctly 👍

@miq-bot
Copy link
Member

miq-bot commented Jul 19, 2017

Checked commits ZitaNemeckova/manageiq-ui-classic@af3e468~...28373ff with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@himdel
Copy link
Contributor

himdel commented Jul 19, 2017

Tested in the UI, works in both the explorer view and non-explorer, for one or multiple instances.

There are still multiple GTL issues on that screen - created #1715 - but evacuating works 👍

@himdel himdel merged commit e4f4b93 into ManageIQ:master Jul 19, 2017
@himdel himdel added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 19, 2017
@himdel himdel assigned himdel and unassigned romanblanco Jul 19, 2017
@ZitaNemeckova ZitaNemeckova deleted the fix_evacuate branch September 12, 2017 14:19
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.

None yet

5 participants