Include invalid workflows and disable them in the embedded workflow component #9708

elsamaryv · 2025-11-04T12:49:37Z

Disable the invalid workflows from being selected in the embedded workflow component. It can be done by showing the invalid workflows in the table but greying them out and making them not selectable.

Before

After

@miq-bot add-label enhancement

elsamaryv · 2025-11-04T13:49:55Z

@miq-bot add-reviewer @GilbertCherrie

Fryguy · 2025-11-04T15:03:15Z

@agrare Please also take a look

Fryguy · 2025-11-04T15:04:34Z

I like the idea of greying them out - perhaps we can modify the hover text to say add-haproxy.yml (Invalid Workflow)

Fryguy · 2025-11-04T15:05:42Z

Weird question - if a valid workflow was chosen as an entrypoint, then the workflow becomes invalid, then we reenter this selection page, is the invalid workflow still selected?

agrare · 2025-11-04T17:11:27Z

👍 for having (Invalid Workflow) after on greyed out rows

if a valid workflow was chosen as an entrypoint, then the workflow becomes invalid, then we reenter this selection page, is the invalid workflow still selected?

I'd expect that it would be selected but you would be able to submit the form. Maybe we have some way of indicating on the parent object (service template or dialog) that there is an invalid workflow selected. This might be entirely new, I don't know what happens if you delete an automate method that was selected by a service template or dialog but it would be good to check.

Fryguy · 2025-11-04T17:30:58Z

Oh i like the idea of the parent service catalog item having an indicator of an invalid workflow file, or perhaps mark the service catalog item as invalid somehow.

agrare · 2025-11-04T17:33:19Z

perhaps mark the service catalog item as invalid somehow.

Yeah this was exactly what I was thinking, if we had a #valid? method and an error message we could show a red X on the service catalog item list and we could use that to check for a bunch of different conditions like an archived source template too

Fryguy · 2025-11-04T17:33:23Z

I don't know what happens if you delete an automate method that was selected by a service template or dialog but it would be good to check.

I'm certain it's just broken. There are a number of ways to "break" a service catalog item, such as deleting the automate entry point and the other big one is deleting the underlying VM template (or associated resource)

I've wanted a way to mark service catalog items as "orphaned" or "invalidated" or something for a while now (ref)

agrare · 2025-11-04T18:03:54Z

@elsamaryv I know you have to rebase this on @GilbertCherrie's changes but just in case these issues aren't resolved in his branch:

I marked one of my workflows as invalid, and have a valid TFE ConfigurationScriptPayload:

