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

Fixes #23607: When a mandatory field in a technique is not defined, we can save a technique and we have a nasty error #5118

Conversation

amousset
Copy link
Member

@amousset amousset commented Oct 19, 2023

https://issues.rudder.io/issues/23607

In 8.0, rudderc has (inadvertently) changed the output format from a list of plain strings to a list of

{
  "name": "optional",
  "value": "previous value"
}

And regex from a string to:

{
  "error_message": "optional",
  "value": "previous value"
}

This change was not expected in the webapp, but the parser for this file was rewritten at the same time, with a typo on the constraint key name (constraint instead of constraints), which lead to totally ignoring constraints (and hide the error).

As the change in rudderc was made to improve consistency with select parameter type in techniques, this PR changes the expected format on the webapp side. We can do this as rudderc and the webapp come in the same package and are always in sync.

This PR fixes the checks on parameters in the editor:
image

Also adding an annotation in rudderc to avoid the null error_message in regex:

"regex": {
    "value": "^[A-z0-9._-]+$",
    "error_message": null
}

@amousset amousset requested a review from fanf October 19, 2023 21:22
@amousset amousset marked this pull request as draft October 19, 2023 21:23
@amousset amousset marked this pull request as ready for review October 19, 2023 22:27
@amousset
Copy link
Member Author

PR updated with a new commit

2 similar comments
@amousset
Copy link
Member Author

PR updated with a new commit

@amousset
Copy link
Member Author

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5118
-- Your faithful QA
Kant merge: "It is beyond a doubt that all our knowledge begins with experience."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/75253/console)

@amousset
Copy link
Member Author

OK, squash merging this PR

…e can save a technique and we have a nasty error
@amousset amousset force-pushed the bug_23607/when_a_mandatory_field_in_a_technique_is_not_defined_we_can_save_a_technique_and_we_have_a_nasty_error branch from 25be115 to 9c0d9e2 Compare October 25, 2023 09:15
@amousset amousset merged commit 9c0d9e2 into Normation:branches/rudder/8.0 Oct 25, 2023
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants