-
Notifications
You must be signed in to change notification settings - Fork 20
Fix/847 dataset metadata field validations #848
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
Fix/847 dataset metadata field validations #848
Conversation
|
Hey @g-saracca! I think this is a great improvement, not just for making the date validation work in the SPA in the same way that it does for the JSF version of Dataverse, but also for making the error messages much more helpful. Right now in the JSF version, it looks like no matter what a user enters in a date field, the error message is always And even the review you did should make it much more clear to the rest of the team and the Dataverse community what types of dates currently are and aren't allowed, and why these rules were created in the first place. This shared understanding should make it much easier to keep improving things! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes dataset metadata field validations to better align with the backend validation system. The changes enhance error messages for URL and EMAIL fields by including the invalid value entered by the user, and implement comprehensive date field validation that closely mirrors the backend validation logic.
- Replaces existing validation functions with enhanced implementations that provide better error messaging
- Implements comprehensive date validation with specific error codes for different failure scenarios
- Consolidates validation logic into centralized helper classes for better maintainability
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/component/sections/shared/dataset-metadata-form/MetadataFieldsHelper.spec.ts | Adds comprehensive test coverage for the new date validation functionality |
| tests/component/sections/shared/dataset-metadata-form/DatasetMetadataForm.spec.tsx | Updates test expectations to match new error message format including user input |
| tests/component/sections/edit-dataset-metadata/EditDatasetMetadata.spec.tsx | Adds missing test setup for create mode metadata blocks |
| src/shared/helpers/Validator.ts | Adds new validation methods for URL, hostname, and float validation |
| src/sections/shared/form/DatasetMetadataForm/useSubmitDataset.ts | Updates import to use centralized validation error handling |
| src/sections/shared/form/DatasetMetadataForm/MetadataForm/MetadataBlockFormFields/MetadataFormField/useDefineRules.ts | Updates to use new validation methods and enhanced error messaging |
| src/sections/shared/form/DatasetMetadataForm/MetadataFieldsHelper.ts | Implements comprehensive date validation logic and error handling |
| src/sections/dataset/deaccession-dataset/DeaccessionDatasetModal.tsx | Updates to use centralized URL validation |
| src/metadata-block-info/domain/models/fieldValidations.ts | Removes old validation functions that have been moved to centralized helpers |
| src/metadata-block-info/domain/models/MetadataBlockInfo.ts | Removes unused date format constants |
| public/locales/en/shared.json | Updates error message templates to include user input and detailed date error messages |
| package.json | Updates dataverse client dependency version |
| CHANGELOG.md | Documents the validation improvements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/sections/shared/form/DatasetMetadataForm/MetadataFieldsHelper.ts
Outdated
Show resolved
Hide resolved
ChengShi-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! just need update package and changelog once they're ready
|
@ChengShi-1 changes done. |
ChengShi-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
|
very nice, all the messaging looks good. I even tested with leap year dates, that worked as well. |
What this PR does / why we need it:
Fixes some differences on how we were validating the DATE fields different than the backend.
Also includes the invalid value entered by the user in error messages for URL and EMAIL fields.
More info in "Suggestions on how to test this" and screenshots.
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Log in in both SPA and JSF and navigate to the create dataset page on each.
Pick a DATE field like Deposit Date.
Validate that valid values entered in the JSF form are valid also in the SPA. (for the JSF you will need to submit the form to check if an error appears - for the SPA it will be shown as you type)
Validate that invalid values entered in the JSF form are invalid also in the SPA ( the spa will show error messages with more info)
For example these are the valid and invalid cases that I've used for the unit tests.
Validate that the invalid value entered by the user is included in the error message for URL and EMAIL fields.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Yes, not really a user interface change but some new error messages.
Showing the invalid value entered by the user in the error message for EMAIL and URL type fields. 👇
New validations and error messages for DATE field type. 👇
Is there a release notes or changelog update needed for this change?:
Yes, added to the CHANGELOG.