Physical server routing #1162

gabrielsvinha · 2017-04-26T14:56:13Z

This Pull Request adds the specific routes and controller.

physical_server/show_list
physical_server/show
physical_server/protect
physical_server/download_data
physical_server/perf_top_chart

Obs: This Pull Request fixes the physical_server.png error (#1252)

gabrielsvinha · 2017-04-26T18:30:43Z

AparnaKarve · 2017-04-27T18:28:18Z

app/controllers/physical_server_controller.rb

+             "physical_server_blink_loc_led"    => [:blink_loc_led,    "Blink LED"],
+             "physical_server_turn_on_loc_led"  => [:turn_on_loc_led,  "Turn On LED"],
+             "physical_server_turn_off_loc_led" => [:turn_off_loc_led, "Turn Off LED"]}.freeze
+


@gabrielsvinha Since this is a PR that just shows us the basic show_list view with a basic toolbar, can you remove the ACTIONS hash and all related code around it?

All you really need for this first pass is the show_list method.
Please remove all the other unused methods that are never called.

rodneyhbrown7 · 2017-05-02T14:10:59Z

@miq-bot assign @AparnaKarve

AparnaKarve · 2017-05-02T23:10:21Z

@gabrielsvinha I'm thinking if we should also include show in this PR.
Could you point me to the PR that implements the show method?

Since show is currently not implemented in this PR, I get --

