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

Split up miq_capacity into threee separate controllers #1966

Merged
merged 2 commits into from
Aug 22, 2017

Conversation

skateman
Copy link
Member

@skateman skateman commented Aug 21, 2017

The miq_capacity controller was behaving as an explorer, however, it was providing three different (utilization, planning, bottlenecks) screens. Two of these screens are rendering an explorer tree using their own partial distinguished by the action_name. To continue with the explorer tree unification effort I'm splitting it up this into three controllers.

I was thinking how to split this up into small(er) commits but the code is too messy to do that, sorry for that @martinpovolny. More tree-related refactoring in the area will come in some followup PRs.

Depends on: ManageIQ/manageiq#15869

@miq-bot miq-bot added the wip label Aug 21, 2017
@skateman skateman force-pushed the capacity-split branch 3 times, most recently from f9e08fb to dd5e17e Compare August 22, 2017 07:37
@skateman skateman force-pushed the capacity-split branch 3 times, most recently from 0bb21f5 to fcc16de Compare August 22, 2017 11:15
@skateman skateman changed the title [WIP] Split up miq_capacity into threee separate controllers Split up miq_capacity into threee separate controllers Aug 22, 2017
@miq-bot miq-bot removed the wip label Aug 22, 2017
@miq-bot
Copy link
Member

miq-bot commented Aug 22, 2017

Checked commits skateman/manageiq-ui-classic@df27886~...5209295 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
31 files checked, 38 offenses detected

app/controllers/bottlenecks_controller.rb

app/controllers/planning_controller.rb

app/controllers/utilization_controller.rb

app/presenters/menu/default_menu.rb

  • ❗ - Line 252, Col 11 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/views/bottlenecks/_bottlenecks_report.html.haml

  • ⚠️ - Line 10 - Tagging a string as html safe may be a security risk, prefer safe_join or other Rails tag helpers instead.

app/views/planning/_planning_options.html.haml

  • ⚠️ - Line 27 - Line is too long. [164/160]
  • ⚠️ - Line 28 - Duplicated key in hash literal.
  • ⚠️ - Line 38 - Line is too long. [176/160]
  • ⚠️ - Line 48 - Line is too long. [202/160]
  • ⚠️ - Line 75 - Duplicated key in hash literal.

app/views/planning/_planning_report.html.haml

  • ⚠️ - Line 10 - Line is too long. [174/160]
  • ⚠️ - Line 15 - Line is too long. [161/160]
  • ⚠️ - Line 7 - Tagging a string as html safe may be a security risk, prefer safe_join or other Rails tag helpers instead.

app/views/planning/_planning_summary.html.haml

  • ⚠️ - Line 36 - Avoid more than 3 levels of block nesting.
  • ⚠️ - Line 36 - Use r[1].to_i.zero? instead of r[1].to_i == 0.
  • ⚠️ - Line 47 - Line is too long. [161/160]

app/views/planning/_planning_vm_profile.html.haml

  • ⚠️ - Line 38 - = "..." should be rewritten as ...
  • ⚠️ - Line 48 - = "..." should be rewritten as ...
  • ⚠️ - Line 53 - = "..." should be rewritten as ...

app/views/planning/index.html.haml

  • ⚠️ - Line 15 - Missing curly braces around a hash parameter.
  • ⚠️ - Line 28 - Missing curly braces around a hash parameter.
  • ⚠️ - Line 6 - Missing curly braces around a hash parameter.

app/views/utilization/_utilization_details.html.haml

  • ⚠️ - Line 11 - Use the return of the conditional for variable assignment and comparison.
  • ⚠️ - Line 19 - Line is too long. [414/160]

app/views/utilization/_utilization_options.html.haml

  • ⚠️ - Line 11 - Space between { and | missing.
  • ⚠️ - Line 11 - Space missing inside }.

app/views/utilization/_utilization_report.html.haml

  • ⚠️ - Line 14 - Use the return of the conditional for variable assignment and comparison.
  • ⚠️ - Line 20 - Block has too many lines. [26/25]
  • ⚠️ - Line 37 - Line is too long. [414/160]

app/views/utilization/_utilization_summary.html.haml

  • ⚠️ - Line 12 - Block has too many lines. [28/25]
  • ⚠️ - Line 22 - Line is too long. [180/160]
  • ⚠️ - Line 29 - Use the return of the conditional for variable assignment and comparison.
  • ⚠️ - Line 36 - Line is too long. [414/160]

spec/views/utilization/_utilization_options.html.haml_spec.rb

  • ❗ - Line 10, Col 27 - Style/MultilineHashBraceLayout - Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.
  • ❗ - Line 11, Col 11 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.
  • ❗ - Line 8, Col 40 - Style/MultilineHashBraceLayout - Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.

@h-kataria
Copy link
Contributor

verified in UI, maybe some of the code that is split into individual controllers can be unified in a separate PR that will knock down few of the code climate warnings as well.

@h-kataria h-kataria self-assigned this Aug 22, 2017
@h-kataria h-kataria added this to the Sprint 68 Ending Sep 11, 2017 milestone Aug 22, 2017
@h-kataria h-kataria merged commit 1cb052d into ManageIQ:master Aug 22, 2017
@skateman skateman deleted the capacity-split branch August 22, 2017 15:52
h-kataria added a commit to h-kataria/manageiq-ui-classic that referenced this pull request Nov 10, 2018
multiple issues fixed:

1 Download button toolbar was not being rendered for Bottlenecks explorer
2 `change_tab` transaction was not working because the tab id being passed in miq_tabs_init had typo, also fixed typo in method name `bottleneck_get_node_info` it should be `get_node_info`
3 download button was always disabled, added condition to enable download buttons on report tab
4 added missing routes for `change_tab` and `report_download`
5 `report_download` method was missing in controller
Issue was introduced in ManageIQ#1966

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1648500
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.

3 participants