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

fix(ui): fix broken workflowtemplate submit button. Fixes #13892 #13913

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

instauro
Copy link
Contributor

Fixes #13892

Motivation

Currently the submit button for workflowtemplates is broken both in main and the new 3.6.0 release. The reason is that the UI thinks the content of the workflowtemplate has been updated in the editor, even when it has not.

The process looks like this:
We fetch the workflow from the backend and keep it as a javascript object, convert it to a yaml string, update the editor content with this string, and then when the editor content is updated we convert it back to a javascript object. We then compare to original object with the parsed object to determine if it has been updated or not. This requires parse and stringify to be inverses, but when we downgraded yaml version from 1.2 to 1.1 in #13750 we accidently broke this assumption, because we are now parsing as 1.1 but stringifying as 1.2. This fails when we are dealing with some fields like creationTimestamp, where these versions seem to differ.

Modifications

This PR sets the yaml version as 1.1 for both parse and stringify.

Verification

I ran the UI locally in the devcontainer with the fix and submitted workflowtemplates and clusterworkflowtemplates. I also added a test which fails before the fix but passes after the fix.

@instauro instauro marked this pull request as ready for review November 17, 2024 11:05
Signed-off-by: instauro <instauro@proton.me>
Copy link
Contributor

@MasonM MasonM left a comment

Choose a reason for hiding this comment

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

Good catch! I entered #13915 that fixes this by eliminating the unnecessary stringify() calls, but that's orthogonal to this fix, and I think both should be merged.

Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

Thanks, this makes sense, I am glad this isn't to do with React.

@isubasinghe isubasinghe merged commit 2fd5488 into argoproj:main Nov 19, 2024
16 checks passed
@instauro instauro deleted the fix/yaml-version-mismatch branch November 19, 2024 07:19
Joibel pushed a commit that referenced this pull request Nov 22, 2024
)

Signed-off-by: instauro <instauro@proton.me>
Co-authored-by: instauro <instauro@proton.me>
(cherry picked from commit 2fd5488)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI (when 2 tabs open) 3.6.0rc4 - submit workflowtemplate greyed out
3 participants