From b29251a77253ca1228783fe050fc49380512d56a Mon Sep 17 00:00:00 2001 From: raffazizzi Date: Thu, 10 Oct 2019 15:29:33 -0400 Subject: [PATCH] Fix and test for valItems introduced by custumization upload so that they can be removed properly --- src/reducers/odd/processAttributes.js | 37 ++++++++++++++++++ test/reducers/updateOdd.js | 55 ++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/reducers/odd/processAttributes.js b/src/reducers/odd/processAttributes.js index b7284ea..efc71c9 100644 --- a/src/reducers/odd/processAttributes.js +++ b/src/reducers/odd/processAttributes.js @@ -165,6 +165,26 @@ function changeAttr(att, localAtt, attDef, odd) { valItem.setAttribute('ident', item.ident) } } + // finally check that the XML actually reflects the JSON and drop what doesn't. + // This may happen when a valItem was specified in an uploaded customization and + // now it has been deleted. In this case it won't be in the comparison BUT it will be in the XML + for (const vi of attDef.querySelectorAll('valItem')) { + // if there is an element, but no definition in either customization or localsource, + // get rid of the element + const valItemIdent = vi.getAttribute('ident') + if (!comparison.filter(c => c.ident === valItemIdent)[0] && + !att.valList.valItem.filter(v => v.ident === valItemIdent)[0]) { + const valList = vi.parentNode + valList.removeChild(vi) + } + } + // remove empty valList without a type change + const valListEl = attDef.querySelector('valList') + if (valListEl) { + if (valListEl.children.length === 0 && !valListEl.getAttribute('type')) { + attDef.removeChild(valListEl) + } + } break default: } @@ -340,6 +360,23 @@ export function processAttributes(specElement, specData, localData, localsource, // noop. We're attempting to restore an unchanged attribute. } } + // remove empty attDef without other attributes + const attDef = specElement.querySelector(`attDef[ident="${att.ident}"]`) + if (attDef) { + if ( + (!attDef.getAttribute('mode') || attDef.getAttribute('mode') === 'change') && + attDef.getAttribute('ident') && + attDef.attributes.length <= 2 && attDef.children.length === 0) { + attDef.parentNode.removeChild(attDef) + } + } + // remove empty valLists without @org + const attList = specElement.querySelector('attList') + if (attList) { + if (!attList.getAttribute('org') && attList.children.length === 0) { + attList.parentNode.removeChild(attList) + } + } } return odd } diff --git a/test/reducers/updateOdd.js b/test/reducers/updateOdd.js index 97f3097..112884a 100644 --- a/test/reducers/updateOdd.js +++ b/test/reducers/updateOdd.js @@ -688,7 +688,7 @@ describe('Update Customization (handles UPDATE_CUSTOMIZATION_ODD)', () => { }) let xml = parser.parseFromString(state.odd.customization.updatedXml) xml = global.usejsdom(xml) - expect(xml.querySelector('elementSpec[ident="title"] > attList > attDef[ident="level"]').getAttribute('usage')).toNotExist() + expect(xml.querySelector('elementSpec[ident="title"] > attList > attDef[ident="level"]')).toNotExist() }) it('should change an attribute defined on local element and some changes are already done. (desc returning to localsource value)', () => { @@ -938,7 +938,7 @@ describe('Update Customization (handles UPDATE_CUSTOMIZATION_ODD)', () => { xml = global.usejsdom(xml) // Returning to original definition means that usage should not be there any longer. // This test may need to be updated if better cleanup is implemented. - expect(xml.querySelector('elementSpec[ident="title"] > attList > attDef[ident="key"]').getAttribute('usage')).toNotExist() + expect(xml.querySelector('elementSpec[ident="title"] > attList > attDef[ident="key"]')).toNotExist() }) it('should change the valList type of an attribute defined on an element.', () => { @@ -1415,4 +1415,55 @@ describe('Update Customization (handles UPDATE_CUSTOMIZATION_ODD)', () => { xml = global.usejsdom(xml) expect(xml.querySelector('elementSpec[ident="div"] > attList > attDef[ident="type"] > valList > valItem > desc').textContent).toEqual('!!!') }) + + it('should remove a valItem added in a previous customization step.', () => { + customJson = JSON.parse(customization) + localJson = JSON.parse(localsource) + + const firstState = romajsApp({ + odd: { + customization: { isFetching: false, json: customJson, xml: customizationXMLString }, + localsource: { isFetching: false, json: localJson } + }, + selectedOdd: '' + }, { + type: 'CHANGE_CLASS_ATTRIBUTE_ON_ELEMENT', + element: 'div', + className: 'att.typed', + attName: 'type' + }) + const secondState = romajsApp(firstState, { + type: 'ADD_VALITEM', + member: 'div', + memberType: 'element', + attr: 'type', + value: 'chapter' + }) + const thirdState = romajsApp(secondState, { + type: 'UPDATE_CUSTOMIZATION_ODD' + }) + + // We simulate an upload of the previous state + const fourthState = romajsApp({ + odd: { + customization: { isFetching: false, json: thirdState.odd.customization.json, xml: thirdState.odd.customization.updatedXml }, + localsource: { isFetching: false, json: localJson } + }, + selectedOdd: '' + }, { + type: 'DELETE_VALITEM', + member: 'div', + memberType: 'element', + attr: 'type', + value: 'chapter' + }) + const state = romajsApp(fourthState, { + type: 'UPDATE_CUSTOMIZATION_ODD' + }) + // console.log(state.odd.customization.json.elements.filter(e => e.ident === 'div')[0].attributes[0].valList) + let xml = parser.parseFromString(state.odd.customization.updatedXml) + xml = global.usejsdom(xml) + // No other changes exist in this element, so the elementSpec should no longer be there. + expect(xml.querySelector('elementSpec[ident="div"]')).toNotExist() + }) })