Block Parser: Implement block schema numeric range validation #19433
Conversation
aduth
added
[Type] Enhancement
aduth
requested review from
ajitbohra,
ellatrix,
jorgefilipecosta,
Soean,
talldan and
youknowriad
as code owners
aduth
force-pushed
the
add/block-parser-numeric-range-validation
branch
from
4d0bc7a to
a0f977a
Compare
|
Size Change: +100 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
I tend to agree the code size aspect probably means avoiding using a pre-packaged JSON schema validation solution, but it would be good to see parity with the server-side interpretation of JSON schema, potentially including some form of format validation (which is slightly tricky in a reactive context, but it would be useful perhaps to validate on save, for instance). |
I'd agree with this, and it makes for a nice simple target baseline. Separately, I'd still wonder the question of: What subset of JSON schema are those REST validation functions intending to support? Or, more directly, is there a well-defined criteria, or is it the same sort of ad hoc decisions we've made client-side? I guess my fear there is that it could become a moving target, if and when the REST schema capability changes. |
Previously: #16314 (comment)
This pull request seeks to enhance the block parser validation to support JSON schema numeric ranges. This follows existing validation behavior for enums (#14810) as precedent.
In doing so, it resolves a typo in the Column block schema use of the
minimum(previouslymin) andmaximum(previouslymax) schema keywords, which previously had no effect since the validation was not yet in place.Implementation Notes: If and as this sort of validation becomes further enhanced, we should consider to improve developer communication of invalid values. For example, logging useful warning messages not unlike the equivalent REST errors. For the time being, I might consider this blocked by #19317, and worth considering for a subsequent pull request.
Open Questions: There's an open question as to just how far it makes sense to take this validation, given that to this point it's been implemented in an ad hoc fashion. Similarly, it begs the question whether we should consider leveraging an existing JSON schema validation solution (of which there are many) rather than to implement our own. Personally, I am mostly content to implement a subset of the validation features, considering the full extent of JSON schema is quite substantial (often manifesting itself in code size). Ideally, the implementations of schema validation is consistent with the equivalent
rest_validate_value_from_schemaimplementation to assure a consistent expected behavior. The supported validation should be documented as well.Testing Instructions:
Ensure unit tests pass:
Verify that manually changing the width of a Column block (likely modifying the comment-delimited attributes using the "Code Editor" mode) to a value less than 0 or greater than 100, will result in the value to be reset to an
undefinedstate.