Changing default "<response>" and "<item>" tags in XML export

changes to improve functionality and code:

• I needed some more elements inside the root node, so I introduced an additional config field called "metadata" as a textarea
• also, I replaced the custom replacement of {{ lang }} by allowing the use of token in the attributes and metadata fields
• additionally I switched to dep. inj. where possible

still left to do:

• instead of strpos and str_replace, switch to a better way of manipulating the XML (i.e. using DOMDocument)

The code in #47 and in the MR need to be reconciled. It would also be great to add a batch export test for these custom xml tags.

The #47 has one issue.
If you open the settings of the data export format with enabled warning logging ($config['system.logging']['error_level'] = 'verbose';) you will get a warning "Warning: Undefined array key "metadata" in /var/www/html/web/modules/contrib/views_data_export/src/Plugin/views/style/DataExport.php on line 241".

I have updated the #47 patch to fix this warning.

Thank you very much the path #50 work well!

how to add attributes to Item node name?
can this be done via the API?

#50 works also.

I would suggest to move the "Item wrapper node name" before "Item node name" configuration field.

Hello,

I am not seeing the drop down info for the xml settings. however, I do see it for the CSV settings.

#50 works

The XML configuration worked in my environment

#50 works +1

This looks great, and thanks for the hard work everyone, but this sort of change really should have some tests to make sure we're not breaking things for existing sites, and that the new settings work correctly etc. so setting back to needs work.

joelpittet → changed the visibility of the branch 2886357-changing-default-response to hidden.

joelpittet → changed the visibility of the branch 2886357-changing-default-response to active.

joelpittet → changed the visibility of the branch 8.x-1.x to hidden.

@steven jones Thanks for the feedback. Reading between the lines, it sounds like the minimum needed here might be a test confirming that the XML response remains unchanged with the new feature settings unchanged. That would help ensure existing sites aren’t affected. Does that sound like the right approach? (Noting there are currently no XML output tests.)

@steven jones I added a test as I suggested in #62 plus what it should look like with this change as well. It's super bare bones but maybe give you the confidence to commit?

Couple of notes:

• the new options weren't defaulted so I added some defaults unioned on to prevent
  

  Exception: Warning: Undefined array key "metadata"

  issues from showing up after commit.

• The previous version of this didn't have encoding="utf-8" so my test would likely fail in a test-only run. Though likely a good thing to have on XML so I didn't remove it from this MR.

Hiding patches to avoid confusion.

I reviewed the latest changes in the MR and tested them locally. Everything works as expected. The changes are minor, focused on code improvements, and include appropriate test coverage. I did not encounter any regressions or issues introduced by this patch.

Regarding the CSV and XML collapsible panes: when I expand these panes, collapse them, and then expand them again, they appear shortened as if they did not expand because only a portion of the content is immediately visible. The remaining content requires scrolling to view, which can be misleading. This seems to be a UX quirk, not caused by this MR, and likely a general issue in Drupal core.

Marking this MR RTBC. Thanks for the improvements and for adding test coverage.

Thanks @joelpittet for the work on this, and thanks everyone else for your continued patience with Views Data Export maintership!

Okay, so the code as it stands does have a few issues, specifically because of using string replacements to do some of the heavy lifting rather than an XML library. This reminds me of the famous stack overflow answer about parsing HTML with regex.

For example, at the moment I can set both the 'root node name' and 'item node name' to say 'response' and then use the new 'XML metadata field to add some more stuff in, and all that stuff actually gets repeated throughout the file, not only after the root node, as is the intention of the code.

I think there are a lot of sticky issues that we might need to sort out with this stuff, like the fact that it seems weird that you can inject arbirary non-valid XML into the middle of the XML document anyway. Seems like this is going to be super brittle going forward if this doesn't get done right.

I appreciate that the current batched export code also does string replacements, but I think that's another bug waiting to happen. Presumably you can't have a field called 'response' in a batch XML export, yes in fact, that breaks very badly at the moment!

I think what I'd like to do is:

1. Restrict the scope of this issue to the original report only, i.e. allowing someone to change the <response> and <item> tags.
2. Work on getting a test in for that functionality, which should be pretty easy tbh.
3. Get just those points committed and mark this ticket as done. This will give us a proper place to put options, the beginnings of the code that's currently in the MR anyway and add in custom handling for XML where we'll need it in the future anyway.
4. Replace out the current string manipulation of XML in the batched export with proper XML manipulations. This can be a new ticket that could be worked on separately.
5. Raise separate tickets for the other features introduced in this ticket, which is:
   • Removing the root node entirely (presumably only valid to do if there's only a single child node anyway.)
   • Adding attributes to the root node - This doesn't need to be done as a single text field, as the Symfony serializer actually has built-in support for doing these already, so we could provide nice escaping etc. and not do broken plain string swaps.
   • XML Header (the metadata) This seems like a bit of a weird one to me, but I understand that there might be some use-cases out there that would want to add some arbitrary XML to the root node of the file. Again this should get added via the XML encoder if possible, but maybe having big warning messages all over this entry field that you're likely to break things unless you know exactly what you're doing, then maybe it's okay.
   • Removing empty values from the output.
   • Adding an extra wrapper with an arbitrary name.
6. Encourage others to work on those tickets/get them committed in a timely way.

Sorry if that sounds super annoying / convoluted, but I think rather than hold up some of these features because they aren't all ready, it makes sense to split them out.

I'll make the other tickets in moment, and we can refocus the work here to be back to the original issue.

Sort of tempted to remove the cheeky addition of the encoding="utf-8" bit, and then the changes here really will be absolutely minimal and could get merged right in.

I recommend keeping encoding="UTF-8" in there — it ensures correct character decoding, avoids cross-platform inconsistencies, and even if XML parsers are required to assume UTF-8 by default, it’s still best practice to include it explicitly.

@joelpittet yeah, agreed, I think it should almost always be in there, but I don't want to break something random in someone's export, so I think I'll punt that change to ✨ Allow specifying the XML encoding property Active and maybe we can make it the default for new installs or something like that.

You happy with the code otherwise?

Thanks everyone for your patience, I'll note that I'm committing a partial version of what was in MR!75, so if you're using all the features of this MR/patch then you might want to stick with that until all the sub issues of ✨ [meta]Make XML output more configurable Active are in, and then upgrade to the version of VDE that has those in.

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