zhhealthcare wrote on Monday, August 23, 2010:
Hi guys,
Haven’t heard any comments from anyone else. We will also try to work out a design for the “CPT wise billing status” after this patch is committed.
So from the conversation I understand that the patch is good to go except for the aggregate functions.
Thanks and regards
Sam
sunsetsystems wrote on Monday, August 23, 2010:
Sam, two more things:
Some coding-level description of the changes, please? There are very few comments in the code and I think it’s important to describe, for each new and changed module, what was done.
Regarding payers who are not the patient and not insurance: don’t you think it’s important to record the responsible party when demographics are entered? This will be relevant when sending statements, and will surely also simplify data entry when payments are entered. Given that, I’m not sure it’s even useful to identify a non-insurance payer in ar_session.
bradymiller wrote on Tuesday, August 24, 2010:
Sam and Rod,
This stuff is flying over my head. Are you recommending a database scheme fix. If so, then I suggest Sam fix that and then resubmit the code for review. Once the database scheme and functionality is ironed out, then I’d be glad to review code for the normal stuff (internationalization etc.)
-brady
zhhealthcare wrote on Tuesday, August 24, 2010:
Rod and Brady
Here is what we are gonna do:
1.Change the database and the code to make changes to include aggregate functions for the undistributed amount.
2. Include coding level descriptions
3. Bug fixes that was encountered while entering live data.
The following will be included in the next patch.
1. Include responsible party in demographics. This will take some time to work on.
3. CPT wise billing status. Needs design and identify impact.
If this is fine with you we will get on with the work.
Please confirm
Sam
sunsetsystems wrote on Tuesday, August 24, 2010:
Sam I suggest making “CPT wise billing status” a separate project - or perhaps that is what you mean? Otherwise it sounds good.
Thanks!
zhhealthcare wrote on Tuesday, August 24, 2010:
Rod,
That is what I meant: responsible party and CPT will be separate project/projects
bradymiller wrote on Wednesday, August 25, 2010:
hey,
Sounds like a plan. Here’s my more specific review:
ZH_PAYMENT_1 (8/7/10 payment branch from zhhealth on github)
OVERALL:
1. No need to encircle all your additions with Z&H (this is done automatically when you commit it). Better to focus on providing some succinct documentation.
2. Need to incorporate your database changes in sql/database.sql and sql/3_2_0-to-3_3_0_upgrade.sql.
3. Jquery provides very simple one liner ajax functionality. No need to recreate this mechanism (in common_functions.js).
4. Do not do raw sql calls on ‘list_options’ tables to create your selection list. Instead, this can all be done in one line with function generate_select_list() in library/options.inc.php. Grep the fucntion to see examples of it in use. You need to use this function to support internationalization and it also has xss/sql-injection prevention built into it.
5. Consider wrapping all output that goes to screen that a user can touch (including the xl() calls) with the htmlspecialchars() function to stop xss attacks:
This is described in item number 4 in this wiki section: http://www.openmedsoftware.org/wiki/Active_Projects#PLAN . See the links to commit examples in this wiki section to get an idea of what I mean. Don’t worry about items 1-3; that would be way too extensive, and you’ve made good use of the formData function. In the future, though, when you make new script(s) (again, do not do for this project), I’d rec using the technique outlined there (then you no longer need the confusing formData function(s), and sql-injection is stopped with the sql binding).
6. Confused by the DateToYYYYMMDD() function and changing calendar to $DateFormat. I’m not saying it’s bad or anything, but can you clarify what this stuff is for?
7. Do not include special characters (such as \n) within xl() functions. If really needs the newline, then need to separate into two separate xl() calls. I’d rec just removing the \n and keeping it one xl() since your lines are not too long. (Note the xl() function actually will remove the \n anyways; the problem is these special characters in the constants will confuse the translators).
8. Do edit_payment.php and new_payment.php have basically the same code? If so, then could consider placing same code in one file and including this in both (so easier to maintain).
SPECIFIC FILES:
interface/billing/billing_process.php:
–No issues.
interface/billing/billing_report.php:
–No issues.
interface/billing/edit_payment.php:
–Translate New Payment, Search Payment, ERA Posting
–Watch out with the is_resource call. Note the library/sql.inc functions can return a resource or a recordSet depending on whether binding is used(at some point all pertinent code will be using binding, so your call will make harder to upgrade in the future). I’d rec using another way to see if there is any data returned by the sql call.
interface/billing/era_payments.php:
–Several strings/alerts are not getting translated
–Translate New Payment, Search Payment, ERA Posting
interface/billing/new_payment.php:
–Translate New Payment, Search Payment, ERA Posting
interface/billing/search_payments.php:
–Translate New Payment, Search Payment, ERA Posting
interface/billing/sl_eob_process.php:
–Translate Check Number along with other strings/warnings
–Don’t use htmlentities(); use htmlspecialchars() instead. htmlentities is not compatible with UTF-8 encoding.
interface/main/left_nav.php:
–Don’t remove the bottom screen when click the Payments links.
–Consider putting the Payments link in the Fees folder
–Do Encounter History and New Encounter need to be translated?? (I’m just asking, since not sure what the code is doing)
interface/pic/Delete.gif:
–No issues.
library/common_functions.js:
–Some useful common functions here, but need to separate the specific functions into other specific library files. I think we have a common.js library already, which is better to place the commonly useful function (I’m not sure about this though)
library/common_functions.php:
–Lots of untranslated strings in the ajax stuff
–Again, need to organize these functions into other pertinent files or new files.
library/parse_era.inc.php:
–No issues (although this stuff is way over my head).
library/sl_eob.inc.php:
–Some debug statements left in there
thanks,
brady
zhhealthcare wrote on Wednesday, September 15, 2010:
We have coded the Brady’s comments and various bugs were fixed.
It is put in the testing mode.As soon as the testing is finished the code will be submitted.
The action per comment is explained below.
1. No need to encircle all your additions with Z&H (this is done automatically when you commit it).Better to focus on providing some succinct documentation.
Z&H labels are removed.A short note is put in each function,explaining what it does.Documentation is done at places which are not so obvious.
2. Need to incorporate your database changes in sql/database.sql and sql/3_2_0-to-3_3_0_upgrade.sql.
Database changes are incorporated.
3. Jquery provides very simple one liner ajax functionality. No need to recreate this mechanism (in common_functions.js).
The ajax drop down is now replaced with Jquery library.Sample Ajax was found in the ACL section.Implemented it in New Payment.But conflict occured in the Search Payment.Both, edit popup(uses file==>jquery.fancybox-1.2.6.js) & ajax(uses file==>jquery.livequery101.js) were using differet libraries based on Jquery.Resolved it by implementing ajax withe core ajax file(jquery121.js).One more challenge was that,2 ajaxes(patient and insurance) were needed at the same text box.With minimum duplication it is coded.Testing is done in the 3 browsers(Chrome,Internet Explorer & Mozilla firefoz).
4. Do not do raw sql calls on ‘list_options’ tables to create your selection list.Instead, this can all be done in one line with function generate_select_list() in library/options.inc.php. Grep the fucntion to see examples of it in use. You need to use this function to support internationalization and it also has xss/sql-injection prevention built into it.
In 7 places the generate_select_list() function is used.One place(Payment Category in new and edit screen) does extra functionality with the drop down.A similar function generate_list_payment_category() is written and used, which uphelds the standanrds.The function generate_print_field() of the library is used to just display a list data.
5. Consider wrapping all output that goes to screen that a user can touch (including the xl() calls) with the htmlspecialchars() function to stop xss attacks:
This is described in item number 4 in this wiki section: http://www.openmedsoftware.org/wiki/Active_Projects#PLAN . See the links to commit examples in this wiki section to get an idea of what I mean. Don’t worry about items 1-3; that would be way too extensive, and you’ve made good use of the formData function. In the future, though, when you make new script(s) (again, do not do for this project), I’d rec using the technique outlined there (then you no longer need the confusing formData function(s), and sql-injection is stopped with the sql binding).
xl() calls,echo to input controls and listing of data from database are all passed through the htmlspecialchars() function.
Item No:4 in the wiki section: http://www.openmedsoftware.org/wiki/Active_Projects#PLAN is used.
6. Confused by the DateToYYYYMMDD() function and changing calendar to $DateFormat. I’m not saying it’s bad or anything, but can you clarify what this stuff is for?
For the 3 supported date format,the javascript code also should be twicked to display the date as per it.Output of DateFormatRead() function is given to ‘ifFormat’ parameter of the ‘Calendar.setup’.This will show the date as per the global settings.Second, in database the date can be stored only in the yyyy-mm-dd format. DateToYYYYMMDD() function accepts a date in any of the 3 formats, and as per the global setting, converts it to the yyyy-mm-dd format.
7. Do not include special characters (such as \n) within xl() functions. If really needs the newline, then need to separate into two separate xl() calls. I’d rec just removing the \n and keeping it one xl() since your lines are not too long. (Note the xl() function actually will remove the \n anyways; the problem is these special characters in the constants will confuse the translators).
For the short lines the \n is removed.For lengthy lines separate xl() calls are used in New and Edit payment screens.
8. Do edit_payment.php and new_payment.php have basically the same code? If so, then could consider placing same code in one file and including this in both (so easier to maintain).
Combined common parts of edit and new payment screens.Following is the new file structure.
\interface\billing\payment_master.inc.php========>(New Page.Check/cash details are entered here.Used in New Payment and Edit Payment screen.)
\interface\billing\payment_pat_sel.inc.php========>(New Page.Patient ajax section and listing of charges.Used in New Payment and Edit Payment screen.)
\library\payment.inc.php========>(New Page.This section handles payment related php functios.Add, Search and Edit screen uses these functions.)
\library\payment_jav.inc.php========>(New Page.This section handles payment related javascript functios.Add, Search and Edit screen uses these functions.)
\library\ajax\payment_ajax.php========>(New Page.This section handles ajax for insurance,patient and for encounters.)
\library\ajax\payment_ajax.js========>(New Page.This section handles ajax functions for insurance,patient and for encounters.)
\interface\billing\new_payment.php========>(New Payment screen.)
\interface\billing\edit_payment.php========>(Edit Payments Screen.)
SPECIFIC FILES:
interface/billing/billing_process.php:
–No issues.
This is billing section.The files are not needed for payment.
Wrongly submitted.
interface/billing/billing_report.php:
–No issues.
This is billing section.The files are not needed for payment.
Wrongly submitted.
interface/billing/edit_payment.php:
–Translate New Payment, Search Payment, ERA Posting
Translated the labels
–Watch out with the is_resource call. Note the library/sql.inc functions can return a resource or a recordSet depending on whether binding is used(at some point all pertinent code will be using binding, so your call will make harder to upgrade in the future). I’d rec using another way to see if there is any data returned by the sql call.
is_resource call is removed.Used ‘if statement’ using sqlFetchArray and instead of ‘while loop’ used ‘do-while’.
interface/billing/era_payments.php:
–Several strings/alerts are not getting translated
Strings/alerts are translated
–Translate New Payment, Search Payment, ERA Posting
Translated the labels
interface/billing/new_payment.php:
–Translate New Payment, Search Payment, ERA Posting
Translated the labels
interface/billing/search_payments.php:
–Translate New Payment, Search Payment, ERA Posting
Translated the labels
interface/billing/sl_eob_process.php:
–Translate Check Number along with other strings/warnings
Translated the strings/warnings
–Don’t use htmlentities(); use htmlspecialchars() instead. htmlentities is not compatible with UTF-8 encoding.
Replaced htmlentities() with htmlspecialchars() in 2 places.
interface/main/left_nav.php:
–Don’t remove the bottom screen when click the Payments links.
Bottom frame is not removed when the Payment link is clicked.
–Consider putting the Payments link in the Fees folder
Payment link is put in the Fees folder.
–Do Encounter History and New Encounter need to be translated?? (I’m just asking, since not sure what the code is doing)
This section is of the GUI Interface we created.This code is not needed for payment.We will not submit this in the new version.
interface/pic/Delete.gif:
–No issues.
No Action Needed
library/common_functions.js:
–Some useful common functions here, but need to separate the specific functions into other specific library files. I think we have a common.js library already, which is better to place the commonly useful function (I’m not sure about this though)
This file no longer exist.Separated the specific functions into other specific library files.The files are
\library\ajax\payment_ajax.js========>(New Page.This section handles ajax functions for insurance,patient and for encounters.)
\library\payment_jav.inc.php========>(New Page.This section handles payment related javascript functios.Add, Search and Edit screen uses these functions.)
\library\js\common.js========>(Common Function=>PreventIt)
library/common_functions.php:
–Lots of untranslated strings in the ajax stuff
Translated the strings in the ajax stuff
–Again, need to organize these functions into other pertinent files or new files.
This file no longer exist.Separated the specific functions into other specific library files.The files are
\library\formatting.inc.php========>(Date formating functions are stored here.)
\library\ajax\payment_ajax.php========>(New Page.This section handles ajax for insurance,patient and for encounters.)
library/parse_era.inc.php:
–No issues (although this stuff is way over my head).
No Action Needed
library/sl_eob.inc.php:
–Some debug statements left in there
Unwanted statements are removed
thanks,
Paul,
bradymiller wrote on Wednesday, September 15, 2010:
Your group is awesome.
thanks,
brady
sraj49 wrote on Wednesday, September 15, 2010:
Thanks Paul/Sam for the update on PID. Also in the exsting EOB page there are a few functions which are very useful and essential.
1 Select the payment posting is for Ins 1, Ins 2 or patient
2 Select queue for secondary billing and that automatically queues up the claim for secondary billing in Billing report.
3 Many times the copay is posted later and after the fee sheet is got ready. We had provided a feature to enter copay in the EOB/now payment posting area. May be this is available in your system.
4 Is there a place to enter the insurance adj reasons in the payment systems. This is very essential.
Thank you for your clarification and a wonderful job.
Raj
zhhealthcare wrote on Wednesday, September 15, 2010:
Brady,
Thanks. IT means much coming from you. And hey, right back at you.
Regards
From all of us at www.zhservices.com
zhhealthcare wrote on Thursday, September 16, 2010:
Regarding few functions in the exsting EOB page,we plan like this.
1 Select the payment posting is for Ins 1, Ins 2 or patient
We have changed the label to a drop down values of Ins 1, Ins 2 or patient.So now it is selectable.
2 Select queue for secondary billing and that automatically queues up the claim for secondary billing in Billing report.
Automatic movement to the next level and finally to the patient level is implemented.
After Payment of Ins1 it will be queued for secondary.If in any case he wants to pay once again for primary it is possible with drop down as explained in the first one above.
Items 3,4
We will let you know soon.
zhhealthcare wrote on Friday, September 24, 2010:
Patch file for payment system is attached.
It can be got at https://sourceforge.net/tracker/?func=detail&atid=1245239&aid=3040507&group_id=60081
1) Apply patch ‘0001-Modified-Payment-Module.patch’
2) Copy Delete.gif to openemr/interface/pic/
Thanks
Paul
sraj49 wrote on Saturday, September 25, 2010:
Paul,
Can you please clarify on what base tha patch is to be applied. Our system is already running on 4.0 . Can I apply the patch directly?
Thanks for your clarification
Raj
zhhealthcare wrote on Saturday, September 25, 2010:
You can apply the patch to the latest cvs version.
This patch is created yesterday and the latest modifications were pulled from master.
Thanks
Paul
sraj49 wrote on Saturday, September 25, 2010:
Thanks Paul. I applied the patch , all tables are updated. The insurance dropdown Ajax is not working. When I strat typing no drop down appears. Also when I fill in Medicare it does not recognize since probably it is not selescted from the drop down. Can you guide me as to what could probably be the issue.
Thanks
Raj
sraj49 wrote on Sunday, September 26, 2010:
Paul,
I have installed the patch and now it works very nicely. Thanks for all the hard work. I really like this module and helps in a big way. Thanks once again and I sincerely appreciate the efforts of Sam, yourself and your team.
While I uploaded ERA,s couple of times I get a message " you cannot access this page directly" . What is this and why does this happen. Another field which will be better if added is the PID or pubID for the patients. This is because when you deal with larger clinics you get people with the same name lots of times and PID would be very useful for search.
I need your assistance in resolving the issue related to the error message. Thanks and have a great night….Raj
sraj49 wrote on Sunday, September 26, 2010:
Hey Paul,
I have resolved the issue which happened to be an error on my part. The system is working. I am now testing the same. While on this I was informed by Sam as well I saw on your demo site the ‘ledger’ module. This is one thing that is really missing in the biling. Thanks a lot to Sam and yoruself.
Raj
Once again lot of thanks to Sam and you
Raj
sraj49 wrote on Tuesday, September 28, 2010:
Hey Sam/Paul,
1 I am trying to understand the significance of Patient ID in ar_session table .
2 Also there is already a field called adj reasons in ar_activity and in the new payment system there is a new field in ar_session adjustment_code. How is this related to the adj reasons in ar_activity.
3 The payer_id field in ar session in the present system defines the payer. " 0 " is patient and any other number is the id of the Insurance Company. Any specific reason why another filed called ‘payment_type’ is created when the Paying entity can be easily related to the existing payer_id field.
4 When there is a large check received from an Insurance comapny the posting is done in more than one session. In the serach area under the pay status in cases where there are more than one session for the same check or any kind of payment against all sessions is shown as " Unsettled" and only in the session where the balance becomes zero it shows as fully paid. Probably the system should look for all sessions related to the same check number and and once it fully posted should give only one status. What do you think?
Raj
Please clarify if I have the coorect understanding and I would sincerely appreciate your kind clarification.
Thanks and have a great day
Raj
bradymiller wrote on Tuesday, September 28, 2010:
hey,
Code Review. Broken it down into MANDATORY and OPTIONAL stuff. Everything in the per file review below is mandatory, while the overall review below this is mostly optional, but important for your future code submissions.
interface/billing/edit_payment.php
—Pick one jquery library to include (you have 3!; library/js/jquery.js; library/js/jquery.1.3.2.js; library/js/jquery121.js)
—No leading/trailing spaces witin the xl calls ("UNDISTRIBUTED is ")
interface/billing/era_payments.php
—Pick one jquery library to include (you have 3!; library/js/jquery.js; library/js/jquery.1.3.2.js; library/js/jquery121.js)
—Why are you overriding a user configuration setting? ($INTEGRATED_AR = $GLOBALS === 2;)
—Translate “Please Choose a file”
—Place colons outside the xl function
interface/billing/new_payment.php
—No leading/trailing spaces witin the xl calls (“UNDISTRIBUTED is “)
interface/billing/search_payments.php
—Pick one jquery library to include (you have 3!; library/js/jquery.js; library/js/jquery.1.3.2.js; library/js/jquery121.js)
interface/billing/sl_eob_process.php
—No leading/trailing spaces witin the xl calls " is not full.” Also, make one string rather than break them up if possible, For example xl(“Total Distribution for following check number is not full”).”: ".$checkNumber .
library/ajax/payment_ajax.js
—Note there is no good translation mechanism from js files. So, if you think your gonna need to add text stuff here in the future, better to make it a file that ends in .php, so when it gets included it gets run through the php runtime and translated as needed (note this is the solution that is used for the js date widgets to get translated)
library/sl_eob.inc.php
—$GLOBALS=0; This is very worrisome code, since messing with this stuff can cause the database to break (this code was added by Visolve for logging). Instead of doing this, report the bug to Visolve, so they can fix it correctly without causing serious issues in the future.
sql/3_2_0-to-4_0_0_upgrade.sql
—Note the commands #IfMissingColumn etc. are used in the sql_upgrade script, and are not comments. Putting comments there (your stuff in parentheses) will break this feature. Also, best way to test is to run the sql_upgrade script on a test upgrade twice (first time it will install the block of code and second time it will skip it).
—Clarify your code (Why have multiple ALTER TABLE’s for adding columns to same table?? Why are you adding and modifying the same column??)
—Do not mention specific character set (ie. UTF8) in this code
—For appropriate way to add list_options look at examples in this file; there are many of them
—Very important to get this stuff right and that it tests right, because database bugs are the only thing we can’t easily fix with a patch after an official release.
sql/database.sql
—Modify the original tables, do not modify after the tables are created.
—Place the new list_options lists within that section. Use other lists as examples of how to do this. Note you need to add an entry for the list title of each list.
—Very important to get this stuff right and that it tests right, because database bugs are the only thing we can’t easily fix with a patch after an official release.
1. Add your copyright info to all new files (your company and year) - MANDATORY
2. Ensure incorporate the top.restoreSession() stuff in code (since I’m not following flow of code, not sure if your using this everywhere it’s needed). - MANDATORY
3. Confusion on htmlspecialchars use:
—Do not use ‘e’ (echo) parameters if nesting xl within it (instead, place echo in front of htmlspecialchars), since this essentially skips the htmlspecialchars function. This is OPTIONAL in this code submission (but will be mandatory for other projects you submit). I suggest in your future code to not ever use any parameters in the xl() function, which will avoid this problem. If you decide not to change this (which is fine), just put following comments at top of each file (so it gets fixed during the security walk-through at some point):
“TODO: Remove e parameter from xl functions nested within htmlspecialchars functions”
—Use ENT_QUOTES flag when needed. Please read up on this function and flags. Using ENT_QUOTES everytime would be safest route (this flag will encode the single quotes, so won’t break html within javascript code and within html tags). OPTIONAL in this submission, but important to understand for future code.
—Do not double escape stuff (running htmlspecialchars twice) $alertmsg string is getting escaped twice. OPTIONAL in this code, but important to understand for future code
4. Confusion of formData function:
—Please read these functions and understand them. Note that formData is specifically used to prepare data for use in a sql statement, and is not meant to prepare data for use within algoritms (trim(formData(‘type_name’ ))==‘Patient’) or for output to html (echo formData(‘type_code’)). What I’m talking about will make more sense if you read through the functions from formdata.inc.php script. The confusion of these functions (because of php’s annoying magic quotes and sql’s need for escaping) is why it’s a good idea in your new scripts (of your future projects) to use the http://www.openmedsoftware.org/wiki/Active_Projects#PLAN methods. All of this is OPTIONAL stuff but is important for future code submissions of other projects.
I didn’t do much testing (just made sure things showed up). Will be important to get testing from Raj, Rod, Tony, and anybody else with billing experience. Once above mandatory elements are dealt with and testing bugs are dealt with, then can commit.
-brady