UIQM-431 Marc record fixed field 008 with proper order. #602
Conversation
Depends of marc specifications api for fixed field 008.
|
Hey @przemyslawturek, could you add to the description a screencast of the feature please? |
|
Ok - sure |
| @@ -292,7 +293,7 @@ const QuickMarcEditorRows = ({ | |||
| isRequestToCentralTenantFromMember, | |||
| onCheckCentralTenantPerm, | |||
| ); | |||
| const canBeLinkedAuto = isRecordForAutoLinking(recordRow, autoLinkableBibFields); | |||
| const canBeLinkedAuto = isRecordForAutoLinking(recordRow, autoLinkableBibFields) || false; | |||
There was a problem hiding this comment.
Why do we need additional || false if the returned value will be either true or falsy value?
There was a problem hiding this comment.
I don't remember why I do this :)
| return (typeOK && subTypeOK); | ||
| }; | ||
|
|
||
| marcSpec.spec.types.forEach((marcType, indexType) => { |
There was a problem hiding this comment.
I'd prefer to avoid indexes and use find method instead
const fixedFieldType = marcSpec.spec.types.find((fixedFieldType) => {
return fixedFieldRype.identifiedBy.or.some((fieldIdentifier) => {
return fieldIdentifier.tag === LEADER_TAG && checkTypeSubType(fieldIdentifier);
});
});
return fixedFieldType;
There was a problem hiding this comment.
Nice and clean - ok
| @@ -39,6 +39,7 @@ const propTypes = { | |||
| location: ReactRouterPropTypes.location.isRequired, | |||
| locations: PropTypes.object.isRequired, | |||
| marcType: PropTypes.oneOf(Object.values(MARC_TYPES)).isRequired, | |||
| marcSpec: PropTypes.object.isRequired, | |||
There was a problem hiding this comment.
I think marcSpec prop name is a little confusing. Without knowing what it is I though it was a general MARC file specification, but it's just for Fixed Fields. Let's rename all it's uses to fixedFieldSpec
There was a problem hiding this comment.
Yes I had doubt too with this name
|
|
||
| return FixedField ? <FixedField name={name} /> : null; | ||
| getDocumentType(marcSpec, type, subtype) { |
There was a problem hiding this comment.
I think the method name is a little misleading, because we're returning Fixed Field type for Leader\06 and Leader\07 values
Let's rename to getFixedFieldType
|
|
||
| it('should create correct document type', () => { |
There was a problem hiding this comment.
Also I think fixed field type would be better name than document type
| @@ -57,6 +57,10 @@ jest.mock('../../queries', () => ({ | |||
| sourceFiles: [], | |||
| isLoading: false, | |||
| }), | |||
| useMarcSpecifications: jest.fn().mockReturnValue({ | |||
There was a problem hiding this comment.
Probably this can be removed because there's no hook like this
There was a problem hiding this comment.
Yeah - forget to remove - thanks
Made changes as suggested
| BytesField, | ||
| } from '../BytesField'; | ||
|
|
||
| const BibliographicFixedField = ({ name, config }) => { |
There was a problem hiding this comment.
I think with the new implementation this separation between BibliographicFixedField, AuthorityFixedField and HoldingsFixedField components is not needed - they have the same api and the field configuration is coming from back-end so we can have just one FixedField component
Replace Bibliographic, Authority, Holdings FixedField to one FixedField Refactory getFixedField without marcType Removed unnecessary tests, fixed current without using the marcType props
|
SonarCloud Quality Gate failed.
|
Set marc fixed field 008 based on api specification with proper order of subfields.
Issue
UIQM-431
Approach
Using API for marc specification according to MODQM-332
Set fixed field 008 with proper order of subfields.
Changes applies to field 008 for bibliographic / holding / authority records
Extra
Adding hints with full name of subfield edited in fixed fields 006/007/008
Screen and Screencast
Before
After UIQM-431
2023-09-29-15-10-08.mp4
Learning
Pre-Merge Checklist
Before merging this PR, please go through the following list and take appropriate actions.
If there are breaking changes, please STOP and consider the following:
Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.
While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.