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

Introducing embedded_workflow to dynamic select box in service dialogs #8844

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

jeffibm
Copy link
Member

@jeffibm jeffibm commented Jun 28, 2023

Related PR -

After
The automation Type Field introduced
image

Can select Embedded Workflow from the select-box
image

Opens up a dialog box when we click on the workflow load-balancer icon
image

Select an item from the list to see the selected item in the embedded_workflow's field
image

@@ -0,0 +1,44 @@
module Mixins
module AutomationMixin
# Returns an object to be resued for passing down to service catalogs(rails) and dialog(angular) form.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Returns an object to be resued for passing down to service catalogs(rails) and dialog(angular) form.
# Returns an object to be reused for passing down to service catalogs(rails) and dialog(angular) form.

end

# Returns an array of entry point options.
# used in provision_entry_poins, retirement_entry_points, reconfigure_entry_points html files
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# used in provision_entry_poins, retirement_entry_points, reconfigure_entry_points html files
# used in provision_entry_points, retirement_entry_points, reconfigure_entry_points html files

@Fryguy
Copy link
Member

Fryguy commented Jun 28, 2023

@jeffibm Sorry to make you refactor again, but can you separate the linting fixes from the actual changes? If not, that's ok. Reason I'm asking is because those make backport much harder, and we'd like to backport this to petrosian.

(For future reference, try to avoid lint/cleanup changes mixed with feature changes - linting could be a separate PR beforehand)

@jeffibm jeffibm force-pushed the dialog-workflow-options branch 2 times, most recently from 890a960 to 658aa70 Compare June 28, 2023 14:08
@agrare
Copy link
Member

agrare commented Jun 30, 2023

Tested locally and see the configuration_script_id saved to the dialog's ResourceAction. When ordering a service using this dialog I see the workflow run and the dialog dropdown is populated.

One weird thing I noticed was the ID is shown to the right of the selector button, is this leftover debug info?
image

@jeffibm
Copy link
Member Author

jeffibm commented Jun 30, 2023

Tested locally and see the configuration_script_id saved to the dialog's ResourceAction. When ordering a service using this dialog I see the workflow run and the dialog dropdown is populated.

One weird thing I noticed was the ID is shown to the right of the selector button, is this leftover debug info? image

that field is a hidden field - https://github.com/ManageIQ/ui-components/blob/e3024f51a0092794d5cc3ab779adec1cbbeb9621/src/dialog-editor/components/modal-field-template/drop-down-list.html#L133C27-L133C27

could you please try a git pull to see if you have any new changes..

@agrare
Copy link
Member

agrare commented Jun 30, 2023

commit d7670e0a23d0868b24383b0d19184b597b050fe7 (HEAD -> dialog-workflow-options, noopurAG/dialog-workflow-options)

Ah I see the issue, yours and Noopur's branch had the same name and I was tracking her remote.

@jeffibm
Copy link
Member Author

jeffibm commented Jun 30, 2023

commit d7670e0a23d0868b24383b0d19184b597b050fe7 (HEAD -> dialog-workflow-options, noopurAG/dialog-workflow-options)

I created a new PR for ui-components recently (had linked it in the description above) - ManageIQ/ui-components#471.

@agrare
Copy link
Member

agrare commented Jun 30, 2023

👍 looks good with the latest changes

@agrare
Copy link
Member

agrare commented Jul 5, 2023

@Fryguy we have to wait to merge this until we can build the ui-components repo right? Should this be WIP then @jeffibm ?

@Fryguy
Copy link
Member

Fryguy commented Jul 5, 2023

I think we can build now, however I don't think the dependent PR has been merged (and thus not released)

@agrare
Copy link
Member

agrare commented Jul 5, 2023

Oh okay so ManageIQ/ui-components#471 (comment) has been resolved?

@Fryguy
Copy link
Member

Fryguy commented Jul 8, 2023

Similar to the other side, we need to hide the workflows stuff behind the prototype flag.

@jeffibm jeffibm force-pushed the dialog-workflow-options branch 2 times, most recently from 2fd4f26 to 5e4f4e5 Compare July 10, 2023 14:00
@@ -30,6 +30,8 @@

:javascript
ManageIQ.angular.app.value('dialogIdAction', '#{ dialog_id_action.to_json }');
ManageIQ.angular.app.value('automationKeys', '#{ Mixins::AutomationMixin::AUTOMATION_TYPES.to_json }');
Copy link
Member

