{"action":"created","issue":{"url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/247","repository_url":"https://api.github.com/repos/Islandora/islandora_xml_forms","labels_url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/247/labels{/name}","comments_url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/247/comments","events_url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/247/events","html_url":"https://github.com/Islandora/islandora_xml_forms/pull/247","id":302724274,"number":247,"title":"ISLANDORA-2140: Prevent garbled MODS when going back and forth in ingest forms","user":{"login":"d-r-p","id":22350048,"avatar_url":"https://avatars1.githubusercontent.com/u/22350048?v=4","gravatar_id":"","url":"https://api.github.com/users/d-r-p","html_url":"https://github.com/d-r-p","followers_url":"https://api.github.com/users/d-r-p/followers","following_url":"https://api.github.com/users/d-r-p/following{/other_user}","gists_url":"https://api.github.com/users/d-r-p/gists{/gist_id}","starred_url":"https://api.github.com/users/d-r-p/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/d-r-p/subscriptions","organizations_url":"https://api.github.com/users/d-r-p/orgs","repos_url":"https://api.github.com/users/d-r-p/repos","events_url":"https://api.github.com/users/d-r-p/events{/privacy}","received_events_url":"https://api.github.com/users/d-r-p/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-03-06T14:34:08Z","updated_at":"2018-03-06T15:06:25Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/Islandora/islandora_xml_forms/pulls/247","html_url":"https://github.com/Islandora/islandora_xml_forms/pull/247","diff_url":"https://github.com/Islandora/islandora_xml_forms/pull/247.diff","patch_url":"https://github.com/Islandora/islandora_xml_forms/pull/247.patch"},"body":"**JIRA Ticket**: https://jira.duraspace.org/browse/ISLANDORA-2140\r\n\r\n# What does this Pull Request do?\r\n\r\nAddress [ISLANDORA-2140](https://jira.duraspace.org/browse/ISLANDORA-2140), i.e. prevent garbled MODS when going back and forth in ingest forms.\r\n\r\nIn multi-paged ingest forms, when the user returns to the metadata form and re-submits, the original code swallows certain XML sub-elements inside the resulting MODS. More precisely, whenever an XML fragment of the form `<element><sub-element>%value%</sub-element></element>` is created, going back to and re-submitting the metadata form results in a MODS containing only `<element>the inserted value</element>`. This breaks the MODS standard, along with subsequent logic such as CSL-driven metadata views.\r\n\r\nThe proposed solution fixes this behaviour.\r\n\r\n# What's new?\r\n\r\nWhen creating an XML fragment:\r\n* adjust the DOMNode to register in the [NodeRegistry](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/NodeRegistry.inc) of the handled [XMLDocument](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/XMLDocument.inc)\r\n* update the FormElement's value with the [Read](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/Read.inc)-value of the DOMNode just created inside the DOMDocument\r\n\r\nIf, during ingest, the user goes back to the metadata form, those adjustments allow the [Update action](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/Update.inc) triggered upon re-submission to work correctly.\r\n\r\n# How should this be tested?\r\n\r\n* Convince yourself of the issue by following the ten steps described in the [JIRA ticket](https://jira.duraspace.org/browse/ISLANDORA-2140)\r\n* Apply the diff and assess that the problem has disappeared\r\n* Test other ingest forms (content models)\r\n\r\nOf course, do not test directly on a production environment!\r\n\r\n_Note_: The modifications change the [NodeRegistry](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/NodeRegistry.inc) built during DOMNode creation ([Create action](https://github.com/Islandora/islandora_xml_forms/blob/7.x/api/Create.inc)) in a substantial manner. Therefore, **regression testing** might be advisable.\r\n\r\n# Additional Notes:\r\n* Does this change require documentation to be updated? **no**\r\n* Does this change add any new dependencies? **no**\r\n* Does this change require any other modifications to be made to the repository (ie. Regeneration activity, etc.)? **no**\r\n* Could this change impact execution of existing code? **possibly**\r\n\r\n# Interested parties\r\n@DiegoPino, @Islandora/7-x-1-x-committers\r\n"},"comment":{"url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/comments/370811195","html_url":"https://github.com/Islandora/islandora_xml_forms/pull/247#issuecomment-370811195","issue_url":"https://api.github.com/repos/Islandora/islandora_xml_forms/issues/247","id":370811195,"user":{"login":"DiegoPino","id":6946023,"avatar_url":"https://avatars2.githubusercontent.com/u/6946023?v=4","gravatar_id":"","url":"https://api.github.com/users/DiegoPino","html_url":"https://github.com/DiegoPino","followers_url":"https://api.github.com/users/DiegoPino/followers","following_url":"https://api.github.com/users/DiegoPino/following{/other_user}","gists_url":"https://api.github.com/users/DiegoPino/gists{/gist_id}","starred_url":"https://api.github.com/users/DiegoPino/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DiegoPino/subscriptions","organizations_url":"https://api.github.com/users/DiegoPino/orgs","repos_url":"https://api.github.com/users/DiegoPino/repos","events_url":"https://api.github.com/users/DiegoPino/events{/privacy}","received_events_url":"https://api.github.com/users/DiegoPino/received_events","type":"User","site_admin":false},"created_at":"2018-03-06T15:06:25Z","updated_at":"2018-03-06T15:06:25Z","author_association":"CONTRIBUTOR","body":"@d-r-p cool, code is nice!! still, need to understand [what is happening there and your suggested solution](https://jira.duraspace.org/browse/ISLANDORA-2140?focusedCommentId=58316&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-58316). Will ask you a few questions once I get to test this. \r\nFor now, and forgive my ignorance on the approach, i'm not sure if `self` as xpath concept is refering actually to \"what a field created\" or to the \"field itself\". It is tricky since this form fields can create single nodes of course but also complex trees. Which means if the field (or a human through the field really) is adding a complex, deep nested snipped of XML, `self`, still refers to the \"parents node value\"... but since its a node, `self::node` would be more true to XML xpath realuty ==  'der hammer'.? Also the reason (not only because i have no life) i did not keep doing this and got a bit lost. Let me test this and come back to you this week. We have 7.x committers call and i can get some extra opinions. Committers call is on Thursday at 2PM EST in case you would like to explain this to others in digital-person. Thanks a lot. "}}
{"actor":{"display_login":"DiegoPino"}}