When the parent has attributes, the xml data_parser migrate_plus plugin reduces single-value results to scalars wrongly: it takes the attributes instead of the value.

The count() function only counts the children of the SimpleXMLElement. The attributes which the element might have are not counted, but returned when the reset function is used. An alternative could be to return the first child of the element, but the attributes may be relevant to the processor, hence leaving control to the processor. Therefor only reducing single-value arrays to scalar seems to be the viable option for this problem.
The inline code documentation also states transforming the result to scalar which would negate the possibility of a child node of a SimpleXmlElement for backwards compatibility.

CommentFileSizeAuthor
#10 2986883-10.patch3.65 KBheddn
#10 interdiff_6-10.txt2.14 KBheddn
#6 migrate_plus-XML_parser_result_to_scalar_bug-2986883-5.patch3.31 KBStefanPr
#4 migrate_plus-XML_parser_result_to_scalar_bug-2986883-4.patch3.31 KBStefanPr
#2 migrate_plus_fix_xml_reduce_single_value-2986883-2.patch591 bytesrojan raj

Comments

rojan raj created an issue. See original summary.

rojan raj’s picture

rojan raj commented
StatusFileSize
new migrate_plus_fix_xml_reduce_single_value-2986883-2.patch591 bytes

Patch attached for fix.

rojan raj’s picture

rojan raj commented
Parent issue: #2976091: Simple Xml data_parser reduce single-value to parent's attribute »
StefanPr’s picture

StefanPr commented
Title: Xml data_parser reduce single-value to parent's attribute » Xml data_parser reducing single-value results to scalar for SimpleXMLElements
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new migrate_plus-XML_parser_result_to_scalar_bug-2986883-4.patch3.31 KB

Added/adjusted some code comments for clarification.
Added tests.

Status: Needs review » Needs work

The last submitted patch, 4: migrate_plus-XML_parser_result_to_scalar_bug-2986883-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

StefanPr’s picture

StefanPr commented
StatusFileSize
new migrate_plus-XML_parser_result_to_scalar_bug-2986883-5.patch3.31 KB

Code style fix for succeeding tests

StefanPr’s picture

StefanPr commented
Status: Needs work » Needs review
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work 
+++ b/tests/data/xml_data_parser.xml
@@ -0,0 +1,22 @@
+    <id>1</id>
+    <name>Elizabeth</name>
+    <children>
+      <child age="8">
+        <name>Elizabeth Junior</name>
+      </child>
+    </children>
+  </person>
+  <person>
+    <id>2</id>
+    <name>George</name>
+    <children>
+      <child>
+        <name>George Junior</name>
+      </child>
+    </children>
+  </person>

Could we have a scenario with multiple children too to confirm those are imported as arrays still?

StefanPr’s picture

StefanPr commented

I'll try and add some more tests since there aren't any for this data_parser, but these wouldn't be needed for this issue since it selects a single child. If we wanted to select multiple children the selector would be /children. The scenario might not be the best example since it implies working for multiple children, but it tests the current issue retrieving a single value from the last child retrieved from xpath when it has attributes.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_6-10.txt2.14 KB
new 2986883-10.patch3.65 KB

Here's a couple more scenarios.

gnuget’s picture

gnuget
Primary language Spanish
commented
Status: Needs review » Reviewed & tested by the community

I found this issue today and #10 looks GREAT!

Thanks @heddn, @StefanPr and @rojan raj

gnuget’s picture

gnuget
Primary language Spanish
commented

I just noted that the SimpleXML plugin already do this check so even the comment explaining the change might be unnecessary.

https://git.drupalcode.org/project/migrate_plus/-/blob/8.x-5.x/src/Plugi...

  • heddn committed 40b1c91 on 8.x-5.x 
    Issue #2986883 by StefanPr, heddn, rojan raj: Xml data_parser reducing...
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Reviewed & tested by the community » Fixed

Thanks everyone for your contributions.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.