vmdb(dev)> ConfigurationScriptPayload.pluck(:type, :name, :payload_valid, :payload_error)
=> 
[["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "email.asl", true, nil],
 ["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "provision-vm-small-med-large.asl", true, nil],
 ["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "provision.asl", false, "syntax error"],
 ["ManageIQ::Providers::TerraformEnterprise::AutomationManager::ConfigurationScriptPayload", "agrare/terraform-test@master", true, nil]]

In the ServiceCatalog entrypoint selection I see that I can select provision.asl, and my TFE workspace is in the list greyed out:

I'd expect the provision.asl to be greyed out, and the TFE workspace to not show up at all.

In the automate dialog editor I don't see the TFE workspace, but I can select an invalid workflow:

elsamaryv · 2025-11-05T14:06:02Z

In the automate dialog editor I don't see the TFE workspace, but I can select an invalid workflow:

@agrare Should the dialog editor behave the same way — by keeping invalid workflows disabled, or should it hide them entirely?

Fryguy · 2025-11-05T14:07:58Z

In the automate dialog editor I don't see the TFE workspace, but I can select an invalid workflow:

@agrare Should the dialog editor behave the same way — by keeping invalid workflows disabled, or should it hide them entirely?

I think it should work the same way keeping invalid workflows disabled.

agrare · 2025-11-05T14:09:48Z

@elsamaryv I thought it was the same component (@GilbertCherrie ?) so yes it should behave the same way.

Fryguy · 2025-11-05T14:23:33Z

Technically there are 2 components - one is a react component, the other is an angular component (I believe in the ui-components repo). Ideally they should work the same way.

agrare · 2025-11-05T14:31:21Z

Technically there are 2 components

I was going off of this comment #9670 (comment)

GilbertCherrie · 2025-11-05T14:34:15Z

@agrare oh yeah I thought they used the same underlying code but I guess not. @Fryguy is right the dialog editor code is here: ManageIQ/ui-components#471.

Either way we can't make both components share the same code (which is what I think you initially requested) until that dialog form is converted to react. That pr is in progress by @elsamaryv: #9592.

elsamaryv · 2025-11-05T14:56:55Z

I like the idea of greying them out - perhaps we can modify the hover text to say add-haproxy.yml (Invalid Workflow)

Done

elsamaryv · 2025-11-05T14:59:51Z

@elsamaryv I know you have to rebase this on @GilbertCherrie's changes but just in case these issues aren't resolved in his branch:

I marked one of my workflows as invalid, and have a valid TFE ConfigurationScriptPayload:
vmdb(dev)> ConfigurationScriptPayload.pluck(:type, :name, :payload_valid, :payload_error)
=> 
[["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "email.asl", true, nil],
 ["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "provision-vm-small-med-large.asl", true, nil],
 ["ManageIQ::Providers::Workflows::AutomationManager::Workflow", "provision.asl", false, "syntax error"],
 ["ManageIQ::Providers::TerraformEnterprise::AutomationManager::ConfigurationScriptPayload", "agrare/terraform-test@master", true, nil]]
In the ServiceCatalog entrypoint selection I see that I can select provision.asl, and my TFE workspace is in the list greyed out:

I'd expect the provision.asl to be greyed out, and the TFE workspace to not show up at all.

In the automate dialog editor I don't see the TFE workspace, but I can select an invalid workflow:

Hi @agrare , I’ve made the updates except for the Service Dialog Editor. Could you please verify the changes on your side?

elsamaryv · 2025-11-05T15:01:40Z

@agrare oh yeah I thought they used the same underlying code but I guess not. @Fryguy is right the dialog editor code is here: ManageIQ/ui-components#471.

Either way we can't make both components share the same code (which is what I think you initially requested) until that dialog form is converted to react. That pr is in progress by @elsamaryv: #9592.

Since the react conversion is in progress, @agrare @Fryguy do you think it makes sense to handle the invalid workflow for dialog editor in this PR?

agrare · 2025-11-05T15:02:59Z

@elsamaryv even if they are two different components for now, they definitely should behave the same way

GilbertCherrie · 2025-11-05T15:18:39Z

@elsamaryv you can make a pr to the ui-components repo and change that component there if we need to

…omponent

elsamaryv · 2025-11-06T11:14:53Z

@elsamaryv you can make a pr to the ui-components repo and change that component there if we need to

@agrare @Fryguy @GilbertCherrie Please review this PR for the changes in angular component (Service Dialog editor)

agrare · 2025-11-06T21:27:16Z

That PR was merged and released in v1.6.1 https://github.com/ManageIQ/ui-components/releases/tag/v1.6.1

elsamaryv · 2025-11-07T11:00:43Z

Hi @agrare @Fryguy @GilbertCherrie, Is this PR good to merge?

agrare

LGTM

agrare · 2025-11-07T14:23:46Z

...script/spec/settings-label-tag-mapping/__snapshots__/settings-label-tag-mapping.spec.js.snap

                                  <span
                                    className="bx--front-line"
-                                    title="Delete"
+                                    title="Click to delete this mapping"


hey @elsamaryv I just noticed this, this doesn't look related to the invalid embedded workflows change was this added accidentally?

The tests were failing due to outdated snapshots, ~~which were unrelated to the current task~~. Would it have been better to update them in a separate PR?

No, that's fine - strange they were broken though - I would have thought that would break master.

Actually - can you verify these are broken on master and maybe move into a separate PR? It makes me think something in this PR is changing these.

Yeah I was thinking this might have been from the updated ui-components, in which case should be fixed in a separate PR.

ohhh I hadn't thought of that - good point.

agrare · 2025-11-07T14:23:55Z

app/javascript/spec/tenant-quota-form/__snapshots__/tenant-quota-form.spec.js.snap

                                      <span
                                        className="bx--front-line"
-                                        title=""
+                                        title="Enforce a Value"


Same with these

elsamaryv · 2025-11-07T15:46:03Z

app/javascript/components/miq-data-table/miq-table-cell.jsx

+  const cellTitle = cell.data.title ? cell.data.title : cell.value;
  const truncateText = (
-    <span title={cell.value} className={classNames('bx--front-line', wrapClass, longerTextClass)}>
+    <span title={cellTitle} className={classNames('bx--front-line', wrapClass, longerTextClass)}>


Okay, so this is the change that caused the snapshot mismatch
@agrare @Fryguy

I made this change to modify the hover text for Invalid workflows
#9708 (comment)

miq-bot added the enhancement label Nov 4, 2025

elsamaryv force-pushed the fix-20077-embedded-workflows branch from 9fb7738 to 15154ed Compare November 4, 2025 13:31

elsamaryv marked this pull request as ready for review November 4, 2025 13:49

elsamaryv requested a review from a team as a code owner November 4, 2025 13:49

miq-bot requested a review from GilbertCherrie November 4, 2025 13:51

GilbertCherrie self-assigned this Nov 4, 2025

elsamaryv force-pushed the fix-20077-embedded-workflows branch from 15154ed to 5d6c514 Compare November 5, 2025 13:56

Include invalid workflows and disable them in the embedded workflow c…

c3a7ef1

…omponent

elsamaryv force-pushed the fix-20077-embedded-workflows branch from b81a242 to c3a7ef1 Compare November 5, 2025 15:28

Snapshots updated

88dda60

agrare approved these changes Nov 7, 2025

View reviewed changes

agrare reviewed Nov 7, 2025

View reviewed changes

elsamaryv commented Nov 7, 2025

View reviewed changes

GilbertCherrie approved these changes Nov 7, 2025

View reviewed changes

GilbertCherrie merged commit a60fc7e into ManageIQ:master Nov 7, 2025
19 checks passed

elsamaryv deleted the fix-20077-embedded-workflows branch November 7, 2025 16:37

Include invalid workflows and disable them in the embedded workflow component #9708

Include invalid workflows and disable them in the embedded workflow component #9708

Uh oh!

Conversation

elsamaryv commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elsamaryv commented Nov 4, 2025

Uh oh!

Fryguy commented Nov 4, 2025

Uh oh!

Fryguy commented Nov 4, 2025

Uh oh!

Fryguy commented Nov 4, 2025

Uh oh!

agrare commented Nov 4, 2025

Uh oh!

Fryguy commented Nov 4, 2025

Uh oh!

agrare commented Nov 4, 2025

Uh oh!

Fryguy commented Nov 4, 2025

Uh oh!

agrare commented Nov 4, 2025

Uh oh!

elsamaryv commented Nov 5, 2025

Uh oh!

Fryguy commented Nov 5, 2025

Uh oh!

agrare commented Nov 5, 2025

Uh oh!

Fryguy commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agrare commented Nov 5, 2025

Uh oh!

GilbertCherrie commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elsamaryv commented Nov 5, 2025

Uh oh!

elsamaryv commented Nov 5, 2025

Uh oh!

elsamaryv commented Nov 5, 2025

Uh oh!

agrare commented Nov 5, 2025

Uh oh!

GilbertCherrie commented Nov 5, 2025

Uh oh!

elsamaryv commented Nov 6, 2025

Uh oh!

agrare commented Nov 6, 2025

Uh oh!

elsamaryv commented Nov 7, 2025

Uh oh!

agrare left a comment

Choose a reason for hiding this comment

Uh oh!

agrare Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

elsamaryv Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fryguy Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Fryguy Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

agrare Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Fryguy Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

agrare Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

elsamaryv commented Nov 4, 2025 •

edited

Loading

Fryguy commented Nov 5, 2025 •

edited

Loading

GilbertCherrie commented Nov 5, 2025 •

edited

Loading

elsamaryv Nov 7, 2025 •

edited

Loading