[----] F, [2017-05-02T16:04:04.108307 #77531:3fd8bd1043cc] FATAL -- : Error caught: [ActionController::UnknownFormat] PhysicalServerController#show is missing a template for this request format and variant.

request.formats: ["text/html"]
request.variant: []

NOTE! For XHR/Ajax or API requests, this action would normally respond with 204 No Content: an empty white screen. Since you're loading it in a web browser, we assume that you expected to actually render a template, not nothing, so we're showing an error to be extra-clear. If you expect 204 No Content, carry on. That's what you'll get from an XHR or API request. Give it a shot.

gabrielsvinha · 2017-05-02T23:18:37Z

@AparnaKarve This one: #1169

AparnaKarve · 2017-05-02T23:26:17Z

Looks like #1169 would also have to be broken down into smaller functioning units.

Can you include the show implementation in this PR? That way we would not get the above error.
A basic detail page with a basic toolbar should be good enough for now.

gabrielsvinha · 2017-05-02T23:27:37Z

Without the toolbar also?

AparnaKarve · 2017-05-02T23:28:54Z

Either with a basic toolbar or without the toolbar

AparnaKarve · 2017-05-03T21:07:13Z

app/controllers/physical_server_controller.rb

+    @lastaction = session[:physical_server_lastaction]
+  end
+
+  def collect_data(server_id)


Can you delete this method too?
It might be needed later, but not now.

AparnaKarve · 2017-05-03T21:11:34Z

The rails log shows this error since it is not able to find physical_server.png

[----] F, [2017-05-03T14:06:29.210734 #20011:3fcd4b565f74] FATAL -- : ActionController::RoutingError (No route matches [GET] "/images/100/physical_server.png"):

Do you have a PR that would address this?

Other than this issue, and the comment above to remove the collect_data method, everything else looks great.

miq-bot · 2017-05-03T22:14:34Z

Checked commit gabrielsvinha@c0f9dbe with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

- Add physical server controller. - app/controllers/physical_server_controller.rb - Implement show_list view and basic toolbar - Implements show view for specific physical server

gabrielsvinha · 2017-05-03T22:29:48Z

@AparnaKarve The PR we are thinking is #1252

AparnaKarve · 2017-05-03T22:46:55Z

@gabrielsvinha Thanks.

@dclarizio This PR gets us the basic Physical Server view.
There are other PRs in the pipeline that depend on this.
This is now ready to go.

gabrielsvinha · 2017-05-03T23:01:31Z

martinpovolny · 2017-05-04T04:56:15Z

@gabrielsvinha, @AparnaKarve, @dclarizio : we have invested a significant effort to remove the

app/views/*/_main.html.haml and cleanup app/views/*/show.html.haml partials.

This PR introduces back the stuff that we have spent time removing. See:
app/views/physical_server/_main.html.haml and app/views/physical_server/show.html.haml partials.

Please, make sure that these files go away in the follow up PRs.

+#main_div
 +    - arr = %w(physical_server physical_servers)
 +    - if arr.include?(@display) && @showtype != "compare"
 +        = render(:partial => "layouts/gtl", :locals => {:action_url => "show/#{@ph_server.id}"})
 +    - else
 +        = render(:partial => @showtype)

compare has a separate path, does not have to go through here.
Prefer to use helpers to construct URLs or you will introduce bugs when moving to the "restful" URLs.
don't ever render :partial => VARIABLE. It is hard to get rid of as you have to find elsewhere what are the allowed values and it can have serious security implications!

dclarizio · 2017-05-04T13:47:59Z

@gabrielsvinha see Martin's note above ... might be best to just do a follow up PR with these changes alone ... Thx, Dan

AparnaKarve · 2017-05-04T14:04:01Z

@martinpovolny Thanks for catching that.

@gabrielsvinha The original PR #755 for Physical Server view had a lot of these issues addressed.
Please refer back to this PR for addressing the _main and _showrelated changes. Thanks.

MaysaMacedo mentioned this pull request Apr 26, 2017

Physical Server textual summary #1168

Merged

AparnaKarve reviewed Apr 27, 2017

View reviewed changes

gabrielsvinha force-pushed the physical_server_routing branch from d3b608e to 3211b95 Compare April 28, 2017 12:06

gabrielsvinha mentioned this pull request Apr 28, 2017

[WIP] Physical Server Pages #1169

Closed

miq-bot assigned AparnaKarve May 2, 2017

gabrielsvinha force-pushed the physical_server_routing branch 3 times, most recently from 60b5ec3 to 9f619d1 Compare May 3, 2017 11:59

CharlleDaniel mentioned this pull request May 3, 2017

Add network data in physical server page #1200

Closed

AparnaKarve reviewed May 3, 2017

View reviewed changes

dclarizio added compute/physical infrastructure enhancement fine/no labels May 3, 2017

dclarizio assigned dclarizio and unassigned AparnaKarve May 3, 2017

gabrielsvinha force-pushed the physical_server_routing branch from bb8ca87 to c0f9dbe Compare May 3, 2017 22:11

- Add routing to server.

c0f9dbe

- Add physical server controller. - app/controllers/physical_server_controller.rb - Implement show_list view and basic toolbar - Implements show view for specific physical server

dclarizio merged commit 5bb3535 into ManageIQ:master May 3, 2017

dclarizio added this to the Sprint 60 Ending May 8, 2017 milestone May 3, 2017

AparnaKarve mentioned this pull request May 4, 2017

Removed _main.html.haml and cleaned up _show.html.haml #1283

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Physical server routing #1162

Physical server routing #1162

gabrielsvinha commented Apr 26, 2017 •

edited

gabrielsvinha commented Apr 26, 2017

AparnaKarve Apr 27, 2017

AparnaKarve Apr 27, 2017

gabrielsvinha Apr 28, 2017

rodneyhbrown7 commented May 2, 2017

AparnaKarve commented May 2, 2017

gabrielsvinha commented May 2, 2017

AparnaKarve commented May 2, 2017

gabrielsvinha commented May 2, 2017

AparnaKarve commented May 2, 2017

AparnaKarve May 3, 2017

gabrielsvinha May 3, 2017 •

edited

AparnaKarve commented May 3, 2017

miq-bot commented May 3, 2017

gabrielsvinha commented May 3, 2017

AparnaKarve commented May 3, 2017

gabrielsvinha commented May 3, 2017

martinpovolny commented May 4, 2017 •

edited

dclarizio commented May 4, 2017

AparnaKarve commented May 4, 2017

Physical server routing #1162

Physical server routing #1162

Conversation

gabrielsvinha commented Apr 26, 2017 • edited

gabrielsvinha commented Apr 26, 2017

AparnaKarve Apr 27, 2017

Choose a reason for hiding this comment

AparnaKarve Apr 27, 2017

Choose a reason for hiding this comment

gabrielsvinha Apr 28, 2017

Choose a reason for hiding this comment

rodneyhbrown7 commented May 2, 2017

AparnaKarve commented May 2, 2017

gabrielsvinha commented May 2, 2017

AparnaKarve commented May 2, 2017

gabrielsvinha commented May 2, 2017

AparnaKarve commented May 2, 2017

AparnaKarve May 3, 2017

Choose a reason for hiding this comment

gabrielsvinha May 3, 2017 • edited

Choose a reason for hiding this comment

AparnaKarve commented May 3, 2017

miq-bot commented May 3, 2017

gabrielsvinha commented May 3, 2017

AparnaKarve commented May 3, 2017

gabrielsvinha commented May 3, 2017

martinpovolny commented May 4, 2017 • edited

dclarizio commented May 4, 2017

AparnaKarve commented May 4, 2017

gabrielsvinha commented Apr 26, 2017 •

edited

gabrielsvinha May 3, 2017 •

edited

martinpovolny commented May 4, 2017 •

edited