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

Consistent json schema validation of SliderParams #5533

Merged
merged 8 commits into from
Oct 21, 2022

Conversation

antgamdia
Copy link
Contributor

Description of the change

As pointed out by @dud225, we weren't enforcing the numeric validation consistently in both the slider and the text input. Besides, we were missing some json schema fields like exclusiveMinimum and exclusiveMaximum.
This PR is to fix that.

Benefits

Consistent param validation

Possible drawbacks

N/A

Applicable issues

Additional information

Note
This PR is part of a series of PRs aimed at closing this milestone. I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes).
PR 4 out of 6

    "number-maxmin": {
      "type": "number",
      "maximum": 10,
      "exclusiveMaximum": 9,
      "minimum": 2,
      "exclusiveMinimum": 1
    },

image

image

Also, note the HTML5 additional validation (user's browser locale msgs):

image

Copy link
Collaborator

@castelblanque castelblanque left a comment

Choose a reason for hiding this comment

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

Thanks!

Base automatically changed from 3-5525-schemaAnnotations to 1-3535-customSchema October 21, 2022 16:42
### Description of the change

In #5436 we mentioned that several fields were still pending, namely

- 1st level: object (at least, as array of tuples <string, any> for
defining each property.
-  Array level:  enum, arrays, objects

WRT the top-level objects, it is not an actual problem: if they have
`properties`, they will get rendered as any other nested param more. It
is only the case when it lacks the `properties` fields that we need to
handle.
- Solution: allow any serialized object like `{"foo": 1234}` and add
more validation messages.

WRT the array-level properties:
- Solution for enum: use the same approach as in the top-level enum.
- Solution for arrays and objects: use the same approach as in the
top-level objects: render a text field but enforce validation.

Additionally, the array now enforces `maxItems` and `minItems`

### Benefits

Every json schema datatype is now supported.

### Possible drawbacks

N/A

### Applicable issues

- fixes #5436 

### Additional information

> **Note**
> This PR is part of a series of PRs aimed at closing [this
milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27). I
have split the changes to ease the review process, but as there are many
interrelated changes, the tests will be performed in a separate PR (on
top of the branch containing all the changes).
>  PR 5 out of 6

#### `Top-level objects`:


![image](https://user-images.githubusercontent.com/11535726/197029146-c81a3001-4ca3-4251-87a1-d2eab55df864.png)

#### `Array<enum>`:


![image](https://user-images.githubusercontent.com/11535726/197028415-653f1e3d-f7b3-4d49-aca1-ac788ca2924f.png)

#### `Array<array>`:


![image](https://user-images.githubusercontent.com/11535726/197028515-a64aee32-5796-4dac-9eb3-8fc179435e71.png)

#### `Array<object>`:


![image](https://user-images.githubusercontent.com/11535726/197028310-fd815a03-4f5b-4004-bd0d-c3640f0dd023.png)

#### `YAML output`


![image](https://user-images.githubusercontent.com/11535726/197028679-83146445-7dc1-4516-9b37-d180e403ccee.png)

#### `Array max/min items`

Min items:

![image](https://user-images.githubusercontent.com/11535726/197030005-98e84cca-d78f-4fff-91ac-b02ec84c6a53.png)

Max items:

![image](https://user-images.githubusercontent.com/11535726/197029933-a162625e-da17-4846-be73-4339b97f9f1c.png)

Signed-off-by: Antonio Gamez Diaz <[email protected]>
Signed-off-by: Antonio Gamez Diaz <[email protected]>

Conflicts:
	dashboard/src/components/DeploymentForm/DeploymentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/ArrayParam.tsx
	dashboard/src/components/DeploymentForm/DeploymentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/SliderParam.tsx
@antgamdia antgamdia merged commit ee3e7b2 into 1-3535-customSchema Oct 21, 2022
@antgamdia antgamdia deleted the 4-5520-fixNumValidation branch October 21, 2022 16:47
@antgamdia antgamdia mentioned this pull request Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants