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

[WIP] Structure for Physical Server Pages #755

Closed
wants to merge 34 commits into from

Conversation

odravison
Copy link

@odravison odravison commented Mar 21, 2017

This commit is only about Physical Server and some changes in Physical Infra as well.
Physical Server:

  • Toolbar
  • Routes
  • Helpers
  • Assets (used on quadicon)
  • Ui Constants
  • Textual Summary
  • Some flags to recognize Physical Server
  • Views (_show, _form, ...)
  • Controller

Physical Infrastructure:

  • Change the view of Physical Infrastructure into grid

Depends on: ManageIQ/manageiq#14026

755-pr

Odravison Amaral added 13 commits March 20, 2017 20:52
1. Adding routes to physical_server in routes.rb file
1. Adding physical_server flag into ems_common/_show.html.haml file.
1. Adding PhysicalServer structure to ui_constants.rb file.
1. Adding PhysicalServer structure to quadicon_helper.rb file to render properly physical_server's quadicon.
1. Adding physical_server flag into host textual summary.
1. Adding PhysicalServer structure into textual_summary of ems_physical_infra.
Adding physical server flags into three files:
1. app/helpers/application_helper/toolbar_chooser.rb
2. app/helpers/application_helper.rb
3, app/controllers/ems_common.rb
This last one has view_setup_param structure with only physical_server.
Add all helpers and list_nav to physical server.
Adapt physical infra helpers to show physical servers.
Add assets to physical server health state.
@rodneyhbrown7
Copy link

@miq-bot add_label wip

@miq-bot miq-bot changed the title Structure for Physical Server Pages [WIP] Structure for Physical Server Pages Mar 21, 2017
@miq-bot miq-bot added the wip label Mar 21, 2017
@rodneyhbrown7
Copy link

@miq-bot assign @AparnaKarve

@walteraa
Copy link
Member

@odravison is fixing the style warnings.

@odravison odravison force-pushed the physical_server_page branch from 2590202 to 0a3a4a3 Compare March 22, 2017 15:57
@odravison
Copy link
Author

odravison commented Mar 27, 2017

@AparnaKarve see attached screen shots you asked for above.

I have already attached into PR's description @abbondanzio

@odravison odravison closed this Mar 28, 2017
@odravison
Copy link
Author

Reopen to rebuild

@odravison odravison reopened this Mar 28, 2017
@AparnaKarve
Copy link
Contributor

@odravison The Physical Server menu is still not visible

It looks like you would have to create a PR in core MIQ that adds this feature physical_server_show_list in the db/fixtures/miq_product_features.yml and db/fixtures/miq_user_roles.yml

@odravison
Copy link
Author

@odravison The Physical Server menu is still not visible
It looks like you would have to create a PR in core MIQ that adds this feature physical_server_show_list in the db/fixtures/miq_product_features.yml and db/fixtures/miq_user_roles.yml

Ok, @AparnaKarve I'll do that ASAP
First, I need to solve the issues that @martinpovolny asked
@walteraa If possible, can you do this? If no, NP

@skovic
Copy link

skovic commented Mar 28, 2017

@walteraa @odravison @AparnaKarve Actually, the change is already in master, but it is misspelled: "shosical_server_show_list" instead of "physical_server_show_list"

@AparnaKarve
Copy link
Contributor

@skovic Ok, we already have an entry for physical_server_show_list except that there is a typo there.
Created ManageIQ/manageiq#14545 to fix it.

output << flobj_img_simple("layout/base.png")

output << flobj_p_simple("a72", (item.host ? 1 : 0))
output << flobj_img_simple("svg/currentstate-#{h(item.power_state.downcase)}.svg", "b72")
Copy link
Contributor

Choose a reason for hiding this comment

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

item. power_state is nil in my case.
Getting this --

Error caught: [ActionView::Template::Error] undefined method `downcase' for nil:NilClass

Is there a core MIQ PR to address power_state?

Copy link
Member

Choose a reason for hiding this comment

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

You need execute a migration in core, @AparnaKarve. There are some migration that change the properties in physical server(e.g. powerState to power_state).

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm up-to-date on my migrations and yet, I'm seeing the above error.

Is there any other reason why I continue to see the error?

Copy link
Author

Choose a reason for hiding this comment

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

However, I read about a concern that @Fryguy had. ManageIQ/manageiq#14287

@juliancheal sent this PR and was accepted but I don't have sure if this PR override power_state or not. According with this PR he jut added a migration with a new property called raw_power_state

Copy link
Author

Choose a reason for hiding this comment

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

There so many PR to track and push to manageiq that I could have missed this one.
I'll find and send a PR to miq-core

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Waiting for ManageIQ/manageiq#14488 to be merged to resolve
Error caught: [ActionView::Template::Error] undefined local variable or method `vendor' for #<ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalServer:0x007fb965178178>

Copy link
Contributor

Choose a reason for hiding this comment

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

Would also need a migration to add uuid to physical_servers

Choose a reason for hiding this comment

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

uuid was removed from the physical_servers table. This value is avaiable as the ems_ref column

@odravison
Copy link
Author

Hey, everybody.
Seems that Travis build is falling because a internal error.
Same occur with Hakiri tests.

@AparnaKarve @martinpovolny

@AparnaKarve
Copy link
Contributor

@odravison You need to rebase and repush to address the Hakiri error.

'pficon pficon-edit fa-lg',
N_('Manage Policies for this item'),
N_('Manage Policies'),
:klass => ApplicationHelper::Button::HostProtect
Copy link
Contributor

Choose a reason for hiding this comment

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

@odravison Like we discussed the other day, :klass => ApplicationHelper::Button::HostProtect is not applicable here, and seems like the reason why the Physical Server summary page cannot be rendered.

screen shot 2017-03-31 at 2 17 59 pm

Copy link
Author

Choose a reason for hiding this comment

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

Ok, @AparnaKarve .
This came from a copy and paste to a fastest development.
We'll discuss here about this issue, PhysicalServer can have a policy.
Probably we'll implement this for the next PR.
cc @walteraa @rodneyhbrown7 @skovic

@odravison
Copy link
Author

@odravison You need to rebase and repush to address the Hakiri error.

Ok @AparnaKarve

Odravison Amaral added 3 commits April 3, 2017 17:14
1. Adding i18n syntax;
2. Removing all 'display_message_...' methods;
3. Using 'render_flash' method from 'application_controller';
4. Using Rbac method to find objects;
@miq-bot
Copy link
Member

miq-bot commented Apr 5, 2017

Checked commits odravison/manageiq-ui-classic@ca99b62~...21c2cd9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
14 files checked, 15 offenses detected

app/controllers/physical_server_controller.rb

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.

10 participants