-
Notifications
You must be signed in to change notification settings - Fork 940
feat: Expose the values contained in an HCL validation string to the API #1587
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
Conversation
This allows the frontend to render inputs displaying these values!
SourceScheme database.ParameterSourceScheme `json:"source_scheme"` | ||
DestinationScheme database.ParameterDestinationScheme `json:"destination_scheme"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid importing database types into codersdk
. Since these are both strings under the hood, let's just call a spade a spade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, and fixed!
Co-authored-by: Cian Johnston <cian@coder.com>
@@ -122,6 +124,37 @@ func (api *api) deleteParameter(rw http.ResponseWriter, r *http.Request) { | |||
}) | |||
} | |||
|
|||
func convertParameterSchema(parameterSchema database.ParameterSchema) (codersdk.ParameterSchema, error) { | |||
contains := []string{} | |||
if parameterSchema.ValidationCondition != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figure we'll have validation conditions other than contains
at some point, it's probably good to do this now:
switch parameterSchema.ValidationCondition {
case "contains": // parameter.Contains
default:
return codersdk.ParameterSchema{}, xerrors.Errorf(...)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our contains
function already does this, and since it has to parse HCL I'd rather it be self-contained and reaaally well tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than existing comments (and once the linter gods are appeased), LGTM! Might also be good to get input from FE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
…API (#1587) * feat: Expose the values contained in an HCL validation string to the API This allows the frontend to render inputs displaying these values! * Update codersdk/parameters.go Co-authored-by: Cian Johnston <cian@coder.com> * Call a spade a space * Fix linting errors with type conversion Co-authored-by: Cian Johnston <cian@coder.com>
This allows the frontend to render inputs displaying these values!