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

Adding embedded workflows to service catalog item #8815

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

jeffibm
Copy link
Member

@jeffibm jeffibm commented Jun 7, 2023

Before
Screenshot 2023-06-26 at 2 44 33 PM

  • Provisioning Entry Point is mandatory, has a default value
  • Reconfigure Entry Point is optional, and does not have a default value
  • Retirement Entry Point is optional, has a default value

After

Service Catalog item

  1. Go to Add a new Service Catalog item Form

  2. Provision Entry Point, Reconfigure Entry Point, Retirement Entry Point's input changed to Drop down list

image
  1. Drop-down options are Embedded Automate & Embedded Workflows
image
  1. When one item is selected, the corresponding input field appears
image
  1. When you click on the input field, a new modal box appears for embedded workflows. The old modal will appear for embedded automated
image
  1. When we select one of the items in the list, the name appears on the input field of
image

@jeffibm jeffibm requested a review from a team as a code owner June 7, 2023 11:47
@jeffibm jeffibm changed the title Adding automated workflows to service catalog item [WIP] Adding automated workflows to service catalog item Jun 7, 2023
@miq-bot miq-bot added the wip label Jun 7, 2023
@jeffibm jeffibm changed the title [WIP] Adding automated workflows to service catalog item [WIP] Adding embedded workflows to service catalog item Jun 7, 2023
@Fryguy
Copy link
Member

Fryguy commented Jun 7, 2023

@agrare Please review. While the name appearing seems ok, I think we should be sending the ID number as part of the form.

@agrare
Copy link
Member

agrare commented Jun 7, 2023

@jeffibm I'm not sure if you want to handle this in a separate PR, or if there is a way to handle it generically but there is also the entrypoint selection in the Automation / Customization / Service Dialogs / Add a new Dialog area
image
image
image

@agrare
Copy link
Member

agrare commented Jun 7, 2023

Hit a bug setting an entrypoint back to No Entry Point
image

