diff --git a/CHANGELOG.md b/CHANGELOG.md index b680b02a..a2facfd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ * [UIQM-712](https://issues.folio.org/browse/UIQM-712) In field 007 for Projected Graphic type: change the `MfS` field type to `Byte` to allow only 1 character to be entered. * [UIQM-715](https://issues.folio.org/browse/UIQM-715) Reuse existing ids for fields after saving a record to avoid re-rendering and be able to focus on a field by ref. * [UIQM-723](https://issues.folio.org/browse/UIQM-723) Rename permissions. +* * [UIQM-724](https://issues.folio.org/browse/UIQM-724) Do not group together subfields during linking. ## [8.0.1] (https://github.com/folio-org/ui-quick-marc/tree/v8.0.1) (2024-04-18) diff --git a/src/QuickMarcEditor/utils.js b/src/QuickMarcEditor/utils.js index 4f1512b7..82832f93 100644 --- a/src/QuickMarcEditor/utils.js +++ b/src/QuickMarcEditor/utils.js @@ -6,7 +6,6 @@ import omit from 'lodash/omit'; import compact from 'lodash/compact'; import isString from 'lodash/isString'; import isNumber from 'lodash/isNumber'; -import toPairs from 'lodash/toPairs'; import flatten from 'lodash/flatten'; import flow from 'lodash/flow'; import assignWith from 'lodash/assignWith'; @@ -38,6 +37,7 @@ import { SUBFIELD_TYPES } from './QuickMarcEditorRows/BytesField'; import getMaterialCharsFieldConfig from './QuickMarcEditorRows/MaterialCharsField/getMaterialCharsFieldConfig'; import getPhysDescriptionFieldConfig from './QuickMarcEditorRows/PhysDescriptionField/getPhysDescriptionFieldConfig'; import { FixedFieldFactory } from './QuickMarcEditorRows/FixedField'; +import { MarcFieldContent } from '../common'; import { MARC_TYPES, ERROR_TYPES, @@ -70,6 +70,9 @@ export const isContentRow = (recordRow, marcType) => { || isControlNumberRow(recordRow)); }; +// returns an object with subfields values. order of subfields is not kept +// '$a valueA1 $a value A2 $b valueB' -> { '$a': ['valueA1', 'valueA2'], '$b': ['valueB'] } + export const getContentSubfieldValue = (content = '') => { return content.split(/\$/) .filter(str => str.length > 0) @@ -1008,47 +1011,42 @@ export const getCorrespondingMarcTag = (records) => { }; export const groupSubfields = (field, authorityControlledSubfields = []) => { - const subfields = toPairs(getContentSubfieldValue(field.content)); + const subfields = new MarcFieldContent(field.content); return subfields.reduce((groups, subfield) => { - const isControlled = authorityControlledSubfields.includes(subfield[0].replace('$', '')); - const isNum = /\$\d/.test(subfield[0]); - const isZero = /\$0/.test(subfield[0]); - const isNine = /\$9/.test(subfield[0]); - - const fieldContent = subfield[1].reduce((content, value) => [content, `${subfield[0]} ${value}`].join(' ').trimStart(), ''); + const isControlled = authorityControlledSubfields.includes(subfield.code.replace('$', '')); + const isNum = /\$\d/.test(subfield.code); + const isZero = /\$0/.test(subfield.code); + const isNine = /\$9/.test(subfield.code); - const formattedSubfield = { - content: fieldContent, - code: subfield[0], - }; + const subfieldCodeAndValue = `${subfield.code} ${subfield.value}`; if (isControlled) { - groups.controlled = [groups.controlled, formattedSubfield.content].join(' ').trim(); + groups.controlled = [groups.controlled, subfieldCodeAndValue].join(' ').trim(); return groups; } if (!isControlled && !isNum) { - groups[UNCONTROLLED_ALPHA] = [groups[UNCONTROLLED_ALPHA], formattedSubfield.content].join(' ').trim(); + groups[UNCONTROLLED_ALPHA] = [groups[UNCONTROLLED_ALPHA], subfieldCodeAndValue].join(' ').trim(); return groups; } if (isZero) { - groups.zeroSubfield = [groups.zeroSubfield, formattedSubfield.content].join(' ').trim(); + groups.zeroSubfield = [groups.zeroSubfield, subfieldCodeAndValue].join(' ').trim(); return groups; } if (isNine) { - groups.nineSubfield = [groups.nineSubfield, formattedSubfield.content].join(' ').trim(); + groups.nineSubfield = [groups.nineSubfield, subfieldCodeAndValue].join(' ').trim(); return groups; } if (isNum) { - groups[UNCONTROLLED_NUMBER] = [groups[UNCONTROLLED_NUMBER], formattedSubfield.content].join(' ').trim(); + groups[UNCONTROLLED_NUMBER] = [groups[UNCONTROLLED_NUMBER], subfieldCodeAndValue].join(' ').trim(); return groups; } diff --git a/src/common/entities/MarcFieldContent/MarcFieldContent.js b/src/common/entities/MarcFieldContent/MarcFieldContent.js new file mode 100644 index 00000000..fc13ac8b --- /dev/null +++ b/src/common/entities/MarcFieldContent/MarcFieldContent.js @@ -0,0 +1,96 @@ +// returns an array of subfields in the same order as in content string +// '$a valueA1 $a value A2 $b valueB' -> [{ '$a': 'valueA1' }, { '$a': 'valueA2' }, { '$b': 'valueB' }] + +export const getContentSubfieldValueArr = (content = '') => { + return content.split(/\$/) + .filter(str => str.length > 0) + .reduce((acc, str) => { + if (!str) { + return acc; + } + + const key = `$${str[0]}`; + const value = str.substring(2).trim(); + + return [...acc, { code: key, value }]; + }, []); +}; + +const reduceArr = (arr) => { + return arr.reduce((acc, cur) => { + const { code, value } = cur; + const reducedValues = acc[code]; + + if (reducedValues) { + return { + ...acc, + [code]: [...reducedValues, value], + }; + } + + return { + ...acc, + [code]: [value], + }; + }, {}); +}; + +export class MarcFieldContent { + constructor(content) { + this.content = content || ''; + + this.subfieldsArr = getContentSubfieldValueArr(this.content); + + // Proxy allows to define generic property getters + return new Proxy(this, this); + } + + map(callback) { + return this.subfieldsArr.map(callback); + } + + reduce(...args) { + return this.subfieldsArr.reduce(...args); + } + + forEach(callback) { + return this.subfieldsArr.forEach(callback); + } + + toContentString() { + return this.subfieldsArr.reduce((acc, cur) => { + return `${acc} ${cur.code} ${cur.value}`; + }, '').trim(); + } + + append(code, value) { + this.subfieldsArr.push({ code, value }); + + return this; // return this to be able to chain together method calls + } + + prepend(code, value) { + this.subfieldsArr.unshift({ code, value }); + + return this; + } + + removeByCode(code) { + this.subfieldsArr = this.subfieldsArr.filter(subfield => subfield.code !== code); + + return this; + } + + findAllByCode(code) { + return this.subfieldsArr.filter(subfield => subfield.code === code); + } + + get(target, property) { + // should be able to get array of subfields by calling marcFieldContent['$a'] + if (property.match(/\$\w$/)) { + return reduceArr(target.findAllByCode(property))[property]; + } + + return target[property]; + } +} diff --git a/src/common/entities/MarcFieldContent/MarcFieldContent.test.js b/src/common/entities/MarcFieldContent/MarcFieldContent.test.js new file mode 100644 index 00000000..6d407005 --- /dev/null +++ b/src/common/entities/MarcFieldContent/MarcFieldContent.test.js @@ -0,0 +1,95 @@ +import { MarcFieldContent } from './MarcFieldContent'; + +describe('MarcFieldContent', () => { + const content = '$a a1 $b b1 $b b2 $a a2'; + + let marcFieldContent = null; + + beforeEach(() => { + marcFieldContent = new MarcFieldContent(content); + }); + + describe('when calling `forEach`', () => { + it('should call the callback with each subfield', () => { + const cb = jest.fn(); + + marcFieldContent.forEach(cb); + + expect(cb).toHaveBeenCalledTimes(4); + expect(cb.mock.calls[0][0]).toEqual({ code: '$a', value: 'a1' }); + expect(cb.mock.calls[1][0]).toEqual({ code: '$b', value: 'b1' }); + expect(cb.mock.calls[2][0]).toEqual({ code: '$b', value: 'b2' }); + expect(cb.mock.calls[3][0]).toEqual({ code: '$a', value: 'a2' }); + }); + }); + + describe('when calling `map`', () => { + it('should call the callback with each subfield', () => { + const cb = jest.fn(); + + marcFieldContent.map(cb); + + expect(cb).toHaveBeenCalledTimes(4); + expect(cb.mock.calls[0][0]).toEqual({ code: '$a', value: 'a1' }); + expect(cb.mock.calls[1][0]).toEqual({ code: '$b', value: 'b1' }); + expect(cb.mock.calls[2][0]).toEqual({ code: '$b', value: 'b2' }); + expect(cb.mock.calls[3][0]).toEqual({ code: '$a', value: 'a2' }); + }); + }); + + describe('when calling `reduce`', () => { + it('should call the callback with each subfield', () => { + const cb = jest.fn().mockImplementation((acc, cur) => [...acc, cur]); + + marcFieldContent.reduce(cb, []); + + expect(cb).toHaveBeenCalledTimes(4); + expect(cb.mock.calls[0][1]).toEqual({ code: '$a', value: 'a1' }); + expect(cb.mock.calls[1][1]).toEqual({ code: '$b', value: 'b1' }); + expect(cb.mock.calls[2][1]).toEqual({ code: '$b', value: 'b2' }); + expect(cb.mock.calls[3][1]).toEqual({ code: '$a', value: 'a2' }); + }); + }); + + describe('when calling `toContentString`', () => { + it('should transform subfields array back to a string', () => { + expect(marcFieldContent.toContentString()).toEqual(content); + }); + }); + + describe('when calling `append`', () => { + it('should add a new subfield to the end', () => { + marcFieldContent.append('$a', 'a3'); + + expect(marcFieldContent.toContentString()).toEqual(`${content} $a a3`); + }); + }); + + describe('when calling `prepend`', () => { + it('should add a new subfield to the beginning', () => { + marcFieldContent.prepend('$a', 'a3'); + + expect(marcFieldContent.toContentString()).toEqual(`$a a3 ${content}`); + }); + }); + + describe('when calling `removeByCode`', () => { + it('should remove all subfields by code', () => { + marcFieldContent.removeByCode('$a'); + + expect(marcFieldContent.toContentString()).toEqual('$b b1 $b b2'); + }); + }); + + describe('when calling `findAllByCode`', () => { + it('should return all subfields by code', () => { + expect(marcFieldContent.findAllByCode('$a')).toEqual([{ code: '$a', value: 'a1' }, { code: '$a', value: 'a2' }]); + }); + }); + + describe('when using a subfield getter', () => { + it('should return an array of subfields values', () => { + expect(marcFieldContent.$a).toEqual(['a1', 'a2']); + }); + }); +}); diff --git a/src/common/entities/MarcFieldContent/index.js b/src/common/entities/MarcFieldContent/index.js new file mode 100644 index 00000000..bfbce8cb --- /dev/null +++ b/src/common/entities/MarcFieldContent/index.js @@ -0,0 +1 @@ +export * from './MarcFieldContent'; diff --git a/src/common/entities/index.js b/src/common/entities/index.js new file mode 100644 index 00000000..bfbce8cb --- /dev/null +++ b/src/common/entities/index.js @@ -0,0 +1 @@ +export * from './MarcFieldContent'; diff --git a/src/common/index.js b/src/common/index.js new file mode 100644 index 00000000..4ceefa21 --- /dev/null +++ b/src/common/index.js @@ -0,0 +1,2 @@ +export * from './constants'; +export * from './entities'; diff --git a/src/hooks/useAuthorityLinking/useAuthorityLinking.js b/src/hooks/useAuthorityLinking/useAuthorityLinking.js index 32c1b0c6..930d1b87 100644 --- a/src/hooks/useAuthorityLinking/useAuthorityLinking.js +++ b/src/hooks/useAuthorityLinking/useAuthorityLinking.js @@ -4,7 +4,6 @@ import { } from 'react'; import { useLocation } from 'react-router-dom'; import get from 'lodash/get'; -import omit from 'lodash/omit'; import pick from 'lodash/pick'; import { useStripes } from '@folio/stripes/core'; @@ -14,7 +13,6 @@ import { useAuthoritySourceFiles, useLinkSuggestions, } from '../../queries'; - import { getContentSubfieldValue, groupSubfields, @@ -25,6 +23,7 @@ import { isFieldLinked, applyCentralTenantInHeaders, } from '../../QuickMarcEditor/utils'; +import { MarcFieldContent } from '../../common'; import { AUTOLINKING_STATUSES, UNCONTROLLED_ALPHA, @@ -32,12 +31,6 @@ import { QUICK_MARC_ACTIONS, } from '../../QuickMarcEditor/constants'; -const joinSubfields = (subfields) => Object.keys(subfields).reduce((content, key) => { - const subfield = subfields[key].join(` ${key} `); - - return [content, `${key} ${subfield}`].join(' '); -}, '').trim(); - const formatSubfieldCode = (code) => { return code.startsWith('$') ? code : `$${code}`; }; const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { @@ -71,34 +64,70 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { const copySubfieldsFromAuthority = useCallback((bibSubfields, authField, bibTag) => { const linkingRule = findLinkingRule(bibTag, authField.tag); - const authSubfields = getContentSubfieldValue(authField.content); - let subfieldsFromAuthority = {}; + const authSubfields = new MarcFieldContent(authField.content); + const subfieldsAfterLinking = new MarcFieldContent(); - linkingRule.authoritySubfields.forEach(subfieldCode => { - const subfieldModification = linkingRule.subfieldModifications?.find(mod => mod.source === subfieldCode); + const isSubfieldControlled = (subfieldLetter) => { + return linkingRule.authoritySubfields.includes(subfieldLetter) + || linkingRule.subfieldModifications.find(mod => mod.target === subfieldLetter); + }; + + /* + Rules for linking fields are: + 1. Iterate over authority subfields. + 2. If a subfield is not controlled - don't add it to linked field + 3. If a subfield is modified - apply the modification rule and add it to linked field + 4. If a subfield is a target of modification - don't add it to linked field + 5. If a subfield is not modified - add it to linked field + 6. Iterate over bib subfields + 7. If a subfield is controlled - don't add it to linked field + 8. If a subfield is uncontrolled - add it to linked field + */ + + authSubfields.forEach(subfield => { + const subfieldLetter = subfield.code.replace('$', ''); + const isControlled = isSubfieldControlled(subfieldLetter); + const subfieldModification = linkingRule.subfieldModifications + ?.find(mod => mod.source === subfieldLetter); + + // if authority subfield is a target of modification then don't add it to linked field + const isSubfieldTarget = linkingRule.subfieldModifications + ?.find(mod => mod.target === subfieldLetter); + + if (isSubfieldTarget) { + return; + } + + if (!isControlled) { + return; + } if (subfieldModification) { - subfieldsFromAuthority[formatSubfieldCode(subfieldModification.target)] = - authSubfields[formatSubfieldCode(subfieldCode)]; - } else if (authSubfields[formatSubfieldCode(subfieldCode)]?.[0]) { - subfieldsFromAuthority[formatSubfieldCode(subfieldCode)] = authSubfields[formatSubfieldCode(subfieldCode)]; + // special case of modification, where $t becomes $a and should therefore be the first subfield + // in this case we'll pre-pend the subfield + if (bibTag === '240' && subfieldLetter === 't' && subfieldModification.source === 't' && subfieldModification.target === 'a') { + subfieldsAfterLinking.removeByCode('$a'); + subfieldsAfterLinking.prepend('$a', subfield.value); + } else { + subfieldsAfterLinking.append(formatSubfieldCode(subfieldModification.target), subfield.value); + } } else { - delete bibSubfields[formatSubfieldCode(subfieldCode)]; + subfieldsAfterLinking.append(subfield.code, subfield.value); } }); - if (bibTag === '240' && linkingRule.subfieldModifications?.find(mod => mod.source === 't')?.target === 'a') { - subfieldsFromAuthority = { - $a: subfieldsFromAuthority.$a, - ...omit(subfieldsFromAuthority, '$a'), - }; - } + bibSubfields.forEach(subfield => { + const subfieldLetter = subfield.code.replace('$', ''); + const isControlled = isSubfieldControlled(subfieldLetter); - // take authority subfields first and then bib subfields - return { - ...subfieldsFromAuthority, - ...omit(bibSubfields, Object.keys(subfieldsFromAuthority)), - }; + if (isControlled) { + return; + } + + subfieldsAfterLinking.append(subfield.code, subfield.value); + }); + + return subfieldsAfterLinking; }, [findLinkingRule]); const getLinkableAuthorityField = useCallback((authoritySource, bibField) => { @@ -138,7 +167,7 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { }, [findLinkingRule]); const updateBibFieldWithLinkingData = useCallback((bibField, linkedAuthorityField, authorityRecord) => { - const bibSubfields = getContentSubfieldValue(bibField.content); + const bibSubfields = new MarcFieldContent(bibField.content); const sourceFile = sourceFiles.find(file => file.id === authorityRecord.sourceFileId); let newZeroSubfield = ''; @@ -149,13 +178,15 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { newZeroSubfield = authorityRecord.naturalId; } - bibSubfields.$0 = [newZeroSubfield]; + bibSubfields.removeByCode('$0'); + bibSubfields.append('$0', newZeroSubfield); const updatedBibSubfields = copySubfieldsFromAuthority(bibSubfields, linkedAuthorityField, bibField.tag); - updatedBibSubfields.$9 = [authorityRecord.id]; + updatedBibSubfields.removeByCode('$9'); + updatedBibSubfields.append('$9', authorityRecord.id); bibField.prevContent = bibField.content; - bibField.content = joinSubfields(updatedBibSubfields); + bibField.content = updatedBibSubfields.toContentString(); }, [copySubfieldsFromAuthority, sourceFiles]); const getSubfieldGroups = useCallback((field, suggestedField) => { @@ -170,15 +201,15 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { }, [linkingRules]); const updateLinkedField = useCallback((field) => { - const uncontrolledNumberSubfields = getContentSubfieldValue(field.subfieldGroups?.[UNCONTROLLED_NUMBER]); - const uncontrolledAlphaSubfields = getContentSubfieldValue(field.subfieldGroups?.[UNCONTROLLED_ALPHA]); + const uncontrolledNumberSubfields = new MarcFieldContent(field.subfieldGroups?.[UNCONTROLLED_NUMBER]); + const uncontrolledAlphaSubfields = new MarcFieldContent(field.subfieldGroups?.[UNCONTROLLED_ALPHA]); const uncontrolledNumber = uncontrolledNumberSubfields.$9?.[0] - ? joinSubfields(omit(uncontrolledNumberSubfields, '$9')) + ? uncontrolledNumberSubfields.removeByCode('$9').toContentString() : field.subfieldGroups[UNCONTROLLED_NUMBER]; const uncontrolledAlpha = uncontrolledAlphaSubfields.$9?.[0] - ? joinSubfields(omit(uncontrolledAlphaSubfields, '$9')) + ? uncontrolledAlphaSubfields.removeByCode('$9').toContentString() : field.subfieldGroups[UNCONTROLLED_ALPHA]; return { @@ -192,15 +223,15 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { }, []); const updateAutoLinkableField = useCallback((field, suggestedField) => { + const subfields = new MarcFieldContent(field.content); + if ( suggestedField.linkDetails?.status === AUTOLINKING_STATUSES.ERROR - && getContentSubfieldValue(field.content).$9?.[0] + && subfields.$9?.[0] ) { - const subfields = getContentSubfieldValue(field.content); - return { ...field, - content: joinSubfields(omit(subfields, '$9')), + content: subfields.removeByCode('$9').toContentString(), }; } @@ -295,13 +326,13 @@ const useAuthorityLinking = ({ tenantId, marcType, action } = {}) => { ]); const unlinkAuthority = useCallback((field) => { - const bibSubfields = getContentSubfieldValue(field.content); + const bibSubfields = new MarcFieldContent(field.content); - delete bibSubfields.$9; + bibSubfields.removeByCode('$9'); delete field.linkDetails; delete field.subfieldGroups; - field.content = field.prevContent ?? joinSubfields(bibSubfields); + field.content = field.prevContent ?? bibSubfields.toContentString(); delete field.prevContent; return { diff --git a/src/hooks/useAuthorityLinking/useAuthorityLinking.test.js b/src/hooks/useAuthorityLinking/useAuthorityLinking.test.js index 89590277..196c1c07 100644 --- a/src/hooks/useAuthorityLinking/useAuthorityLinking.test.js +++ b/src/hooks/useAuthorityLinking/useAuthorityLinking.test.js @@ -305,7 +305,7 @@ describe('Given useAuthorityLinking', () => { }; expect(result.current.linkAuthority(authority, _authoritySource, field)).toMatchObject({ - content: '$a field for modification $f ff $g gg $h hh $k kk $0 some.url/n0001 $9 authority-id', + content: '$a field for modification $f ff $h hh $g gg $k kk $0 some.url/n0001 $9 authority-id', linkDetails: { authorityId: 'authority-id', authorityNaturalId: 'n0001', @@ -344,6 +344,35 @@ describe('Given useAuthorityLinking', () => { }); }); + it('should not group subfields together', () => { + const { result } = renderHook(() => useAuthorityLinking(), { wrapper }); + + const authority = { + id: 'authority-id', + sourceFileId: '1', + naturalId: 'n0001', + }; + const field = { + tag: '600', + content: '$c (Fictitious character) $x x1 $y y $x x2 $a Black Panther $2 fast $0 (OCoLC)fst02000849', + }; + const _authoritySource = { + fields: [{ + tag: '100', + content: '$a Black Panther $c (Fictitious character)', + }], + }; + + expect(result.current.linkAuthority(authority, _authoritySource, field)).toMatchObject({ + content: '$a Black Panther $c (Fictitious character) $x x1 $y y $x x2 $2 fast $0 some.url/n0001 $9 authority-id', + linkDetails: { + authorityId: 'authority-id', + authorityNaturalId: 'n0001', + linkingRuleId: 8, + }, + }); + }); + describe('when authority record doesnt pass required subfields validation', () => { it('should throw validation error', () => { useAuthorityLinkingRules.mockReturnValue({ diff --git a/test/jest/fixtures/linkingRules.js b/test/jest/fixtures/linkingRules.js index 5270190f..c7b0bba4 100644 --- a/test/jest/fixtures/linkingRules.js +++ b/test/jest/fixtures/linkingRules.js @@ -5,30 +5,35 @@ export const linkingRules = { authorityField: '100', authoritySubfields: ['a', 'b', 't', 'd'], validation: {}, + subfieldModifications: [], autoLinkingEnabled: true, }, { id: 8, bibField: '600', authorityField: '100', authoritySubfields: ['a', 'b', 'c', 'd', 'g', 'j', 'q', 'f', 'h', 'k', 'l', 'm', 'n', 'o', 'p', 'r', 's', 't'], + subfieldModifications: [], autoLinkingEnabled: true, }, { id: 12, bibField: '650', authorityField: '150', authoritySubfields: ['a', 'b', 'g'], + subfieldModifications: [], autoLinkingEnabled: false, }, { id: 15, bibField: '700', authorityField: '100', authoritySubfields: ['a', 'b', 'c', 'd', 'j', 'q', 'f', 'h', 'k', 'l', 'm', 'n', 'o', 'p', 'r', 's', 't', 'g'], + subfieldModifications: [], autoLinkingEnabled: true, }, { id: 17, bibField: '711', authorityField: '111', authoritySubfields: ['a', 'c', 'e', 'q', 'f', 'h', 'k', 'l', 'p', 's', 't', 'd', 'g', 'n'], + subfieldModifications: [], autoLinkingEnabled: true, }], isLoading: false,