dcarbone / php-fhir

Tools for consuming data from a FHIR server with PHP
Apache License 2.0
129 stars 40 forks source link

Extra "value" child elements in 1.0.1 #53

Closed mmcev106 closed 4 years ago

mmcev106 commented 5 years ago

The status of a FHIRComposition was previously JSON encoded as follows:

"status": "preliminary"

In 1.0.1 this changed to the following, which does not follow the R4 spec:

"status": {
    "value": "preliminary"
}

Another example is system under FHIRContactPoint.

I'm hoping to make time to work on a fix for this myself, but that could take a while so I wanted to go ahead and report it.

dcarbone commented 5 years ago

This and #28 stem from the same source. I need to implement a system that identifies "value container" elements properly, such that their value is output as attributes / direct values to their containing object's field / element, but also be able to output the other data present on every element within FHIR.

This release introduced the ability to parse the _{field} input from JSON, which was missing before, but I have yet to finish the logic that will enable output in the same fasion.

I have some very basic logic in there around this, but it is not complete. Will work on it some more this weekend.

dcarbone commented 5 years ago

@mmcev106: i have finished an update to the generator that I believe takes care of this bug. I will create a new release once tests have passed, most likely later this evening.

dcarbone commented 5 years ago

@mmcev106: Could you give v1.0.5 a try and see if the output looks as you would expect?

mmcev106 commented 5 years ago

It looks like 1.0.5 reverts back to a previous behavior where the value elements are correct, but other elements are missing. The latest revision of the following gist shows missing system values being manually added make a Bundle I'm working on valid:

https://gist.github.com/mmcev106/b80ab4b70063ec53b09e5503d755192a/revisions

Let me know if I can provide any more info, or if you know the solution but don't have time to implement it.

dcarbone commented 5 years ago

@mmcev106: I'm not sure I understand, the provided bundle has no "system" keys in it. Where are you deriving this value from?

mmcev106 commented 5 years ago

The two missing system values in the example are under the following resources within the Bundle:

https://www.hl7.org/fhir/datatypes.html#Identifier https://www.hl7.org/fhir/datatypes.html#ContactPoint (under telecom in the export)

I just did a little more testing and serializing the FHRIdentifier and FHIRContactPoint objects directly includes the system value correctly. I think the problem might be limited to serializing "sub-resources" that have a value (but not only a value).

mmcev106 commented 5 years ago

I can come up with a succinct example if that's still not quite clear.

dcarbone commented 5 years ago

@mmcev106 hmm maybe i'm missing something here. is that bundle present in your gist generated by my code or is that one you provided?

If its generated by my bundle, then that's something I can look at.

If its something you provided, then as i said above: there are no system keys in that bundle:

as an example from your gist:

{
                "contact": [
                    {
                        "name": {
                            "family": "Smith",
                            "given": [
                                "Jane"
                            ]
                        },
                        "telecom": [
                            {
                                "value": "jane@smith@test.edu"
                            }
                        ]
                    },
                    {
                        "name": {
                            "family": "Dean",
                            "given": [
                                "Jimmy"
                            ]
                        },
                        "telecom": [
                            {
                                "value": "Jim.Dean@sausage.net"
                            }
                        ]
                    }
                ]
}

Either way, I'll have a look at it tonight to make sure its working as expected.

dcarbone commented 5 years ago

Ah nevermind, i see the updated bundle. Apologies, was looking at an outdated version.

dcarbone commented 5 years ago

@mmcev106: so in my quick testing, it is being output as such:

{
                "identifier": [
                    {
                        "value": "127-1"
                    }
                ],
                "_identifier": [
                    {
                        "system": "http:\/\/localhost\/"
                    }
                ]
}

This is definitely a bug, I will work on something tonight.

