Rework quickMARC records validation #560
Conversation
| }, null); | ||
|
|
||
| return errorMessage; | ||
| }, [context]); |
There was a problem hiding this comment.
Context is always a new object and will trigger the useCallback continuously. Let's spread it. The rest looks cool.
There was a problem hiding this comment.
I think this hook doesn't need to know what is in the context - only tests need to know what properties there are if they need some additional data for validation.
If we add some property in our component, but forget to add it to dependencies here it could cause a bug, so I'll just remove useCallback
There was a problem hiding this comment.
I should have written more clearly, I meant [...context]. All context data will be cached as soon as all responses are retrieved (for example for the linkingRules).
BogdanDenis
force-pushed
the
poc-improve-validation
branch
from
51a4b01 to
cb4fc25
Compare
BogdanDenis
changed the title
BogdanDenis
force-pushed
the
poc-improve-validation
branch
from
cb4fc25 to
85f5600
Compare
BogdanDenis
force-pushed
the
poc-improve-validation
branch
from
85f5600 to
c556940
Compare
| return undefined; | ||
| }, [initialValues, locations, marcType, linkableBibFields, linkingRules, prepareForSubmit]); | ||
| return validate(formValuesForValidation.records); | ||
| }, [validate, prepareForSubmit]); |
There was a problem hiding this comment.
validate function is not cached
There was a problem hiding this comment.
It's not cached, but we need to include it in dependencies - if context values change but we don't have it here, then validation will be performed with old values
| }); | ||
|
|
||
| it('should not return an error for a not linkable field', () => { | ||
| const record = cloneDeep(initialValues); |
There was a problem hiding this comment.
Please remove variables that are never used.
src/QuickMarcEditor/utils.test.js
Outdated
| expect(utils.validateLocationSubfield({}, locations)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Please remove eslint warnings.
| TAG_LENGTH, | ||
| } from '../../QuickMarcEditor/constants'; | ||
|
|
||
| const uncontrolledSubfieldGroups = ['uncontrolledAlpha', 'uncontrolledNumber']; |
There was a problem hiding this comment.
Please use the UNCONTROLLED_SUBFIELDS constant.
BogdanDenis
force-pushed
the
poc-improve-validation
branch
from
91d63d9 to
6880cfe
Compare
|
Description
Rework MARC record validation by breaking up
validateMarcRecordfunction into smaller re-usable functionsApproach
Validation will now be a
useValidationhook. The hook accepts data that is needed for validation (initial values, marc type, action, locations etc) and returns avalidatefunction.File
validators.jscontains small mostly re-usable functions that check for one specific rule (tag length, leader length, leader non-editable bytes not touched, a field has a subfield etc.)Then in
rules.jsthere are contstants that define for each marc type and action which rules should be applied to which fields.Issues
UIQM-526