Skip to content

feat: Expose the values contained in an HCL validation string to the API#1587

Merged
kylecarbs merged 4 commits into
mainfrom
paramcontains
May 19, 2022
Merged

feat: Expose the values contained in an HCL validation string to the API#1587
kylecarbs merged 4 commits into
mainfrom
paramcontains

Conversation

@kylecarbs
Copy link
Copy Markdown
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 thread codersdk/parameters.go Outdated
Comment thread codersdk/parameters.go
Comment on lines +33 to +34
SourceScheme database.ParameterSourceScheme `json:"source_scheme"`
DestinationScheme database.ParameterDestinationScheme `json:"destination_scheme"`
Copy link
Copy Markdown
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
Copy Markdown
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>
Comment thread coderd/parameters.go

func convertParameterSchema(parameterSchema database.ParameterSchema) (codersdk.ParameterSchema, error) {
contains := []string{}
if parameterSchema.ValidationCondition != "" {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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