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

Possibly malformed YAML in script dialog causes crash loop on subsequent use #536

Merged
merged 10 commits into from
Oct 18, 2023

Conversation

fdelcampo
Copy link
Contributor

Prevent the call function yaml.load() with parameter not defined and when the output is null.
Call method validateConfig, when receive the getScriptConfiguration.

Copy link
Contributor

@sebastian-aranda sebastian-aranda left a comment

Choose a reason for hiding this comment

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

ok I left a few comment on the review, but here the major one:
I think you are not taking advantage of React state handling. So I understand you created postValidate and failValidate functions to execute first the validation and then an action, but I think you should rather handle the validation separately and check the state of that validation on the action to execute (saveNewScriptSchema or updateScriptSchema). You are already calling this.validateConfig on the componentDidUpdate method after retrieving saved script configurations, you should only have to alter saveNewScriptSchema and updateScriptSchema to check the validationStatus state variable instead of embedding those in callbacks to be called inside the validateConfig function.
Please take a look to this 🙏.

@fdelcampo fdelcampo force-pushed the tickets/LOVE-224 branch 2 times, most recently from 80b2e98 to 6e8f40c Compare October 16, 2023 19:12
Copy link
Contributor

@sebastian-aranda sebastian-aranda left a comment

Choose a reason for hiding this comment

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

LGTM, but please take a look first to the CHANGELOG git conflict.

CHANGELOG.rst Outdated Show resolved Hide resolved
@fdelcampo fdelcampo merged commit 67fbcbe into develop Oct 18, 2023
1 check passed
@fdelcampo fdelcampo deleted the tickets/LOVE-224 branch October 18, 2023 16:40
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.

2 participants