FHIR Coverage Resource Added #4164

vishnuyar · 2021-01-06T12:50:36Z

Short description of what this resolves:

FHIR Coverage Resource has been added

Changes proposed in this pull request:

Add Insurance Companies to the FHIR Organization Resources
Add Insurance Companies as Payor Reference in the Coverage FHIR Resource
When Searching for Organizations -- Will check in Both Facilities and Insurance Companies

Latest Dec 29

Latest

Latest Jan 4 2021

vishnuyar · 2021-01-06T12:52:21Z

These below alter table statements are required in the upgrade sql
ALTER TABLE insurance_companies ADD uuid binary(16) DEFAULT NULL;
ALTER TABLE insurance_data ADD uuid binary(16) DEFAULT NULL;

vishnuyar · 2021-01-06T12:55:13Z

While Creating the Coverage Resource, the status field is compulsory
I couldn't find a meaningful way to decipher the status of the Insurance based on the data in the table "insurance_data" data. Help on this requested.
One Possible solutions was check if the current date is less than "date" in the table. Is it a correct interpretation ?

stephenwaite · 2021-01-06T14:45:24Z

hi @vishnuyar , yes, that's correct, but if the current date is less than the date in the table then the insurance is not yet effective which is not that common in my experience. More common will be a null date since folks add them and often don't enter the effective date.

Easiest solution might be to grab the most recent insurance entered, and just talking about primary right now for simplicity and then use it if passes the current date test.

It might be nice to add a date to check when calling this function.

vishnuyar · 2021-01-06T17:12:22Z

@stephenwaite how will we know if a specific plan has expired

stephenwaite · 2021-01-06T18:14:37Z

only definitive is if another ins has been added that has a more recent effective date

sjpadgett · 2021-01-06T18:32:05Z

Hi @vishnuyar
I not sure the best way to handle what constitutes a Organization is inside the FHIR service.
Our API design concept is to extend this type of core database arbitration from a standard service. The reason for this is that many other current and future development relies on these types of service. In this case, Institutional development and upcoming HIE feature will rely on similar need for Organization.
So now would be the best time to do this as if we bring this PR into codebase as it is now, i'll have to rewrite.
How can I help you to do this?

vishnuyar · 2021-01-06T18:44:52Z

Looking at OpenEmr code, I assumed only Fhir is categorizing each of these entities as Organization with different types. For a similar thing in API, probably best to create a single table holding various Organizations, probably being populated via stored Procedure, nit even sure if that is a good idea.

…

On Thu, Jan 7, 2021, 00:02 Jerry Padgett ***@***.***> wrote: Hi @vishnuyar <https://github.com/vishnuyar> I not sure the best way to handle what constitutes a Organization is inside the FHIR service. Our API design concept is to extend this type of core database arbitration from a standard service. The reason for this is that many other current and future development relies on these types of service. In this case, Institutional development and upcoming HIE feature will rely on similar need for Organization. So now would be the best time to do this as if we bring this PR into codebase as it is now, i'll have to rewrite. How can I help you to do this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2HCSIT4T554UUQIBNG3NDSYSUDPANCNFSM4VXPXGJA> .

sjpadgett · 2021-01-06T18:55:20Z

It is true we currently don't use Organization per se. However, we have an ongoing project for institutional support where both Organization and Location are important. Since we are now needing this capabilities to support FHIR it has pushed up the timeline on adding these services.
I thought I addressed this on the issue thread on adding the organization table support via Practice UI. And I don't really like stored procedures as they are hard to maintain over time.
Though we can add the new table and I can help with that integration.

bradymiller · 2021-01-06T21:52:53Z

hi,
I'd rec just making a standard api organization service to do what you need, which the FHIR service can then call. This organization service will also be useful for the standard api. Since I'm lazy, I would also avoid any heavy lifting or database changes unless new datapoints need to be stored.

vishnuyar · 2021-01-07T08:40:46Z

@sjpadgett @bradymiller Based on both of your views, This is the solution i have implemented.

Create OrganizationService which is being used by FhirOrganizationService
OrganizationService in turn uses InsuranceService and FacilityService (This should mostly act as Read Service )
Any future additions such as lab organizations can also be integrated into the Organization Service

