-
-
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
feat: openemr 6752 6751 address book user rest api #6767
feat: openemr 6752 6751 address book user rest api #6767
Conversation
Allows an address book config data type to be added for the global config. Fixes openemr#6752
Adds an endpoint for retrieving users from the system. If there are users we wish to retrieve that are not practitioners (such as external entities that are in the address book), we can't use the practitioner api. Instead we expose the users endpoint. Fixes openemr#6751
interface/super/edit_globals.php
Outdated
@@ -373,8 +373,13 @@ function checkBackgroundServices() | |||
'help_file_name' => "" | |||
); | |||
$oemr_ui = new OemrUI($arrOeUiSettings); | |||
$serverConfig = new \OpenEMR\FHIR\Config\ServerConfig(); |
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.
would place this in use clause at top script
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.
Fixed
$textLabel = ""; | ||
$i = $i ?? 0; | ||
if (!empty($fldvalue)) { | ||
$users = \OpenEMR\Common\Database\QueryUtils::fetchRecords("SELECT fname, lname FROM users WHERE id = ?", [$fldvalue]); |
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.
use clause
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.
Fixed.
$user = $users[0] ?? ['fname' => '', 'lname' => '']; | ||
$textLabel = trim($user['fname'] . ' ' . $user['lname']); | ||
} | ||
$serverConfig = new \OpenEMR\FHIR\Config\ServerConfig(); |
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.
use clause
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.
Fixed
<div class="gbl-field-address-book-widget"> | ||
<input type='hidden' class="address-book-widget-input" name='form_<?php echo attr($i); ?>' id='form_<?php echo attr($i); ?>' value='<?php echo attr($fldvalue); ?>' /> | ||
<div class="input-group"> | ||
<input type='text' class="address-book-widget-label form-control" readonly="readonly" id='form_label_<?php echo attr($i); ?>' value='<?php echo text($textLabel); ?>'/> |
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.
attr($textLabel)
(not text())
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.
Fixed.
interface/super/edit_globals.php
Outdated
@@ -373,8 +373,13 @@ function checkBackgroundServices() | |||
'help_file_name' => "" | |||
); | |||
$oemr_ui = new OemrUI($arrOeUiSettings); | |||
$serverConfig = new \OpenEMR\FHIR\Config\ServerConfig(); | |||
$apiUrl = $serverConfig->getBaseApiUrl(); |
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.
getting the full url including server may not always work if the global is not set correctly. Can't we just use the relative path (ie. webroot + path) here like we normally do when calling scripts (then don't need to worry if the global is set correctly)?
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 think the issue here is that I'm hitting the API endpoints which all require the oauth global to be setup. All of our apis utilize that oauth global and if anyone has done any kind of proxy on the api and setup the url to be different than the local relative server url then we'd have problems hitting the api. Since we have a global that could be different then the local setup, I need to make sure if I'm hitting the api that we are hitting the configured endpoint for the api.
I'd rather that we didn't have the global at all, but since we have the global for the api I think we need to use it when communicating with the api.
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.
Resolved by using local api.
After conversations with admins we decided to go with a relative api url for the internal api usage instead of the oauth2 full path. Also fixed a path issue.
Bringing this in as all concerns have been addressed. |
Fixes #6752
Fixes #6751
Allows an address book config data type to be added for the global config.
Adds an endpoint for retrieving users from the system. If there are users we wish to retrieve that are not practitioners (such as external entities that are in the address book), we can't use the practitioner api. Instead we expose the users endpoint.