Skip to content

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

Merged
merged 4 commits into from
May 19, 2022

Conversation

kylecarbs
Copy link
Member

This allows the frontend to render inputs displaying these values!

@kylecarbs kylecarbs requested a review from johnstcn May 19, 2022 13:04
@kylecarbs kylecarbs self-assigned this May 19, 2022
@kylecarbs kylecarbs requested a review from a team as a code owner May 19, 2022 13:04
This allows the frontend to render inputs displaying these values!
Comment on lines +33 to +34
SourceScheme database.ParameterSourceScheme `json:"source_scheme"`
DestinationScheme database.ParameterDestinationScheme `json:"destination_scheme"`
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, and fixed!

kylecarbs and others added 2 commits May 19, 2022 08:10
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 != "" {
Copy link
Member

@johnstcn johnstcn May 19, 2022

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(...)
        }

Copy link
Member Author

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.

Copy link
Member

@johnstcn johnstcn left a 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.

@kylecarbs kylecarbs enabled auto-merge (squash) May 19, 2022 13:19
@kylecarbs kylecarbs merged commit 38ee519 into main May 19, 2022
@kylecarbs kylecarbs deleted the paramcontains branch May 19, 2022 13:29
Copy link
Contributor

@greyscaled greyscaled left a comment

Choose a reason for hiding this comment

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

Nice

kylecarbs added a commit that referenced this pull request Jun 10, 2022
…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>
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.

3 participants