fix(schema-extraction): check for _required field in validation rules #6151
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
No changes to documentation |
|
Component Testing Report Updated Apr 1, 2024 9:10 PM (UTC)
|
sgulseth
requested review from
a team and
skogsmaskin
and removed request for
a team
sgulseth
force-pushed
the
03-29-fix_schema-extraction_check_for__required_field_in_validation_rules
branch
from
b04c584 to
aead6d9
Compare
sgulseth
force-pushed
the
03-29-fix_schema-extraction_check_for__required_field_in_validation_rules
branch
from
aead6d9 to
a4ecae0
Compare
There was a problem hiding this comment.
Looks good. Added a test case for the case where the _required attribute supposedly can be 'optional' - code does the correct check, but just to prevent any regressions as this seems somewhat weird to me.
There is an additional case that would probably handle; on image/file fields, there is an assetRequired() rule which makes it so the asset field is required. This is a bit confusing, but if you have an image/file field that defines additional fields, setting any of those fields would make the base image/file field have a value, and would thus get treated as valid. What you usually want is assetRequired(), which ensures you can't publish a document without an asset for a given field. This weirdness is due to backwards compatibility, and hopefully something we can get rid of in the future.
I am going to merge this as-is for the next release, but might be worth trying to get a separate fix for the assetRequired case.
rexxars
deleted the
03-29-fix_schema-extraction_check_for__required_field_in_validation_rules
branch
…#6151) * fix(schema-extraction): check for _required field in validation rules * test(schema): add test case for extraction on explicit optional field --------- Co-authored-by: Espen Hovlandsdal <[email protected]>
Description
We were missing one check when checking for if a field is required.
The reason we were not catching this in our tests was because we have a "lazy copy" of createSchema to avoid depending on
sanity/core. ThecreateSchemainsanity/corealso infers some validation logic, which our code doens't do.note: The tsdoc marks the _required field as deprecated, but it isn't deprecated.
What to review
Is this correct? Are we missing any other checks?
Testing
Any other test cases we are missing?
Notes for release
Schema extraction: fixes a bug where we don't mark fields as required
fixes #6150