ranjithavi wrote on Wednesday, November 14, 2012:
Hi All,
We have implemented the “Clear active patient” for Home button click.
Here the link for the code changes.
Kindly share your views.
-Ranjith
yehster wrote on Wednesday, November 14, 2012:
In the interest of code reuse and maintenance the ajax function should include and use the setPid method from pid.inc, rather than unset.
Also there is the possibility that other code will treat “unset” session variables differently than “zero’d”
bradymiller wrote on Wednesday, November 14, 2012:
Hi Ranjith,
There’s a big git issue here. Please use the openemr git repository to abse your code on. In order for me to test/review your code, I need to fetch your repository, which has one big commit that contains OpenEMR that I am forced to download (imagine what will happen to everybody’s disk space if everybody did this); if you base your code on a openemr repo, then this does not happen. Also, note that by basing your code on an openemr repo, we can tell just by looking at the commits how recent it it.
Here’s a tutorial for setting up an openemr repo:
http://www.open-emr.org/wiki/index.php/Git_for_dummies
(If you need git pointer/help, please always feel free to ask.)
Also, I tested/review the code, and noted following issues(I also agree with yehster’s input above):
1. Will users be ok with having the Home button automatically clear the current patient? Seems like some users may want to click on the Home button to get to the calendar/messaging while keeping the current patient/encounter information. But I’ll leave this up to the users actively using it. If this is the case, then may make sense to have another button to clear the patient.
2. The Patient:NONE and Encounter:NONE labels after clear seem odd(since not there when login). Perhaps best to just not show them.
3. Placed some other minor coding issues on github.
thanks,
-brady
OpenEMR
ranjithavi wrote on Thursday, November 15, 2012:
Hi Brady/yehster,
Thanks for reviewing the code. Sure will modify the code as per the suggestions and share the same.
Brady - we will use the openEMR repo for upcoming commits. (Note : That’s also a clean openemr repo with out any changes in the base openEMR 4.1. -dev).
Thanks,
Ranjith
ranjithavi wrote on Thursday, November 15, 2012:
Hi All.
As per the suggestion, modified the code and pushed. Kindly review the code and share your views.
Thanks,
Ranjith
bradymiller wrote on Saturday, November 17, 2012:
Hi,
Just placed a review on github.
The main issue now is rather critical:
Will users be ok with having the Home button automatically clear the current patient? Seems like some users may want to click on the Home button to get to the calendar/messaging while keeping the current patient/encounter information. But I’ll leave this up to the users actively using it. If this is the case, then may make sense to have another button to clear the patient.
Are there any thoughts by others(especially users) on this issue?
-brady
OpenEMR
sunsetsystems wrote on Saturday, November 24, 2012:
Added a comment to the commit about the call to restoreSession() being missing. Aside from that and Brady’s question above, it seems OK to me.
visolveemr wrote on Thursday, December 27, 2012:
Hi Brady/Rod,
As per the suggestions and discussions with the users, we have incorporated the following changes and committed in the git-hub for review.
Changes:
* We introduced the new button called “Clear Active Patient” near “New Patient” Button.
* This “Clear Active Patinet” button will be enabled after selecting any patient.
* Once we click the “Clear Active Patient” , it clears the current active patient ,encounter and go back to the default home page as like in the “Home” and “Clear Active Patient” Button will be disappear.
Rod’s comment also incorporated in this commit.
Commit: https://github.com/visolve-openemr/openemr/commit/f0dab6ea67e7451c95016fa8e261d1d6c52ec589
Verify the same and share your views.
Thanks,
Ranjith
bradymiller wrote on Friday, December 28, 2012:
Hi,
This tests well and looks good to me.
Rod, can you take a quick sanity look to ensure it doesn’t break any obvious rules?
Then I’ll commit it.
(I plan to change the script header @package tag to OpenEMR before the commit)
thank,
-brady
OpenEMR
bradymiller wrote on Friday, December 28, 2012:
Actually Rod,
Disregard my above request. I noted your comment above regarding the previous code commit, so this is good to go. I will commit it now.
Thank you Ranjith for the code contribution,
-brady
OpenEMR
tmccormi wrote on Friday, December 28, 2012:
Please make sure the Medical Information Integration is credited. We paid for the work from Visolve.
-Tony
bradymiller wrote on Friday, December 28, 2012:
Hi Tony,
Please feel free to add this(or have Visolve do this) and commit it (or ask me to commit it when it’s ready to go).
(Please note that in the script headers the @link is not the author website, but where the codebase website is; should either use http://www.open-emr.org or http://sourceforge.net/projects/openemr/)
thanks,
-brady
OpenEMR
bradymiller wrote on Friday, December 28, 2012:
To clarify,
This was already committed to sourceforge yesterday.
-brady
tmccormi wrote on Friday, December 28, 2012:
Thanks, I’ll fix that and update the commit.
-Tony