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] Fixed reconfigure button #8428

Closed

Conversation

GilbertCherrie
Copy link
Member

@GilbertCherrie GilbertCherrie commented Aug 29, 2022

The reconfigure service button functionality was unintentionally removed in this PR: #8229

Screen Shot 2022-08-29 at 5 00 34 PM

This PR is adding that button functionality back, however the reconfigure form is still broken and nothing renders on that page. I was thinking this should be fixed in a new PR or should just be fixed by converting this page to react all together.

Screen Shot 2022-08-29 at 5 00 11 PM

@GilbertCherrie
Copy link
Member Author

In order to test this PR you need to replace the line:
resource_action = service_template.resource_actions.find_by(:action => 'Reconfigure') if service_template with
resource_action = service_template.resource_actions.first in service_controller.rb.

and you need to comment out the lines:

:klass   => ApplicationHelper::Button::GenericFeatureButton,
:options => {:feature => :reconfigure}

in service_center.rb

@GilbertCherrie GilbertCherrie force-pushed the fix_reconfigure_button branch 2 times, most recently from 703d43e to 7964c0d Compare August 29, 2022 21:16
@GilbertCherrie GilbertCherrie force-pushed the fix_reconfigure_button branch from 7964c0d to dc9083e Compare August 29, 2022 21:40
@GilbertCherrie
Copy link
Member Author

@miq-bot add_reviewer @jeffibm
@miq-bot add_reviewer @Fryguy
@miq-bot assign @jeffibm
@miq-bot add-label bug

@GilbertCherrie
Copy link
Member Author

GilbertCherrie commented Sep 7, 2022

@jeffibm Can you take a look at this PR? I can't get the values @resource_action_id and @target_id to be sent to the haml file _reconfigure_form.html.haml. If this PR is too confusing and you can fix this issue another way feel free to create a new PR and I can close this one. To find the button click on one of the services below then once on the summary screen of one of them go to configuration -> reconfigure.

Screen Shot 2022-09-06 at 8 06 38 PM

Also make these changes locally to get the PR to work:

In order to test this PR you need to replace the line:
resource_action = service_template.resource_actions.find_by(:action => 'Reconfigure') if service_template with
resource_action = service_template.resource_actions.first in service_controller.rb.

and you need to comment out the lines:

:klass   => ApplicationHelper::Button::GenericFeatureButton,
:options => {:feature => :reconfigure}
in service_center.rb

I don't think the form will load properly since something seems broken there and this should probably be converted to react but for now can you fix the reconfigure button so it properly gets the required values before we worry about converting to react.

Revert "Trying to get the reconfigure page working"

This reverts commit ef41b4e.

Revert "Revert "Trying to get the reconfigure page working""

This reverts commit 3317242.
@jeffibm jeffibm force-pushed the fix_reconfigure_button branch from fdacbec to 3d40e67 Compare September 9, 2022 05:53
@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2022

Checked commits GilbertCherrie/manageiq-ui-classic@dc9083e~...3d40e67 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
5 files checked, 29 offenses detected

app/controllers/service_controller.rb

app/helpers/application_helper/toolbar/service_center.rb

app/views/service/_reconfigure_form.html.haml

  • ⚠️ - Line 11 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 11 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 11 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 11 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 11 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 11 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 11 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 11 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 11 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 24 - Layout/TrailingWhitespace: Trailing whitespace detected.
  • ⚠️ - Line 25 - Layout/TrailingWhitespace: Trailing whitespace detected.
  • ⚠️ - Line 2 - Layout/TrailingEmptyLines: Final newline missing.
  • ⚠️ - Line 2 - id attribute must be in lisp-case

app/views/service/reconfigure_dialog.html.haml

  • ⚠️ - Line 1 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 1 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 2 - Layout/TrailingEmptyLines: Final newline missing.

@miq-bot miq-bot added the stale label Feb 27, 2023
@miq-bot
Copy link
Member

miq-bot commented Feb 27, 2023

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

@kbrock
Copy link
Member

kbrock commented May 24, 2023

this PR adds the button back but the button does not work
Unsure if more functionality needs to be added back

This may be moving to React

@kbrock kbrock added services and removed stale labels May 24, 2023
@kbrock kbrock changed the title Fixed reconfigure button [WIP] Fixed reconfigure button May 24, 2023
@kbrock kbrock added the wip label May 24, 2023
@miq-bot miq-bot added the stale label Aug 28, 2023
@miq-bot
Copy link
Member

miq-bot commented Aug 28, 2023

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2023

This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2023

This pull request has been automatically closed because it has not been updated for at least 3 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

@kbrock kbrock removed the stale label Nov 20, 2023
@kbrock
Copy link
Member

kbrock commented Nov 20, 2023

@GilbertCherrie I'm guessing this is still viable. could you confirm?

@kbrock kbrock reopened this Nov 20, 2023
@kbrock kbrock requested a review from a team as a code owner November 20, 2023 02:28
@GilbertCherrie
Copy link
Member Author

@kbrock I think @DavidResende0 fixed this in a different PR. Going to close this for now.

@GilbertCherrie GilbertCherrie deleted the fix_reconfigure_button branch November 20, 2023 13: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.

5 participants