Skip to content
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

Fix calendar schedule facilities #1137

Merged
merged 1 commit into from Jun 13, 2018

Conversation

apooravc
Copy link
Contributor

@apooravc apooravc commented May 22, 2018

@teryhill Attempted a fix for schedule facilities, please have a look.

There is a global $GLOBALS['restrict_user_facility'] in features tab in Admin which when true restricts Facilities in calendar event panel to only selected schedule facilities set up in Edit User for selected(clicked) user(provider) in calendar. If the global is true, and user selects facility 1 & facility 2 in Schedule Facilities and facility 3 in Default Facility in Edit User, then after saving changes, only facility 1, facility 2 & facility 3 would show up in Facility in calendar event panel for that user(provider). Otherwise, when global is false, all facilities which have non-zero service_location, i.e., facilities which have service location checked will show up in calendar for that user.

Default facility comes in schedule facilities and all schedule facilities are shown as selected with grey background. users_facility table stores id (in facility_id) of all schedule facilities corresponding to all users (table_id is user's id) and is always synchronized with what is shown as selected in Schedule Facilities box in Edit User. getUserFacilities(user_id) is used to get id of schedule facilities of a specific user when global is true otherwise id of all facilities having service location checked when global is false.

So, workflow of Schedule Facilities & Calendar is:

  1. Schedule Facilities box displays all facilities having service location checked in white with schedule ones in grey as selected in code.
    Note: When global is true then only this will happen otherwise all will be in grey.
  2. When user selects some more facilities as schedule ones and saves, they are carried by schedule_facilities[] to get inserted in table users_facility.
  3. Reopening Edit User for same user will now show those facilities also in grey by using getUserFacilities(user_id) from users_facility table to update Schedule Facilities box.
  4. In Calendar event panel for that user(provider), only these schedule facilities + default facility will show up in Facility using getUserFacilities(user_id) from users_facility table when global is true otherwise all facilities having location service checked (not 0) will show up.

//then list only schedule facilities of that user or default facility
//in Facility in calendar's event panel
//e2f denotes id of default facility
if (in_array($facrow['id'], $ufid) || $facrow['id'] == $e2f || $facrow['id'] == 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id 3 is the default facility, yes, which is generally edited, but the "default facility" should be the users default, right? Generally facilities.id == 3 is a facility that gets edited (name changed), but there is no guarantee that a user will have access to it, or that it will be the primary business entity, or any of that.

-example use of these restrictions, and thus the main goal here:

  1. There is a central office. Folks here generally have access to everything.
  2. There are "nursing homes" or "assisted living facilities" or other locations that either have provider staff (nurses, home care person, etc...) that should only every be able to see the patients at that facility. There may be a hundred or more of these satellite facilities.
  3. Some of the providers see patients at more than one facility. There may not be any particular apparent rhyme or reason to the combinations.

Use of this in combination with TAGS:

  1. in patient_data, a new field 'facility` is added. This is a dropdown listing all the facilities. It is (should be) set as a required field for this purpose (configuring required fields is another area of concerns). This is a drop-down list of facilities, which should only list facilities that the USER has access to.
  2. A TAG is created, wherein a user has restricted access, then allowed access based the value in patient_data.facility or on a patient tag that does pretty much the same thing. This actually becomes a set of tags that deny, then allow. Insert examples follow:
Facility ID Tag, Deny Tag, Allow Tag

INSERT INTO `tf_patients_tags` (`id`, `tag_id`, `pid`, `created_at`, `created_by`, `updated_at`, `updated_by`, `status`) VALUES
(12443, 70, 207578, '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0'),
(24876, 1, 207578, '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0000-00-00 00:00:00', '0'),
(37309, 2, 207578, '2018-04-26 11:05:21', '0000-00-00 00:00:00', '2018-04-26 11:05:21', '0000-00-00 00:00:00', '1');

Deny Tag, Allow Tag

INSERT INTO `tf_filters` (`id`, `created_at`, `created_by`, `updated_at`, `updated_by`, `requesting_action`, `requesting_type`, `requesting_entity`, `object_type`, `object_entity`, `note`, `gacl_aro`, `gacl_acl`, `effective_datetime`, `expiration_datetime`, `priority`, `deleted`) VALUES
(10609, '0000-00-00 00:00:00', 'admin', '0000-00-00 00:00:00', 'admin', 'deny', 'user', 'admin', 'tag', 1, '', 0, 0, '0000-00-00 00:00:00', '0000-00-00 00:00:00', 3, 0),
(10610, '2018-05-22 00:00:00', 'admin', '2018-04-26 00:00:00', 'admin', 'allow', 'user', 'admin', 'tag', 2, '', 0, 0, '0000-00-00 00:00:00', '0000-00-00 00:00:00', 1, 0);

Facility Tag, Deny Tag

INSERT INTO `tf_filters` (`id`, `created_at`, `created_by`, `updated_at`, `updated_by`, `requesting_action`, `requesting_type`, `requesting_entity`, `object_type`, `object_entity`, `note`, `gacl_aro`, `gacl_acl`, `effective_datetime`, `expiration_datetime`, `priority`, `deleted`) VALUES
(6476, '0000-00-00 00:00:00', 'admin', '0000-00-00 00:00:00', 'admin', 'allow', 'user', 'aartis', 'tag', 119, '', 0, 0, '0000-00-00 00:00:00', '0000-00-00 00:00:00', 1, 0),
(10185, '0000-00-00 00:00:00', 'admin', '0000-00-00 00:00:00', 'admin', 'deny', 'user', 'aartis', 'tag', 1, '', 0, 0, '0000-00-00 00:00:00', '0000-00-00 00:00:00', 3, 0);

In the calendar, all we are worried about is the schedule_facilities...I think. Essentially, by doing it this way we are using the following hacks:

  1. Using a relatively complex set of TAGS that users will need to replicate.
  2. Using the schedule_facilities feature, which is supposed to restrict what a user can schedule for (different than "see") as an access control feature.

The original implementation of this was the Facility ACL, that basically used the patient's custom facility field as the check against the user's schedule facilities to determine access. This system was a "plugin" by Ken Chapple. Unfortunately, the custom facility_acl plugin, and the merged "tags" plugin used functions with identical names, making it a mod, and not a plugin. The hooks were in the right place and all, but there was considerable modification to get the two to co-exist. The attempt to get the code reviewed and merged fail...two or three times. By the third attempt, too many things had changed, and I could never get the third version to work along side the tags, due to incompetence, spaghetti code, and lack of will.

Doing this in this manner is fine...for now, but we should do a total ACL replacement that includes per-patient-per-facility-per-user capabilities, along with "grouping" based on very clean and intuitive configuration UI components. My guess is that it should be a native system, though I will admit that GACL was probably capable of doing all this, and the implementation was just half-baked.

@aethelwulffe
Copy link
Contributor

I will set up and do a test (and make the test public accessible) with a database that reflects the complexities of the real world applications.
These will include what is commonly seen in Behavioral Health, as well as Assisted Living Facilities, and just the plain old "just show me my patients ignore everyone else" scenarios.

Copy link
Contributor

@aethelwulffe aethelwulffe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEst satisfactory

@aethelwulffe
Copy link
Contributor

User still gets a list of all facilities, clicking on them does not result in seeing provider name headers or appointments though

@aethelwulffe
Copy link
Contributor

This is a high priority fix for me....

@aethelwulffe
Copy link
Contributor

Ooops.
Code works, but does not catch the necessary cases:
Test case:

  • Provider_a works at facility_a and facility_b
  • Provider_b only works at facility_a
  • Provider_c only works at facility_b
  • User_1 works at facility_a and is only supposed to see facility_a patients.

Current issue:

  • User_1 cannot see Provider_c's schedule or patients.
  • User_1 CAN see Provider_b's patients.
  • User_1 CAN see Provider_a's patients...including the ones at facility_b!

The above case is true EVEN IF there is a TAG set to disallow User_1 to view patients not at facility_a.

@apooravc
Copy link
Contributor Author

@aethelwulffe About the first two points in Current issue, how are they issues? If user 1 can't see provider c's patients who are at facility_b where user 1 doesn't work and if user 1 can see provider b's patients who are at facility_a where user 1 works? Did I get it wrong?

This commit only restricts facilities of provider to selected schedule facilities for them, how can we modify it to cover these left out cases?

@aethelwulffe
Copy link
Contributor

Basically, we only want to see appointments that are AT facilities User_1 has access to...and no others, even if the provider has appointments at other facilities.

I will try to provide more useful info, and code suggestions (or a new chunk of code is more likely), but I think I am too frazzled to do more today. I actually have a branch with some related stuff I can put out there to try to attend to some of this junk.

@aethelwulffe
Copy link
Contributor

This needs Reviews/testing folks.

@aethelwulffe
Copy link
Contributor

Pretty pretty pretty please someone test/review this?
-Trying to get the calendar and ACL related stuff for it up to the point where our project is a viable option for a clinic and stuff...

@aethelwulffe
Copy link
Contributor

Re-tested, even in combo with some related PR's.
Facility lists are correct for the schedule_facilities settings, looks great.
-Appointment pop-up modal still has invisible tab labels with "light" style of course.
TESTED

Copy link
Contributor

@teryhill teryhill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I have not tested this.

@aethelwulffe aethelwulffe merged commit ea7bb8f into LibreHealthIO:master Jun 13, 2018
@apooravc apooravc deleted the fix-schedule-facilities branch July 13, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants