fix: run button is enabled when an operator is unconfigured #3914
Conversation
There was a problem hiding this comment.
Pull Request Overview
Fixes the issue where the "Run" button could remain enabled when an operator is unconfigured by ensuring the workflow validation error stream emits on invalid states and by aligning emission order.
- Always emit workflowValidationErrorStream after updating workflowErrors, regardless of validity
- Emit operatorValidationStream after updating the workflow error map to ensure consistent downstream state
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (!validation.isValid) { | ||
| this.workflowErrors[operatorID] = validation; | ||
| } else { | ||
| delete this.workflowErrors[operatorID]; | ||
| this.workflowValidationErrorStream.next({ errors: this.workflowErrors, workflowEmpty: this.workflowEmpty }); | ||
| } |
There was a problem hiding this comment.
[nitpick] Consider adding a brief comment explaining that both streams are intentionally emitted here, and that operatorValidationStream is emitted after updating workflowErrors to keep subscribers' view of the state consistent (e.g., to correctly enable/disable the Run button). This will help future readers understand the ordering requirement.
| } | |
| } | |
| // Both streams are intentionally emitted here. | |
| // operatorValidationStream is emitted after updating workflowErrors to keep subscribers' view of the state consistent | |
| // (e.g., to correctly enable/disable the Run button). Do not change the ordering unless you fully understand the implications. |
Copilot uses AI. Check for mistakes.
…dation state change
|
Could you please add a frontend test case? |
|
@AnzhiZhang Please suggest a reviewer for this PR. |
|
The text "Invalid Workflow" in the button is too long for the button. We could change it to something like "Invalid." |
Thanks for reminding me. I will check how to add a frontend test case and try to add one later. |
Based on the file history. @kunwp1 @shengquan-ni @zuozhiw If you are still working on this project, can I request your review for this PR? |
I agree that it might be too long. Considering we still have enough space on the top, maybe this can provide a more informative message to the user? If we change it to something like "Invalid", we can have a hover text or sentence for more information. |
|
I suggest Zuozhi to review this PR |
3 participants
What changes were proposed in this PR?
This PR fixes the run button is enabled when an operator is unconfigured (#3192)
Any related issues, documentation, discussions?
Fixes #3192
How was this PR tested?
Manual test to ensure the reported behavior is fixed.
Was this PR authored or co-authored using generative AI tooling?
No.