dcarbone commented 5 years ago
{
    "resourceType": "Bundle",
    "entry": [
        {
            "resource": {
                "resourceType": "Composition",
                "author": [
                    {
                        "reference": "PractitionerRole\/130-1"
                    }
                ],
                "confidentiality": "L",
                "date": "2019-11-19T14:02:19-06:00",
                "section": [
                    {
                        "id": "title",
                        "text": {
                            "div": "<div xmlns=\"http:\/\/www.w3.org\/1999\/xhtml\">\r\n    <p>\r\n        <img src=\"pic_IREX.jpg\" alt=\"IREX\"\/>\r\n    <\/p>\r\n    <p style=\"text-align:center;text-decoration: underline;\"> Official Documentation of Reliance<\/p>\r\n    <p>Nov 19, 2018<\/p>\r\n<\/div>",
                            "status": "generated"
                        }
                    },
                    {
                        "id": "information",
                        "entry": [
                            {
                                "reference": "Organization\/128-2"
                            },
                            {
                                "reference": "ResearchStudy\/127-1"
                            },
                            {
                                "reference": "PractitionerRole\/130-1"
                            }
                        ],
                        "text": {
                            "div": "\r\n<div xmlns=\"http:\/\/www.w3.org\/1999\/xhtml\">\r\n    <p>This letter serves as documentation that \r\n        \r\n        <strong>Test University 2<\/strong> has agreed to rely on the\r\n        <strong>Test University 1<\/strong> IRB using the SMART IRB Master Common Reciprocal Institutional Review Board Authorization Agreement.\r\n    <\/p>\r\n    <p>\r\n        <strong>NOTE: This is not a notice of IRB approval. A separate email will be sent when the study is approved by the reviewing IRB.<\/strong>\r\n    <\/p>\r\n    <p>\r\n        <strong>Study Title:<\/strong> Test Study    <\/p>\r\n    <p>\r\n        <strong>Relying Site PI:<\/strong> Miles Davis    <\/p>\r\n<\/div>",
                            "status": "generated"
                        }
                    },
                    {
                        "id": "close",
                        "entry": [
                            {
                                "reference": "ResearchStudy\/127-1"
                            }
                        ],
                        "text": {
                            "div": "\r\n<div xmlns=\"http:\/\/www.w3.org\/1999\/xhtml\">\r\n    <p style=\"font-style: italic;\">This is a Trial Innovation Network study.<\/p>\r\n    <p>Information about this study is available online at: \r\n        \r\n        <br\/>\r\n        <a href=\"https:\/\/www.irbexchange.org\/study\/index\/?proj=127-1\">https:\/\/www.irbexchange.org\/study\/index\/?proj=127-1<\/a>\r\n    <\/p>\r\n    <p>Sincerely,<\/p>\r\n    <p>The IREx Team<\/p>\r\n<\/div>",
                            "status": "generated"
                        }
                    }
                ],
                "status": "preliminary",
                "subject": {
                    "reference": "ResearchStudy\/127-1"
                },
                "title": "Determination Letter",
                "type": {
                    "text": "Determination Letter"
                }
            }
        },
        {
            "resource": {
                "resourceType": "Organization",
                "id": "128-1",
                "contact": [
                    {
                        "name": {
                            "family": "Smith",
                            "given": [
                                "Jane"
                            ],
                            "_given": [
                                null
                            ]
                        },
                        "telecom": [
                            {
                                "system": "email",
                                "value": "jane@smith@test.edu"
                            }
                        ]
                    },
                    {
                        "name": {
                            "family": "Dean",
                            "given": [
                                "Jimmy"
                            ],
                            "_given": [
                                null
                            ]
                        },
                        "telecom": [
                            {
                                "system": "email",
                                "value": "Jim.Dean@sausage.net"
                            }
                        ]
                    }
                ],
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "128-1"
                    }
                ],
                "name": "Test University 1",
                "type": [
                    {
                        "coding": [
                            {
                                "code": "prov"
                            }
                        ]
                    }
                ]
            }
        },
        {
            "resource": {
                "resourceType": "Practitioner",
                "id": "126-1",
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "126-1"
                    }
                ],
                "name": [
                    {
                        "family": "Denver",
                        "given": [
                            "John"
                        ],
                        "_given": [
                            null
                        ]
                    }
                ],
                "telecom": [
                    {
                        "system": "email",
                        "value": "john@denver.com"
                    }
                ]
            }
        },
        {
            "resource": {
                "resourceType": "ResearchStudy",
                "id": "127-1",
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "127-1"
                    }
                ],
                "principalInvestigator": {
                    "reference": "Practitioner\/126-1"
                },
                "sponsor": {
                    "reference": "Organization\/128-1"
                },
                "status": "in-review",
                "title": "Test Study"
            }
        },
        {
            "resource": {
                "resourceType": "Organization",
                "id": "128-2",
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "128-2"
                    }
                ],
                "name": "Test University 2",
                "type": [
                    {
                        "coding": [
                            {
                                "code": "prov"
                            }
                        ]
                    }
                ]
            }
        },
        {
            "resource": {
                "resourceType": "Practitioner",
                "id": "126-2",
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "126-2"
                    }
                ],
                "name": [
                    {
                        "family": "Davis",
                        "given": [
                            "Miles"
                        ],
                        "_given": [
                            null
                        ]
                    }
                ],
                "telecom": [
                    {
                        "system": "email",
                        "value": "miles.davis@trumpet.org"
                    }
                ]
            }
        },
        {
            "resource": {
                "resourceType": "PractitionerRole",
                "id": "130-1",
                "code": [
                    {
                        "coding": [
                            {
                                "display": "Site Investigator"
                            }
                        ]
                    }
                ],
                "identifier": [
                    {
                        "system": "http:\/\/localhost\/",
                        "value": "130-1"
                    }
                ],
                "organization": {
                    "reference": "Organization\/128-2"
                },
                "practitioner": {
                    "reference": "Practitioner\/126-2"
                }
            }
        }
    ],
    "identifier": {
        "system": "http:\/\/localhost\/",
        "value": "125-1"
    },
    "timestamp": "2019-11-19T14:02:19-06:00",
    "type": "document"
}
dcarbone commented 5 years ago

I think I've got it, will push a new build and let tests happen

dcarbone commented 5 years ago

@mmcev106: apologies, this will take a bit longer than I had hoped. I made some assumptions early in the rewrite of this lib that are biting me now, will hopefully have something either today or later this week.

mmcev106 commented 5 years ago

Even just responses to questions are much appreciated, so please don't feel rushed. If it's anything you'd like to pass off, I'd likely get to it eventually.

dcarbone commented 4 years ago

@mmcev106: alright, I have release v2.0.0 which, i believe, finally handles these sorts of issues. I went with a major release vs a minor as I also refactored the generation a bit so -list type classes are now underneath the primitive they extend in the namespace hierarchy.

Let me know if you have any issues with this version, and thank you again for being patient and helping me test this!

mmcev106 commented 4 years ago

I just tested v2.0.2 and it seems to match the spec perfectly in our use cases. Thank you very much!

dcarbone commented 4 years ago

@mmcev106 excellent! I have a new version in the works that further improves upon this issue, but i'm running into bizarre issues with the hapi fhir endpoint returning seemingly malformed json bodies.

haven't had a chance to dig in much yet. will hopefully have an improved release this weekend that includes some basic validation and fhir_comment output when serializing.