[----] D, [2023-06-07T11:26:18.786726 #439637:63574] DEBUG -- :   Rendered /home/grare/adam/src/manageiq/manageiq-ui-classic/app/views/catalog/_reconfigure_entry_points.html.haml (Duration: 24.6ms | Allocations: 6483)
[----] F, [2023-06-07T11:26:18.786936 #439637:63574] FATAL -- : Error caught: [ActionView::Template::Error] undefined method `[]' for nil:NilClass
/home/grare/adam/src/manageiq/manageiq-ui-classic/app/views/catalog/_reconfigure_entry_points.html.haml:20:in `__home_grare_adam_src_manageiq_manageiq_ui_classic_app_views_catalog__reconfigure_entry_points_html_haml__1085212262945803632_418660'
/home/grare/adam/.gem/ruby/3.0.0/gems/actionview-6.1.7.3/lib/action_view/base.rb:247:in `public_send'
/home/grare/adam/.gem/ruby/3.0.0/gems/actionview-6.1.7.3/lib/action_view/base.rb:247:in `_run'
/home/grare/adam/.gem/ruby/3.0.0/gems/actionview-6.1.7.3/lib/action_view/template.rb:154:in `block in render

@agrare
Copy link
Member

agrare commented Jun 7, 2023

Minor, but when you load the form you have No Entry Point with no selection box
image
Pick one and the selection box shows up
image
But if you set it back to No Entry Point the selection box isn't cleared (NOTE maybe because of the exception mentioned above?)
image
But you can still click on the Click to select a provisioning entrypoint button and the workflows list shows up even though No Entry Point is selected.

@agrare
Copy link
Member

agrare commented Jun 7, 2023

If I pick a workflows provisioning entrypoint then pick add I get an error that I have to pick a provisioning entrypoing
image

@agrare
Copy link
Member

agrare commented Jun 7, 2023

I think we should be sending the ID number as part of the form.

It isn't obvious to me what we are sending based on the code changes, @jeffibm do you have an example of a ResourceAction that was created based off of one of these selections? I'm not able to get the form to save locally.

But yes in general we should be using the workflow ID since this is what will be set in the ResourceAction#configuration_script_id

@jeffibm
Copy link
Member Author

jeffibm commented Jun 8, 2023

I think we should be sending the ID number as part of the form.

It isn't obvious to me what we are sending based on the code changes, @jeffibm do you have an example of a ResourceAction that was created based off of one of these selections? I'm not able to get the form to save locally.

But yes in general we should be using the workflow ID since this is what will be set in the ResourceAction#configuration_script_id

I have to save the new values in UI somewhere before the submit action. its in progress...

@jeffibm
Copy link
Member Author

jeffibm commented Jun 8, 2023

@jeffibm I'm not sure if you want to handle this in a separate PR, or if there is a way to handle it generically but there is also the entrypoint selection in the Automation / Customization / Service Dialogs / Add a new Dialog area
image
image
image

adding the entry points here needs changes in ui-components repository. will link the PR in description

@Fryguy
Copy link
Member

Fryguy commented Jun 8, 2023

@jeffibm Please be sure this is hidden behind the prototype flag.

@jeffibm
Copy link
Member Author

jeffibm commented Jun 9, 2023

@agrare , I have a PR for ui-components noopurAg/ui-components#3

@jeffibm jeffibm force-pushed the add-workflows-to-catalogs branch 2 times, most recently from bc6c62a to f4f4358 Compare June 9, 2023 10:33
@jeffibm
Copy link
Member Author

jeffibm commented Jun 9, 2023

If I pick a workflows provisioning entrypoint then pick add I get an error that I have to pick a provisioning entrypoing
image

the changed values are being stored somewhere and validated. checking...

@Fryguy
Copy link
Member

Fryguy commented Jun 15, 2023

@agrare , I have a PR for ui-components noopurAg/ui-components#3

@jeffibm I think you made this PR to the wrong repository - this is pointing to @noopurAg 's fork

@jeffibm
Copy link
Member Author

jeffibm commented Jun 22, 2023

Screen.Recording.2023-06-22.at.5.20.57.PM.mov

yarn.lock Outdated Show resolved Hide resolved
@jeffibm jeffibm force-pushed the add-workflows-to-catalogs branch 2 times, most recently from a0e0d27 to bb48f97 Compare June 30, 2023 09:02
@jeffibm
Copy link
Member Author

jeffibm commented Jun 30, 2023

Screen.Recording.2023-06-30.at.2.44.54.PM.mov

@jeffibm
Copy link
Member Author

jeffibm commented Jul 18, 2023

Screen.Recording.2023-07-18.at.2.35.23.PM.mov

@jeffibm jeffibm force-pushed the add-workflows-to-catalogs branch 3 times, most recently from 98cc25e to 8fc4670 Compare July 18, 2023 10:22
@agrare
Copy link
Member

agrare commented Jul 21, 2023

@jeffibm FYI this conflicts with #8844

@agrare
Copy link
Member

agrare commented Jul 21, 2023

This works for me locally manually updating package.json to 1.5.0

@jeffibm jeffibm changed the title [WIP] Adding embedded workflows to service catalog item Adding embedded workflows to service catalog item Jul 21, 2023
@miq-bot miq-bot removed the wip label Jul 21, 2023
@miq-bot
Copy link
Member

miq-bot commented Jul 24, 2023

Checked commit jeffbonson@7b0ddac with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
12 files checked, 87 offenses detected

app/controllers/catalog_controller.rb

app/helpers/automate_tree_helper.rb

app/views/catalog/_embedded_workflows_modal.html.haml

  • ⚠️ - Line 1 - Layout/TrailingEmptyLines: Final newline missing.

app/views/catalog/_entry_point_selector.html.haml

  • ⚠️ - Line 1 - Avoid using instance variables in partials views
  • ⚠️ - Line 3 - Layout/TrailingEmptyLines: Final newline missing.
  • ⚠️ - Line 4 - Avoid using instance variables in partials views

app/views/catalog/_form_basic_info.html.haml

  • ⚠️ - Line 172 - id attribute must be in lisp-case
  • ⚠️ - Line 181 - id attribute must be in lisp-case
  • ⚠️ - Line 189 - id attribute must be in lisp-case
  • ⚠️ - Line 1 - Avoid using instance variables in partials views
  • ⚠️ - Line 20 - Avoid using instance variables in partials views
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 31 - Avoid using instance variables in partials views
  • ⚠️ - Line 4 - Layout/TrailingEmptyLines: Final newline missing.
  • ⚠️ - Line 4 - id attribute must be in lisp-case

app/views/catalog/_provision_entry_point.html.haml

  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/SpaceAfterComma: Space missing after comma.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Lint/SafeNavigationChain: Do not chain ordinary method call after safe navigation operator.
  • ⚠️ - Line 16 - id attribute must be in lisp-case
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 1 - Avoid using instance variables in partials views
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 3 - Layout/TrailingEmptyLines: Final newline missing.

app/views/catalog/_reconfigure_entry_point.html.haml

  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/SpaceAfterComma: Space missing after comma.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Lint/SafeNavigationChain: Do not chain ordinary method call after safe navigation operator.
  • ⚠️ - Line 16 - id attribute must be in lisp-case
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 1 - Avoid using instance variables in partials views
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 3 - Layout/TrailingEmptyLines: Final newline missing.

app/views/catalog/_retire_entry_point.html.haml

  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 10 - Layout/SpaceAfterComma: Space missing after comma.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 10 - Lint/SafeNavigationChain: Do not chain ordinary method call after safe navigation operator.
  • ⚠️ - Line 16 - id attribute must be in lisp-case
  • ⚠️ - Line 17 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 17 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 1 - Avoid using instance variables in partials views
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 20 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 3 - Layout/TrailingEmptyLines: Final newline missing.

@agrare agrare merged commit 0e07a1d into ManageIQ:master Jul 24, 2023
9 checks passed
@Fryguy
Copy link
Member

Fryguy commented Jul 25, 2023

@jeffibm A conflict occurred during the backport of this pull request to petrosian.

If this pull request is based on another pull request that has not been marked for backport, add the appropriate labels to the other pull request. Otherwise, please create a new pull request direct to the petrosian branch in order to resolve this.

Conflict details:

diff --cc app/stylesheet/miq-data-table.scss
index 5f231a62be,0c6c4e3bd7..0000000000
--- a/app/stylesheet/miq-data-table.scss
+++ b/app/stylesheet/miq-data-table.scss
@@@ -283,3 -283,73 +283,76 @@@ table.miq_preview 
      width: 135px;
    }
  }
