Fix and test for valItems introduced by custumization upload so that …

…they can be removed properly
TEIC · Oct 10, 2019 · b29251a · b29251a
1 parent 0ce509c
commit b29251a
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 2 deletions.
diff --git 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
@@ -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()
+  })
 })