We plan to enhance and expand the number of patient reports that are available in OpenEMR. They will enable OpenEMR to meet the certification requirements of Patient Lists. Please review the proposed solution below:
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-06-01
Initial coding for Patient Lists has been completed. You can log into http://www.medbloom.com to see the reports. For data range, please start with 2008/01/01.
An important aside is none of the search functioning is working; I could
get nothing to return for nay search (except that all pt get returned when
click a 'diagnosis' search).
My contributions are:
---INCORPORATED NEW SECURITY METHODS ((set global reverse magic quotes
flag, set no fake globals flag, use sql binding/placemakers(stop
sql-injection), use htmlspecialchars(stop xss attacks)) Note no need for
formdata.inc.php functions anymore when use this stuff.
---FIXED incorrect demographics2.php links
---CHANGED FULL PHP tags, <?php instead of <?
---USED generate_display_field() for output of a list item.
---USED dropdown_facility function
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-06-30
Please make sure that you have data in all the tables. The search functions work on local computer as well as at www.medbloom.com. You can log in to see them.
We will review your patch and incorporate the suggested changes shortly.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thomas,
My changes are extensive in order for your programmers to learn the new security implementation (you'll be better off just starting from my patch because the extensive htmlspecialchars use basically makes back patching almost impossible). These security steps are discussed here: http://sourceforge.net/projects/openemr/forums/forum/202506/topic/3530656
It actually makes the code much cleaner, because do not need to use the confusing formdata.inc.php functions anymore (sort of a lie, rarely will need to escape a variable if it's a column label in a sql statement).
New pages should follow this method (changes to old pages can use old formdat.inc.php method unless page has been converted to the new method).
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-07-10
The changes have been made. Please review the codes.
1. Note that the following tables need to have data:
TESTING REVIEW:
--Need to ensure the functionality of the original prescription report (that you deleted) is still working. In your code, if I create a new user and make a prescription for the new user, it does not show up in the report. However, it does show up in old deleted report.
--No point in me testing the rest of the functionality until above works. I will say that I noted the age filter is not working (if I use it in the diagnosis function, it will include patients that should not be included (ie. do not fit within the filter). Please ensure comprehensive testing before next submission, since this is getting close to being able to commit to the codebase.
Also, base your patches on the most current cvs codebase. I think your now basing it off your first version off your reports; since I'm not sure what your basing it off, this is another source of potential issues with your code when we test it.
thanks,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Another testing issue is that the patient name links in the report output do not work (they always go to the current active patient).
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-07-13
Sorry about the patch. I applied the latest file without first applying your patch. I have resubmitted the patch. There are only a few lines of changes.
Visolve will do the QA. The patient links work at www.medbloom.com. So we will wait to hear from Visolve.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Make your patches from the most recent cvs(either SF development tip or the github master branch), so anybody can test your stuff. I can easily compare across your branches from git.
Why are you sending code to QA that does not work in the development tip (having it work at www.medbloom.com isn't useful for the codebase). You guys need to test and get your stuff working from the development tip. One cardinal rule of open source is to not break functionality; the prescription report obviously does not work like the previous report. Sending code that doesn't even fulfill this minimal testing requirement to QA by visolve is just wasting resources. You guys need to get in the habit of testing from the tip.
thanks,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-07-13
Of course, the programmer is using patch from Github Master. It's tested to work on his local machine and at www.medbloom.com so that everyone can see the changes and that everyone can see the development and testing are done.
We didn't have access to the SF Tip or the Github Master before (remember)? Please provide instructions if there is another way to test.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2010-07-13
Also, starting today, the programmer(s) will work with GitHub directly under username "ossllc". I will monitor their work from time to time. Please grant access to that account.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
You guys aren't using git in an ideal way. I suggest not ever touching your master or rel-320 branches on your repository; you only update these from the official github openemr repository. I'd suggest making another repository(even better for each programmer to have their own repository, since it's very easy to grab branches from each other). I recommend initially following these instructions, which I created for new git users to get started "correctly" with OpenEMR: http://www.openmedsoftware.org/wiki/Git_for_dummies
For testing, I place this script in the git directory (one above openemr), and run it whenever I want to do a native test (on mandriva, but can quickly script any OS):
So running above script will place newly created version of whatever git branch is checked out to be tested. Doing this kind of testing is vital (this is why most programmers, such as Rod, myself, Aron, Visolve and others can check in straightforward code without a QA cycle). If you then combine this testing with correct use of git/github, then the sky it the limit for your programmers.
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
It only takes a couple minutes to follow it (probably good for you to do it also since very minimal time), and then he/she will grasp the main strategy to employ with the github repository. There is no need for you guys to use SVN, just base your stuff off the github repository (this is why the tuturial shows your how to update your master/rel-320 branches and stresses never to manually modify these branches; all work should go in separate branches.) You also have total control of what you decide to keep on your local git repos vs what you publish on your github repos.
See my repo for an example: http://github.com/bradymiller/openemr
Note that I have each of your patches along with mine (thomas_report1-4) in separate branches. Also note how many other branches I have for other projects (this is the power of git). Also note I never touch the master or rel-320 branches (I just update them via the method in the above git tutorial link). This then allows creation of patches from a branch to be very easy, since can base it on the master branch, which is the most current official openemr codebase.
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
We plan to enhance and expand the number of patient reports that are available in OpenEMR. They will enable OpenEMR to meet the certification requirements of Patient Lists. Please review the proposed solution below:
http://www.openmedsoftware.org/wiki/Patient_Lists
I like it.
-Tony
Initial coding for Patient Lists has been completed. You can log into http://www.medbloom.com to see the reports. For data range, please start with 2008/01/01.
The codes can be reviewed at http://github.com/thomaswong2/openemr
Thomas,
What is difference in this patch from the new one. I have both patches on my git repository and a diff between the two shows no changes:
http://github.com/bradymiller/openemr/tree/thomas_report_1
http://github.com/bradymiller/openemr/tree/thomas_report_2
thanks,
brady
oops,
I got confused. Forget my above confusion. Have yanked my second branch, so now have your code here only:
http://github.com/bradymiller/openemr/tree/thomas_report_1
-brady
Thomas,
Placed my code review in the tracker:
http://sourceforge.net/tracker/index.php?func=detail&aid=3011938&group_id=60081&atid=1245239#
thanks,
-brady
A new patch has been submitted to the Tracker.
Thomas,
An important aside is none of the search functioning is working; I could
get nothing to return for nay search (except that all pt get returned when
click a 'diagnosis' search).
To deal with other code issues, I've contributed quite a bit to your code,
and put the patch here. To see what changes I made from yours, can see my
commit in my github branch here:
http://github.com/bradymiller/openemr/commits/thomas_report_3
Specifically my commit is:
http://github.com/bradymiller/openemr/commit/9c0ea0d0b6594b71f956211f70bc98bd22649385
My contributions are:
---INCORPORATED NEW SECURITY METHODS ((set global reverse magic quotes
flag, set no fake globals flag, use sql binding/placemakers(stop
sql-injection), use htmlspecialchars(stop xss attacks)) Note no need for
formdata.inc.php functions anymore when use this stuff.
---FIXED incorrect demographics2.php links
---CHANGED FULL PHP tags, <?php instead of <?
---USED generate_display_field() for output of a list item.
---USED dropdown_facility function
Thread and links to examples to explain the new security implementations
can be found here:
http://sourceforge.net/projects/openemr/forums/forum/202506/topic/3530656
Stuff you still need to do:
-Incorporate this recent additions from
interface/reports/prescriptions_report.php:
http://github.com/openemr/openemr/commit/212bc5a8137c4ce8dc98e3a2e50123c762ca6b54
-Please explain real quick the need/use for the date range stuff (I'm
confused by it)
-HAVE THE SEARCHING ACTUALLY WORK
-brady
Please make sure that you have data in all the tables. The search functions work on local computer as well as at www.medbloom.com. You can log in to see them.
We will review your patch and incorporate the suggested changes shortly.
Thomas,
My changes are extensive in order for your programmers to learn the new security implementation (you'll be better off just starting from my patch because the extensive htmlspecialchars use basically makes back patching almost impossible). These security steps are discussed here:
http://sourceforge.net/projects/openemr/forums/forum/202506/topic/3530656
It actually makes the code much cleaner, because do not need to use the confusing formdata.inc.php functions anymore (sort of a lie, rarely will need to escape a variable if it's a column label in a sql statement).
New pages should follow this method (changes to old pages can use old formdat.inc.php method unless page has been converted to the new method).
-brady
The changes have been made. Please review the codes.
1. Note that the following tables need to have data:
Diagnosis
- patient_data (Main)
- lists (Second)
- form_encounter (Date)
Procedure
- patient_data (Main)
- procedure_order (Second)(Date)
- procedure_report
- procedure_type
Prescription
- prescriptions (Main) (Date)
- drugs
- drug_sales
- drug_inventory
Medical History
- patient_data (Main)
- history_data (Date)
Lab Results
- patient_data (Main)
- procedure_result (Date)
Service Codes
- patient_data (Main)
- billing (Date)
The main data is from 'Main' table.
Tables marked with "Date" also need to have data. The date (From, To) is compared with the date field in those tables.
2. The date range is used for filtering and displaying the search results. It's common for a lot of reports.
Thomas,
TESTING REVIEW:
--Need to ensure the functionality of the original prescription report (that you deleted) is still working. In your code, if I create a new user and make a prescription for the new user, it does not show up in the report. However, it does show up in old deleted report.
--No point in me testing the rest of the functionality until above works. I will say that I noted the age filter is not working (if I use it in the diagnosis function, it will include patients that should not be included (ie. do not fit within the filter). Please ensure comprehensive testing before next submission, since this is getting close to being able to commit to the codebase.
Also, base your patches on the most current cvs codebase. I think your now basing it off your first version off your reports; since I'm not sure what your basing it off, this is another source of potential issues with your code when we test it.
thanks,
-brady
Thomas,
Another testing issue is that the patient name links in the report output do not work (they always go to the current active patient).
-brady
Sorry about the patch. I applied the latest file without first applying your patch. I have resubmitted the patch. There are only a few lines of changes.
Visolve will do the QA. The patient links work at www.medbloom.com. So we will wait to hear from Visolve.
Thomas,
Make your patches from the most recent cvs(either SF development tip or the github master branch), so anybody can test your stuff. I can easily compare across your branches from git.
Why are you sending code to QA that does not work in the development tip (having it work at www.medbloom.com isn't useful for the codebase). You guys need to test and get your stuff working from the development tip. One cardinal rule of open source is to not break functionality; the prescription report obviously does not work like the previous report. Sending code that doesn't even fulfill this minimal testing requirement to QA by visolve is just wasting resources. You guys need to get in the habit of testing from the tip.
thanks,
-brady
Of course, the programmer is using patch from Github Master. It's tested to work on his local machine and at www.medbloom.com so that everyone can see the changes and that everyone can see the development and testing are done.
We didn't have access to the SF Tip or the Github Master before (remember)? Please provide instructions if there is another way to test.
Also, starting today, the programmer(s) will work with GitHub directly under username "ossllc". I will monitor their work from time to time. Please grant access to that account.
Thomas,
Thomas,
You guys aren't using git in an ideal way. I suggest not ever touching your master or rel-320 branches on your repository; you only update these from the official github openemr repository. I'd suggest making another repository(even better for each programmer to have their own repository, since it's very easy to grab branches from each other). I recommend initially following these instructions, which I created for new git users to get started "correctly" with OpenEMR:
http://www.openmedsoftware.org/wiki/Git_for_dummies
For testing, I place this script in the git directory (one above openemr), and run it whenever I want to do a native test (on mandriva, but can quickly script any OS):
So running above script will place newly created version of whatever git branch is checked out to be tested. Doing this kind of testing is vital (this is why most programmers, such as Rod, myself, Aron, Visolve and others can check in straightforward code without a QA cycle). If you then combine this testing with correct use of git/github, then the sky it the limit for your programmers.
-brady
I don't suppose anyone has created a script for Windows?
Thanks,
Marc
Medbloom. What happened with the User and Password?
I can't remember mine it seems………………
hey,
To better explain the above testing script, I placed comments etc in it, and created one for ubuntu also.
Mandriva script is here:
http://gist.github.com/478787
Ubuntu script is here:
http://gist.github.com/478814
So to do the windows script, would just need to replace the steps with code that will work in a windows batch file.
-brady
Thanks, Brady. That's exactly why we had a patch issue.
Blankev, I have sent you a private message regarding your login.
The programmer has uploaded a new patch to http://github.com/med2000/openemr.
He was experimenting with patch revert, just in case you wonder.
Here're the steps taken by him:
1. Setup Cloning
Master -> http://github.com/med2000/openemr
Master -> Localhost SVN
2. Update/Feed (when making changes)
Master -> Localhost SVN -> http://github.com/med2000/openemr
Thomas,
Getting closer, but have your programmer follow this tutorial from a clean new forked openemr github repos:
http://www.openmedsoftware.org/wiki/Git_for_dummies
It only takes a couple minutes to follow it (probably good for you to do it also since very minimal time), and then he/she will grasp the main strategy to employ with the github repository. There is no need for you guys to use SVN, just base your stuff off the github repository (this is why the tuturial shows your how to update your master/rel-320 branches and stresses never to manually modify these branches; all work should go in separate branches.) You also have total control of what you decide to keep on your local git repos vs what you publish on your github repos.
See my repo for an example:
http://github.com/bradymiller/openemr
Note that I have each of your patches along with mine (thomas_report1-4) in separate branches. Also note how many other branches I have for other projects (this is the power of git). Also note I never touch the master or rel-320 branches (I just update them via the method in the above git tutorial link). This then allows creation of patches from a branch to be very easy, since can base it on the master branch, which is the most current official openemr codebase.
-brady