Choose a reason for hiding this comment

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

Typically you don't reference Mixins directly, they are included into the relevant classes and you would reference them from the class.

Don't know much about the layout here, but you do include that mixin into MiqAeCustomizationHelper is that able to be accessed here or is there another class that you have access to?

Comment on lines 22 to 24
options + AUTOMATION_TYPES.map do |item|
[item[1][:label], item[1][:key]]
end
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a weird way to dereference a hash (the item[1] is actually just the value where item[0] is the hash key)

Suggested change
options + AUTOMATION_TYPES.map do |item|
[item[1][:label], item[1][:key]]
end
options + AUTOMATION_TYPES.values.map do |item|
[item[:label], item[:key]]
end

@agrare
Copy link
Member

agrare commented Jul 11, 2023

The rubocop comments in #8844 (comment) also look valid

@jeffibm jeffibm force-pushed the dialog-workflow-options branch 2 times, most recently from 872ed19 to 187bcc7 Compare July 12, 2023 04:55
@agrare
Copy link
Member

agrare commented Jul 13, 2023

Code looks good, going to give it one more test then I will approve

@jeffibm do we need to do a release of ui-components after merging ManageIQ/ui-components#471 ? Are you able to do that or do we need @Fryguy ?

@jeffibm
Copy link
Member Author

jeffibm commented Jul 13, 2023

Code looks good, going to give it one more test then I will approve

@jeffibm do we need to do a release of ui-components after merging ManageIQ/ui-components#471 ? Are you able to do that or do we need @Fryguy ?

I only know till the merge part. I haven't really done an actual release yet.

@agrare
Copy link
Member

agrare commented Jul 18, 2023

Dependent ui-components PR merged, we still need a release before this can be merged AFAIK

@agrare
Copy link
Member

agrare commented Jul 21, 2023

@jeffibm I think you need to update the package.json to pull in ui-components 1.5.0

@agrare
Copy link
Member

agrare commented Jul 21, 2023

Tested locally after manually updating package.json and everything looks good.

@miq-bot
Copy link
Member

miq-bot commented Jul 21, 2023

Checked commit jeffbonson@39c6042 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
4 files checked, 1 offense detected

app/views/miq_ae_customization/editor.html.haml

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

@agrare agrare assigned agrare and unassigned Fryguy Jul 21, 2023
@agrare agrare merged commit fa1620f into ManageIQ:master Jul 21, 2023
9 checks passed
@jeffibm
Copy link
Member Author

jeffibm commented Jul 24, 2023

just putting it here so we can use this to debug in future..


def self.dialog_data(id)
    puts "\n\n\n"
    dialog = Dialog.find(id)
    puts "\n\n\n========DIALOG = #{dialog.name.inspect}"

    dialog.dialog_tabs.each_with_index do |tab, t_index|
      puts "\n\n\n========TAB(#{t_index}) = #{tab.label} - #{tab.id}"
      tab.dialog_groups.each_with_index do |section, s_index|
        puts "\n\n\n========SECTION(#{s_index}) = #{section.label} - #{section.id}"
        section.dialog_fields.each_with_index do |field, f_index|
          puts "--\n\n\n========FIELD(#{f_index}) = #{field.label} - #{field.id}"
          ra = field.resource_action
          
          ['ae_class',
          'ae_instance',
          'ae_message',
          'ae_attributes',
          'configuration_template_id',
          'configuration_template_type',
          'configuration_script_id'].each do |item|
            puts "---Ra = #{item} = #{ra[item.to_sym]}"
          end
        end
      end
    end
  end

@Fryguy
Copy link
Member

Fryguy commented Jul 25, 2023

Backported to petrosian in commit 41f7227.

commit 41f7227182daba8110dd089df3a585fd85c941de
Author: Adam Grare <adam@grare.com>
Date:   Fri Jul 21 11:46:51 2023 -0400

    Merge pull request #8844 from jeffibm/dialog-workflow-options
    
    Introducing embedded_workflow to dynamic select box in service dialogs
    
    (cherry picked from commit fa1620f97fde061b7f29e3793e6391265358ce74)

Fryguy pushed a commit that referenced this pull request Jul 25, 2023
Introducing embedded_workflow to dynamic select box in service dialogs

(cherry picked from commit fa1620f)
@agrare agrare mentioned this pull request Jul 25, 2023
22 tasks
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