++<<<<<<< HEAD
++=======
+ 
+ .reconfigure-add-button {
+   margin-bottom: 10px;
+ }
+ 
+ .disk-table {
+   .bx--form-item{
+     margin-bottom: 0px !important;
+   }
+ }
+ 
+ .header-button {
+   width: 100px !important;
+ }
+ 
+ .reconfigure-form {
+   .disk-table-list {
+     .bx--table-header-label {
+       white-space: normal !important;
+     }
+   }
+   .bx--btn--ghost:hover, .bx--btn--ghost:active{
+     background-color: transparent !important;
+   }
+   .bx--btn--ghost:focus {
+     border-color: transparent !important;
+     box-shadow: none !important;
+   }
+   .bx--btn--ghost span {
+     min-width: 50px !important;
+   }
+ }
+ .catalogs_form {
+ 
+   input[type="text"] {
+     width: 220px;
+   }
+ 
+   .no-padding{
+     padding: 0;
+   }
+ 
+   .long_text {
+     width: 100% !important;
+   }
+ 
+   .entry_point_selector {
+     padding: 3px;
+ 
+     .entry_point_text, .entry_point_button {
+       border-right: 0;
+     }
+ 
+     span.input-group-addon{
+       visibility: hidden;
+     }
+   }
+   .workflow_modal_wrapper {
+     min-height: 26px;
+     display: flex;
+     align-items: center;
+   }
+   
+   .workflows-entry-point-modal-body {
+     .miq-data-table {
+       margin-top: 0px;
+     }
+   }
+ }
+ 
++>>>>>>> 0e07a1d9ec (Merge pull request #8815 from jeffibm/add-workflows-to-catalogs)

@jeffibm
Copy link
Member Author

jeffibm commented Jul 26, 2023

@Fryguy , the css changes are coming in from this PR - Angular to react conversion Reconfigure form #8710

@Fryguy
Copy link
Member

Fryguy commented Jul 26, 2023

I don't think we are going to backport that file, so you will need to create a direct-to-petrosian backport PR

@Fryguy
Copy link
Member

Fryguy commented Jul 26, 2023

Backported to petrosian via #8873

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.

4 participants