Many times in my customers end users with admin rights “accidently” hit the DELETE patient button. It (as you know) does a cascading delete of ALL of that patients data.
Now I ask: Why do we even have a patient delete button? Why would anyone ever delete patient data in a live production system? If it’s a dupicate, there is a merge tool. If it’s bad, just change he last name to ZZZBAD or something.
Recovering a deleted patient from the logs or from backup is very tricky…
Either a “feature” a “security risk” depending on how you look at it though is that someone could still generate a call to the deleter.php page without using the button and still delete patients. (No global check in the actual delete code. Just the UI element is being hidden…)
Sorry, my comment was not meant to imply that your very useful contribution is inappropriate.
Lot’s of components in the system work in a similar fashion, where UI elements are simply hidden. (left_nav does this all over the palce…) without the additional “fail safe”
My intention was to highlight an aspect of security where the project as a whole could probably do better.
Thanks for the code Terry(and do note Kevin already committed it to the codebase).
Note the deleter.php code is already protected by acl, which should be adequate. The goal of the global seems to be to stop the accidental clicking of the button and is not meant to prevent a staff member with malicous intentions whom also happens to have admin access
On second thought, though, after reading the wording of the option, which is “‘Allow Administrators to Delete Patients’”, Kevin does make a good point and would be easy to add the global check here:
If a user had admin access what is to prevent said user from going in to globals and turning the option on. The users that I have do this by accident because the delete button is located close to the “Documents” label.
As another example. The pages which require a patient or encounter to be set (such as the popups) are only restricted by the left_nav disabling the options. However, if one of those pages is accessed without a patient set, typically there’s no error generated on the page itself.
It’s not a big deal when everything is working properly, but in general more validation is better than less.
Coming from the perspective of customer support as I am, I wonder if the mods above will completely remove the delete button from the reach of normal office workflows? Doesn’t some UI design dictum say that if an object is visible it will be clicked on accident? I.e., simply requiring an Admin ACL to delete the pt is not adequate- in many offices every user has Admin priviledges just so they can do whatever they need to do without bothering the Dr/ Sys Admin.
Maybe the deleted pt’s data could simply be stuffed into a sql file and stashed where the backups go, just to make restoring the pt easier when it (inevitably) becomes necessary. In fact, if this were implemented, you could just leave the delete button as it is- restoring a deleted pt would be trivial.
The Delete button is dangerously close to the Documents link, such that accidentally clicking would occur. Personal experience.
The Delete button can be moved to the upper right corner of the Patient Summary to lessen the chance of accidental clicking.
Another option is to provide another tab in Demographics just for account deletion. A 2 step process with the warning should decrease accidents.
The delete function is useful when a practice is in process of account setup. There are times when setup is so flawed & irreparable that deletion is the most efficient way to deal with it.
In our experience delete has a legitimate purpose - e.g. when a new patient never shows up for appointment. Surely there are workarounds but nothing as complete as deleter. So instead of all or nothing approach, we would suggest the following :
Create a copy of patient_data structre as patient_archive (and maintain in sync forever!).
When user presses ‘Delete’, copy all columns ot that patient_data to patient_archive.
Delete patient_data record
Modify deleter to show global overview of the related records for each archived patient and let administrator take following actions :
a. Restore - copy back to patient_data and delete from patient_archive
b. Delete - Use current logic to eliminate all traces to permanently delete from database.
It is recognized that this takes advantage of lack of referential integrity in the database design. Additional db techniques will be required if we turn on the referential integrity. At one point we looked for a deleted flag but patient_data is so pervasive that propagating such a change will result in review of entire application.