Keeping the unique nature of these organizations Separate API services can be used to write or update these services via API

This solution doesn't require any UI changes nor creating a new table.

Let me know your views. I am open to making further modifications. UI is not my forte, I will probably need help if the solution requires UI change.

stephenwaite · 2021-01-07T15:40:53Z

hi @vishnuyar , the new upgrade script is sql/6_0_0-to-6_1_0_upgrade.sql

vishnuyar · 2021-01-07T15:44:22Z

hi @vishnuyar , the new upgrade script is sql/6_0_0-to-6_1_0_upgrade.sql

I thought the Coverage Resoure will be included in version 6.0.0 too.
Should I move it to sql/6_0_0-to-6_1_0_upgrade.sql ?

bradymiller · 2021-01-08T08:24:28Z

src/Validators/InsuranceValidator.php

+use Particle\Validator\Validator;
+use Particle\Validator\Exception\InvalidValueException;
+use OpenEMR\Common\Uuid\UuidRegistry;
+use Ramsey\Uuid\Exception\InvalidUuidStringException;


alphabetize

Removed unused statements

src/Validators/InsuranceValidator.php

+ *
+ * @package   OpenEMR
+ * @link      http://www.open-emr.org
+ * @author    Vishnu Yarmaneni <vardhanvishnu@gmail.com>


bradymiller · 2021-01-08T08:42:54Z

Regarding the automated testing. I'll bring in a fix for the 8.1 syntax (#4168). It looks like the other fails are because a code change is breaking one test in PHP 7.4/8 testing environments:

Fhir Organization Rest Controller (OpenEMR\Tests\RestControllers\FHIR\FhirOrganizationRestController)
 ✔ Post
 ✔ Invalid post
 ✔ Patch
 ✔ Invalid patch
 ✘ Get one
   │
   │ Trying to access array offset on value of type bool
   │
   │ /usr/share/nginx/html/openemr/src/Validators/BaseValidator.php:115
   │ /usr/share/nginx/html/openemr/src/Services/InsuranceCompanyService.php:102
   │ /usr/share/nginx/html/openemr/src/Services/OrganizationService.php:36
   │ /usr/share/nginx/html/openemr/src/Services/FHIR/FhirOrganizationService.php:299
   │ /usr/share/nginx/html/openemr/src/RestControllers/FHIR/FhirOrganizationRestController.php:75
   │ /usr/share/nginx/html/openemr/tests/Tests/RestControllers/FHIR/FhirOrganizationRestControllerTest.php:96
   │

bradymiller · 2021-01-10T09:12:44Z

src/Services/FacilityService.php

@@ -353,12 +353,10 @@ public function getOne($uuid)
            "limit" => 1
        ));

-        if ($sqlResult) {


just checking that sqlResult is always populated with something? (or was this causing issues otherwise?)

Yes, this was the reason PHP7.4 and PHP8.0 were breaking.
Got it from here.
https://stackoverflow.com/questions/59674903/trying-to-access-array-offset-on-value-of-type-bool-in-php-7-4

in this case, would revert your changes here and do the following for the if:

if (!empty($sqlResult)) {

@vishnuyar , still need to address this issue

bradymiller · 2021-01-10T09:22:33Z

src/Services/InsuranceCompanyService.php

    }

-    public function getOne($id)
+    public function getOne($uuid)


migrate the old getOne function to getOneById (and change where it is called in the codebase; it is used 1 time in the codebase in receipts_by_method_report.php).

Added a new function getOneById() , changed the same in codebase in receipts_by_method_report.php and InsuranceCompanyRestController.php

It's also used in
interface/billing/sl_eob_process.php
I think everywhere else is covered but i'd search code base to double check me...

bradymiller · 2021-01-10T09:28:17Z

src/Services/InsuranceService.php

@@ -60,25 +79,116 @@ public function validate($data)
        return $validator->validate($data);
    }

-    public function getOne($pid, $type)
+    public function getOne($uuid, $type = 'primary')


as above strategy, migrate old getOne() function to getOneByPid() and change accordingly where it is called in the codebase. then you do not need to have hybrid functions that are doing very different things.

Added the function getOneByPid(), made the change in codebase receipts_by_method_report.php and InsuranceRestController.php

bradymiller · 2021-01-10T09:29:14Z

src/Services/InsuranceService.php

    }

-    public function getAll($pid)
+    public function getAll($search = array(), $isAndCondition = true)


note this function is not used in core codebase, so it does not need to support pid

_rest_routes.inc.php

+        RestConfig::authorization_check("admin", "super");
+        $return = (new FhirCoverageRestController())->getOne($uuid);
+        RestConfig::apiLog($return);
+        return $return;


bradymiller · 2021-01-10T09:32:57Z

review complete

bradymiller · 2021-01-10T19:36:30Z

src/Validators/BaseValidator.php

@@ -111,8 +111,11 @@ public static function validateId($field, $table, $lookupId, $isUuid = false)
            "SELECT $field FROM $table WHERE $field = ?",
            array($lookupId)
        );
-
-        return $result[$field] ? true : $validationResult;
+        if ($result) {


guessing this had to be changed for php8 issues. would change this if to:

if (!empty($result[$field])) {

bradymiller · 2021-01-10T21:55:04Z

src/RestControllers/InsuranceCompanyRestController.php

@@ -35,7 +35,7 @@ public function getAll()

    public function getOne($iid)
    {
-        $serviceResult = $this->insuranceCompanyService->getOne($iid);
+        $serviceResult = $this->insuranceCompanyService->getOneById($iid);


the rest controllers should not be changed (the rest endpoints will use uuid then)
Just need to ensure all the other placed in codebase that use the service are changed
(grep insuranceCompanyService and change the getOne to getOneById where used outside fhir/rest)

just checked and otherwise looks good in codebase (just need to change this back to getOne() )
And then need to update lines 1479 and 1484 in API_README.md to show use of uuid.

actually, that is a put. agree best thing to do here is to keep as is (changing to uuid for this restcontroller will require a bunch more more). so keep the getOneById call :)

bradymiller · 2021-01-10T21:55:12Z

src/RestControllers/InsuranceRestController.php

@@ -32,7 +32,7 @@ public function getAll($pid)

    public function getOne($pid, $type)
    {
-        $serviceResult = $this->insuranceService->getOne($pid, $type);
+        $serviceResult = $this->insuranceService->getOneByPid($pid, $type);


the rest controllers should not be changed (the rest endpoints will use uuid then)
Just need to ensure all the other placed in codebase that use the service are changed
(grep insuranceService and change the getOne to getOneByPid where used outside fhir/rest)

actually, this is more complicated since the endpoint takes in pid and type. let me look into that to see best strategy.

TODO for me: see above

ok, lets also keep this as is (getOneByPid). And just need to change one more instance of getOne (to getOneByPid) use in the interface/billing/sl_eob_process.php

Changed the functionname in interface/billing/sl_eob_process.php

FHIR_README.md

@@ -44,6 +44,7 @@ Database Result -> Service Component -> FHIR Service Component -> Parse OpenEMR
 -   [FHIR API Endpoints](FHIR_README.md#fhir-endpoints)
    -   [Capability Statement](FHIR_README.md#capability-statement)
    -   [Patient](FHIR_README.md#patient-resource)
+    -   [Coverage](FHIR_README.md#coverage-resource)


bradymiller · 2021-01-11T00:28:26Z

review complete. just some minor things to fix.

bradymiller · 2021-01-12T08:05:42Z

API_README.md

@@ -162,6 +163,7 @@ This is a listing of scopes:
 - `api:fhir` (user fhir which are the /fhir/ endpoints)
  - `user/AllergyIntolerance.read`
  - `user/CareTeam.read`
+  - `user/Coverage.read`


i'd usually let this go, but since there's another minor change to do, might as well alphabetize (place below Condition) this as well :)

Hoping that my next PR will have less of these kind of mistakes :)

bradymiller · 2021-01-12T08:06:15Z

2 more very minor things to address and then this PR is ready :)

bradymiller · 2021-01-12T22:05:08Z

hi @vishnuyar , Code looks great and bringing in. Thanks for the improvements! -brady

vishnuyar and others added 4 commits December 29, 2020 11:35

Merge pull request #1 from openemr/master

6749f87

Latest Dec 29

Merge pull request #2 from openemr/master

63e04de

Latest

Merge pull request #3 from openemr/master

bd3dd1b

Latest Jan 4 2021

FHIR Coverage Resource added and associated changes

e874a33

style fixes

9338f43

bradymiller added the Status: Needs Review label Jan 6, 2021

bradymiller added the Status: Needs Work label Jan 6, 2021

vishnuyar added 2 commits January 7, 2021 13:48

add OrganizationService API

3ac2602

add uuid column to insurance_companies and insurance_data

12cdce5

vishnuyar added 8 commits January 7, 2021 14:16

PSR12 fixes

7b909af

add insert update to Organization Service

3de6432

add FHIR Coverage route

6ab5e8c

add orgtype while parsing resource

5cb31e3

bug fix

d475121

remove orgType

8f50fc2

fixed handling errors

bffd64d

add validation messages

a38095b

modular functions

6dbd6cd

bradymiller reviewed Jan 8, 2021

View reviewed changes

src/Validators/InsuranceValidator.php

*

* @package OpenEMR

* @link http://www.open-emr.org

* @author Vishnu Yarmaneni <vardhanvishnu@gmail.com>

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

vishnuyar added 5 commits January 8, 2021 14:34

fixed review suggestions

b5e752e

bug fix for php7.4+

752d8a8

bug fix for php7.4

b3daabb

bug fix for php7.4

ae7f9b9

bug fix for php7.4

9815306

bradymiller reviewed Jan 10, 2021

View reviewed changes

_rest_routes.inc.php

RestConfig::authorization_check("admin", "super");

$return = (new FhirCoverageRestController())->getOne($uuid);

RestConfig::apiLog($return);

return $return;

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

vishnuyar added 2 commits January 10, 2021 22:57

add getOneById and getOneByPid functions

f3afe10

styling fix

dc35de7

bradymiller reviewed Jan 10, 2021

View reviewed changes

bradymiller reviewed Jan 11, 2021

View reviewed changes

vishnuyar added 2 commits January 11, 2021 10:39

some more fixes

e4370f9

add insurance organization identifier

d958941

bradymiller reviewed Jan 12, 2021

View reviewed changes

vishnuyar added 2 commits January 12, 2021 13:40

checking for empty results fix

344d402

alphatize Coverage

5f9ba95

bradymiller merged commit bc97754 into openemr:master Jan 12, 2021

vishnuyar deleted the coverage branch January 14, 2021 17:57

FHIR Coverage Resource Added #4164

FHIR Coverage Resource Added #4164

Conversation

vishnuyar commented Jan 6, 2021

Short description of what this resolves:

Changes proposed in this pull request:

vishnuyar commented Jan 6, 2021

vishnuyar commented Jan 6, 2021

stephenwaite commented Jan 6, 2021 • edited Loading

vishnuyar commented Jan 6, 2021

stephenwaite commented Jan 6, 2021

sjpadgett commented Jan 6, 2021

vishnuyar commented Jan 6, 2021 via email

sjpadgett commented Jan 6, 2021

bradymiller commented Jan 6, 2021

vishnuyar commented Jan 7, 2021 • edited Loading

stephenwaite commented Jan 7, 2021

vishnuyar commented Jan 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

bradymiller commented Jan 8, 2021 • edited Loading

bradymiller Jan 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

bradymiller commented Jan 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradymiller Jan 10, 2021 • edited Loading

Choose a reason for hiding this comment

bradymiller Jan 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradymiller Jan 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

bradymiller commented Jan 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradymiller commented Jan 12, 2021

bradymiller commented Jan 12, 2021

stephenwaite commented Jan 6, 2021 •

edited

Loading

vishnuyar commented Jan 7, 2021 •

edited

Loading

bradymiller commented Jan 8, 2021 •

edited

Loading

bradymiller Jan 10, 2021 •

edited

Loading

bradymiller Jan 10, 2021 •

edited

Loading

bradymiller Jan 11, 2021 •

edited

Loading

bradymiller Jan 10, 2021 •

edited

Loading