-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New external MDHT validation #5448
Conversation
- add funtionalliy to validation service. - fix referral from direct to be empty - new globals for validation server
I plan to merge as quickly as possible so @adunsulag can have for his development. |
'mdht_conformance_server' => array( | ||
xl('CCDA MDHT Validation API Server Address'), | ||
'text', // data type | ||
'http://ccda.healthit.gov', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we assume the file will always have the /referenceccdaservice/ path? Or would it be better to specify the full server address here? That leaves the option for people to customize / change this if they never need it to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all arguments are specific to this server.
$this->externalValidatorEnabled = !empty($GLOBALS['mdht_conformance_server_enable'] ?? false); | ||
$this->externalValidatorUrl = null; | ||
if ($this->externalValidatorEnabled) { | ||
$this->externalValidatorUrl = trim($GLOBALS['mdht_conformance_server'] ?? null) ?: 'http://ccda.healthit.gov'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does trim work on a null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
$result = $this->ettValidateDocumentRequest($xml); | ||
} catch (Exception $e) { | ||
$e = $e->getMessage(); | ||
error_log($e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this to (new SystemLogger())->errorLogCaller($e->getMessage(), ["trace" => $e->getTraceAsString()]); This will escape any user data that could have been passed back from the exception since you are doing a web request here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sjpadgett Just noted a couple things, one minor security fix with the error_log. Other than that it looks great!
Looks good to me, feel free to bring it when you're ready @sjpadgett. |
thanks for the review Stephen. |
Hitting the public server is kind of slow so you may want to try against my server Stephen if it seems tedious to you. I'll send the web address in email. |
Fixes #5447