Still needs some work, but it is functional so I thought I'd share it and hopefully get some feedback.
I'm going to update the misc billing options page to tie into the new db table. I am also going to work up migration sql and add it to the upgrade script. I also do intend to make the visual style match other pages / items on the page, but that was glossed over for now.
Still, please try it out, if you have time to do a new install, create a patient, and look at their summary page. The new content should be clear.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Just so I'm clear (my billing knowledge is a bit weak), can you clarify the objective; is it for prior auth approval for MRI's etc?? If so, seems out of place to be putting this in the patient summary screen, but that's just my initial thoughts. I'd have to defer to the utility of this and best strategy to incorporate it to others, such as Tony, Rod, etc. Tony and Rod, any thoughts?
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
First, thanks for the review and links to documentation. I am currently fixing up the code based on your review.
As far as an objective goes, our agency wants to track prior authorizations for various purposes. Primarily, a prior authorization "number" needs to be included in the X12 we send to our payers. However, we also want to be able to notify clinicians and billing personnel when a patient is nearing the end of their prior authorizations and/or when they are scheduled for services that require prior auth., but they don't yet have one (or have not enough units left).
Our agency is mostly behavioral rather than medical, so MRIs wouldn't be an example of what we want to track, but rather Group, Individal, or Family therapy. These can often be scheduled months in advance, so tracking the prior authorizations in our EMR would be a boon.
From the information I am getting from the billing personnel and the developer that has been here longer, a prior authorization is identified by a "number" (I use quotes, because I'm not sure it is all digits; letters might be included), and is for a particular patient to receive a number of units (e.g. 4 - 1 hour) of a certain service (e.g. HCPCS:90847 - Family Therapy w/ Patient) within a certain date range.
A single prior authorization number is kept on the misc. billing options form, but this doesn't provide us enough information to do any real tracking or validation. I expect to replace the text field on the misc. billing options form with a drop-down of prior authorizations populated form the database.
Since the prior authorizations are attached to a particular patient but not necessarily a single encounter, I figure the patient summary screen would be a simple location for them. However, I'm certainly open to ideas for how to handle this.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I think the function that escapes strings destined for the database should be kept the the file with all the rest of the database abstraction layer. It might be useful to escape things from locations other than form data, so it doesn't belong with the form data processing. While it is easy enough to pre-escape anything "hard-coded" when we are dealing with a single database, we might move toward supporting other database in the future, too.
The name of that function should indicate that a) it is for database values and b) what database values it is for. In particular, PostgreSQL uses a different escaping style for CHAR values vs. BYTEARRAY values. That's why mine is named "sqlEscapeString"; "add_slashes_custom" is too generic a name, I think.
This said, I plan on dropping patch #1 in favor of using add_slashes_custom. It always be changed later if we move or rename the function.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Rod - How do you propose we elicit that feedback? I really don't have much exposure to any userbase other than my own, here. Should I just put an informal RFC on the User mailing list? Is the existing SQL that Brady didn't want to review acceptable?
All - I have uploaded a new patch set. It expands the first by altering the Misc. Billing Options form to use new the prior_auth table. I am currently working on supporting DB upgrades. This will also need a review and some testing.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
What do you think of database.sql changes. I'm using this as a learning case for me, and here are my naive comments for sql/database.sql:
-change the formatting of it to look like rest of the code in database.sql (unless I'm missing something here, which is a good possibility)
-restrict keyword in drop database line (per sql docs) seems to do nothing per docs (http://dev.mysql.com/doc/refman/5.5/en/drop-table.html)
-what do the 'references' commands do, don't see these showing up anywhere in database structure on phpmyadmin? (do they ensure the value exist in the other table? Is this a foreign key? If it's a foreign key, don't they only work in InnoDB? )
-why using integer for the id's when in rest of openemr using int(11)? (just curious)
/interface/patient_file/summary/prior_auths.php:
---require the formdata.inc.php script
---this is gonna sound stupid, and is optional. Best to not have more than one space in the translated strings (within xl()) since they will always get reduced to one space for the translators.
---Translate 'Create New'
---Translate 'Clear'
---Utilize the date widgets (see other scripts)
---Translate 'Delete'
Then I'd place a link to the /interface/patient_file/summary/prior_auths.php page in the left tree menu under the billing section. Then all users will be able to use this functionality(from the tree menu), but the widget in the patient summary screen will be optional.
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Unless you really want a link to prior auth management in the demographics page, I would suggest not putting it there at all. Regardless, it should be in the menus and don't forget to handle the case of $GLOBALS{'concurrent_layout'} = 0. This probably means adding an entry to the top menu that you see after selecting a patient.
Rod
www.sunsetsystems.com
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Re: Date Widgets - Absolutely, I just hadn't gotten around to it yet. Thanks for the reminder.
Re: UI - In the menu under billing makes more sense anyway. I'll be dropping the stuff from the patient summary (demographics) page.
Re: Translation - I hope I got everything this time. I went down Brady's list one by one. I'll try and scan the code again before I upload another version of the patchset.
BTW, Thanks for the commit bit.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
SQL Stuff:
- I think the layout is mostly the same once you drop the comment lines. I did try and model after the existing tables, although I did upcase SQL keywords and use tabs instead of spaces (for indentation). If it needs to change, through, give me a bit more direction and I can rearrange.
- Based on the SQL-200x draft I'm working off of, the "RESTRICT" keyword isn't optional. It may be ignored my MySQL since MySQL doesn't support using "CASCADE" there instead. Since MySQL accepts it, I would prefer to leave it; following the standard wherever possible. ("CASCADE" means to also drop other database objects that require that table; "RESTRICT" means it is an error if any other database objects require that table.)
- The "REFERENCES" clauses are one way to write foreign keys in standard SQL. MySQL accepts the syntax and passes the information down to the driver for that specific table type. The MyISAM table type ignores the information. (It might remember it for DESCribe operations, but it doesn't otherwise act on it.) The InnoDB table type does some checking. PostgreSQL does full foreign key enforcement and supports the "ON DELETE" and "ON UPDATE" clauses that commented. It is somewhat optional, but also serves as documentation for the relationship between the tables.
- INT(11) is non-standard. Also, the length is only used for padding with spaces in MySQL, it doesn't affect storage size or table layout. I can simply use "INT" instead of "INTEGER" if you prefer; they are two names for the same data type in standard SQL.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
"Do they ensure the value exist in the other table? Is this a foreign key?" - Mostly, NULL is also allowed even if it is not in the other table, unless the column is "NOT NULL" then it isn't allowed even if it is in the other table. Yes, it is a foreign key
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks for your detailed responses; very useful and informative for me. The only three questions I have left is why not explicitly setting the InnoDB vs. MyISAM, which is done in rest of database.sql ? Is there a better one to use (note almost all tables are now in MyISAM)? Is it an issue if a InnoDB table has a column that refers(foreign key) to a column in a MyISAM database?
thanks,
brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
regarding translations,
A good way to test for missing xl() function issues is to use the 'dummy' langauge. If a word isn't translated to dummy it means only one of two possibilities:
1) The word isn't contained in a xl() function
2) Or the word doesn't have a translation yet (will get sweeped up in the next english constants update).
The only reason not to set a table type is that it is non-standard SQL syntax. Plus, I don't know that it really matters. I prefer InnoDB, but that's because I expect an RDBMS to handle transactions in an ACID-compliant way, and last time I check MyISAM tables didn't provide the ACID guarantees.
Since then, MySQL has improved and I've learned that basically no open-source database provides ACID guarantees by default. (Well, SQLite does by locking the whole database during a transaction.) I'm not really informed enough to make a definitive decision. If most of the tables are currently MyISAM, I'll specify MyISAM. If any knows a good reason to use InnoDB (or another table driver) for this table, please speak up.
The documentation says that InnoDB requires the referenced table to also be InnoDB. I will double-check that by doing some testing. If that is the case, we'll need to use MyISAM for this table and let MySQL ignore the REFERENCES clause. It can remain for documentation purposes though.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've been following the forum postings for about 4 weeks, and have seen that the members are thorough, competent and all have full plates - which speaks for a good crew. I'm still trying to find a place to help and continue to evaluate OEMR for potential. My observations are that the activity seems focused on the scheduling and billing functions - which is understandable because these are key clinic functions, but reporting for quality improvement activities (some of the CCHIT stuff…) requires services, labs, and medications to be categorized and referenced to the demographic info. To insure that the patient information is updated and deleted accurately I see no good option but to use the referential integrity offered by InnoDb tables, Innodb transaction handling (rollbacks, db meltdowns) also meets the various requirements for database stability and integrity. Maybe these features are optional for a billing program but not a program used for chronic care management (which OEMR could be a platform for- and a most needed one), If you standardized to Innodb tables, at least for patient clinic data it'd serve future reporting needs handily.
Dusty Knobel
CDEMS.com
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I don't think any of the code currently in the tree requires atomicity or isolation to correctly function, yet. Consistency and durability are fairly well guaranteed by MySQL, independent of table engine.
That covers the ACID database properties. It is possible that, in the future, the code requires atomicity or isolation. At that point, we'll probably need to do code changes, and we can move over to InnoDB at that point. We'll need atomicity if we update multiple rows or multiple tables in a single logical operation, which means turning off auto-commit at least for that connection. We'll need isolation if we start doing relative ("SET column = column + 2" ) or dependent ("SET column = (SELECT count (*) FROM table WHERE something = TRUE") updates.
I would prefer to run OpenEMR on PostgreSQL instead of MySQL. IME, the consistency and durability guarantees of PostgreSQL are better, and it is easier to reliably recover from critical failures. (I'm thinking power failure in the middle on a multi-thousand row update, for example.)
Since the table I am referencing is not InnoDB, there may be issues with the new table being InnoDB. I can try it and report back.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Is this usable for the 4.1 version or already included? In Mental Health professions tracking this is mandatory. I currently have 3 practices that cannot use this software because of this feature missing. All three practices want a switch to OSS software. (I apologize for rekindling an old post, but there is a need here, just nobody is really addressing it)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Frank, we do not see the code/feature. It was probably dropped during QA review. However, if authorization tracking is the only missing feature for the practices, without any code changes enter the payer & authorization references as an 'issue'.
You can get fancy and if the practices are not going to use a specific built-in issue type (e.g. 'Dental Issues'), use built-in translator to rename it to 'Authorizations'.
With this approach, you can -
track auth effective period (start and end) dates.
associate the authorization to one or more encounters
associate specific conditions / codes with the authorization.
Try it out at the demo site and then find other extra information needed by the practices. If there is budget, you can extend the functionality even further for the community.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Artifact 3000786 - https://sourceforge.net/tracker/?func=detail&aid=3000786&group_id=60081&atid=1245239
Still needs some work, but it is functional so I thought I'd share it and hopefully get some feedback.
I'm going to update the misc billing options page to tie into the new db table. I am also going to work up migration sql and add it to the upgrade script. I also do intend to make the visual style match other pages / items on the page, but that was glossed over for now.
Still, please try it out, if you have time to do a new install, create a patient, and look at their summary page. The new content should be clear.
hey,
Placed a code review in the tracker.
Just so I'm clear (my billing knowledge is a bit weak), can you clarify the objective; is it for prior auth approval for MRI's etc?? If so, seems out of place to be putting this in the patient summary screen, but that's just my initial thoughts. I'd have to defer to the utility of this and best strategy to incorporate it to others, such as Tony, Rod, etc. Tony and Rod, any thoughts?
-brady
First, thanks for the review and links to documentation. I am currently fixing up the code based on your review.
As far as an objective goes, our agency wants to track prior authorizations for various purposes. Primarily, a prior authorization "number" needs to be included in the X12 we send to our payers. However, we also want to be able to notify clinicians and billing personnel when a patient is nearing the end of their prior authorizations and/or when they are scheduled for services that require prior auth., but they don't yet have one (or have not enough units left).
Our agency is mostly behavioral rather than medical, so MRIs wouldn't be an example of what we want to track, but rather Group, Individal, or Family therapy. These can often be scheduled months in advance, so tracking the prior authorizations in our EMR would be a boon.
From the information I am getting from the billing personnel and the developer that has been here longer, a prior authorization is identified by a "number" (I use quotes, because I'm not sure it is all digits; letters might be included), and is for a particular patient to receive a number of units (e.g. 4 - 1 hour) of a certain service (e.g. HCPCS:90847 - Family Therapy w/ Patient) within a certain date range.
A single prior authorization number is kept on the misc. billing options form, but this doesn't provide us enough information to do any real tracking or validation. I expect to replace the text field on the misc. billing options form with a drop-down of prior authorizations populated form the database.
Since the prior authorizations are attached to a particular patient but not necessarily a single encounter, I figure the patient summary screen would be a simple location for them. However, I'm certainly open to ideas for how to handle this.
I guess we're gonna need some more feedback about the needs of various types of practices. Just have to make sure this can work for everyone.
Rod
www.sunsetsystems.com
I think the function that escapes strings destined for the database should be kept the the file with all the rest of the database abstraction layer. It might be useful to escape things from locations other than form data, so it doesn't belong with the form data processing. While it is easy enough to pre-escape anything "hard-coded" when we are dealing with a single database, we might move toward supporting other database in the future, too.
The name of that function should indicate that a) it is for database values and b) what database values it is for. In particular, PostgreSQL uses a different escaping style for CHAR values vs. BYTEARRAY values. That's why mine is named "sqlEscapeString"; "add_slashes_custom" is too generic a name, I think.
This said, I plan on dropping patch #1 in favor of using add_slashes_custom. It always be changed later if we move or rename the function.
Rod - How do you propose we elicit that feedback? I really don't have much exposure to any userbase other than my own, here. Should I just put an informal RFC on the User mailing list? Is the existing SQL that Brady didn't want to review acceptable?
All - I have uploaded a new patch set. It expands the first by altering the Misc. Billing Options form to use new the prior_auth table. I am currently working on supporting DB upgrades. This will also need a review and some testing.
This is the RFC. :-) I think users who care about development plans will check in here.
I do like the idea of a tracking system for prior authorizations, and will take a look at your code shortly.
Rod
www.sunsetsystems.com
OK I have posted some feedback to your tracker item.
Rod
www.sunsetsystems.com
Rod and Stephen,
What do you think of database.sql changes. I'm using this as a learning case for me, and here are my naive comments for sql/database.sql:
-change the formatting of it to look like rest of the code in database.sql (unless I'm missing something here, which is a good possibility)
-restrict keyword in drop database line (per sql docs) seems to do nothing per docs (http://dev.mysql.com/doc/refman/5.5/en/drop-table.html)
-what do the 'references' commands do, don't see these showing up anywhere in database structure on phpmyadmin? (do they ensure the value exist in the other table? Is this a foreign key? If it's a foreign key, don't they only work in InnoDB? )
-why using integer for the id's when in rest of openemr using int(11)? (just curious)
Review of stuff I know more about:
interface/forms/misc_billing_options/new.php
---fix trailing space in xl() function for BOX 23. Prior Authorization No. ( http://www.openmedsoftware.org/wiki/Development_Policies#Internationalization )
---place 'None' within translation xl() function
interface/forms/misc_billing_options/save.php:
---It looks like your running the POST 'pa_id' variable through formData twice
interface/forms/misc_billing_options/view.php:
---fix trailing space in xl() function for BOX 23. Prior Authorization No. ( http://www.openmedsoftware.org/wiki/Development_Policies#Internationalization )
---place 'None' within translation xl() function
/interface/patient_file/summary/prior_auths.php:
---require the formdata.inc.php script
---this is gonna sound stupid, and is optional. Best to not have more than one space in the translated strings (within xl()) since they will always get reduced to one space for the translators.
---Translate 'Create New'
---Translate 'Clear'
---Utilize the date widgets (see other scripts)
---Translate 'Delete'
interface/patient_file/summary/stats.php:
---I'd rec making this widget 1)fit in with the page 2)place at the bottom of everything on right 3)make the visibility of this widget optional ( http://www.openmedsoftware.org/wiki/Development_Policies#Creating_a_global_configuration_setting )
Then I'd place a link to the /interface/patient_file/summary/prior_auths.php page in the left tree menu under the billing section. Then all users will be able to use this functionality(from the tree menu), but the widget in the patient summary screen will be optional.
-brady
Unless you really want a link to prior auth management in the demographics page, I would suggest not putting it there at all. Regardless, it should be in the menus and don't forget to handle the case of $GLOBALS{'concurrent_layout'} = 0. This probably means adding an entry to the top menu that you see after selecting a patient.
Rod
www.sunsetsystems.com
Re: Date Widgets - Absolutely, I just hadn't gotten around to it yet. Thanks for the reminder.
Re: UI - In the menu under billing makes more sense anyway. I'll be dropping the stuff from the patient summary (demographics) page.
Re: Translation - I hope I got everything this time. I went down Brady's list one by one. I'll try and scan the code again before I upload another version of the patchset.
BTW, Thanks for the commit bit.
SQL Stuff:
- I think the layout is mostly the same once you drop the comment lines. I did try and model after the existing tables, although I did upcase SQL keywords and use tabs instead of spaces (for indentation). If it needs to change, through, give me a bit more direction and I can rearrange.
- Based on the SQL-200x draft I'm working off of, the "RESTRICT" keyword isn't optional. It may be ignored my MySQL since MySQL doesn't support using "CASCADE" there instead. Since MySQL accepts it, I would prefer to leave it; following the standard wherever possible. ("CASCADE" means to also drop other database objects that require that table; "RESTRICT" means it is an error if any other database objects require that table.)
- The "REFERENCES" clauses are one way to write foreign keys in standard SQL. MySQL accepts the syntax and passes the information down to the driver for that specific table type. The MyISAM table type ignores the information. (It might remember it for DESCribe operations, but it doesn't otherwise act on it.) The InnoDB table type does some checking. PostgreSQL does full foreign key enforcement and supports the "ON DELETE" and "ON UPDATE" clauses that commented. It is somewhat optional, but also serves as documentation for the relationship between the tables.
- INT(11) is non-standard. Also, the length is only used for padding with spaces in MySQL, it doesn't affect storage size or table layout. I can simply use "INT" instead of "INTEGER" if you prefer; they are two names for the same data type in standard SQL.
"Do they ensure the value exist in the other table? Is this a foreign key?" - Mostly, NULL is also allowed even if it is not in the other table, unless the column is "NOT NULL" then it isn't allowed even if it is in the other table. Yes, it is a foreign key
hey,
Thanks for your detailed responses; very useful and informative for me. The only three questions I have left is why not explicitly setting the InnoDB vs. MyISAM, which is done in rest of database.sql ? Is there a better one to use (note almost all tables are now in MyISAM)? Is it an issue if a InnoDB table has a column that refers(foreign key) to a column in a MyISAM database?
thanks,
brady
clarifcation above:
'MyISAM database' should be 'MyISAM table'
regarding translations,
A good way to test for missing xl() function issues is to use the 'dummy' langauge. If a word isn't translated to dummy it means only one of two possibilities:
1) The word isn't contained in a xl() function
2) Or the word doesn't have a translation yet (will get sweeped up in the next english constants update).
If curious, more on the translation project here:
http://www.openmedsoftware.org/wiki/OpenEMR_Internationalization_Development
And can find a README here describing the scripts to collects cosntants and build tables etc.:
http://openemr.cvs.sourceforge.net/viewvc/openemr/openemr/contrib/util/language_translations/
-brady
The only reason not to set a table type is that it is non-standard SQL syntax. Plus, I don't know that it really matters. I prefer InnoDB, but that's because I expect an RDBMS to handle transactions in an ACID-compliant way, and last time I check MyISAM tables didn't provide the ACID guarantees.
Since then, MySQL has improved and I've learned that basically no open-source database provides ACID guarantees by default. (Well, SQLite does by locking the whole database during a transaction.) I'm not really informed enough to make a definitive decision. If most of the tables are currently MyISAM, I'll specify MyISAM. If any knows a good reason to use InnoDB (or another table driver) for this table, please speak up.
The documentation says that InnoDB requires the referenced table to also be InnoDB. I will double-check that by doing some testing. If that is the case, we'll need to use MyISAM for this table and let MySQL ignore the REFERENCES clause. It can remain for documentation purposes though.
Here's an article about InnoDV vs. MyISAM: http://www.mikebernat.com/blog/MySQL_-_InnoDB_vs_MyISAM
There's no clear winner. For now let's write code that works with either.
Rod
www.sunsetsystems.com
I've been following the forum postings for about 4 weeks, and have seen that the members are thorough, competent and all have full plates - which speaks for a good crew. I'm still trying to find a place to help and continue to evaluate OEMR for potential. My observations are that the activity seems focused on the scheduling and billing functions - which is understandable because these are key clinic functions, but reporting for quality improvement activities (some of the CCHIT stuff…) requires services, labs, and medications to be categorized and referenced to the demographic info. To insure that the patient information is updated and deleted accurately I see no good option but to use the referential integrity offered by InnoDb tables, Innodb transaction handling (rollbacks, db meltdowns) also meets the various requirements for database stability and integrity. Maybe these features are optional for a billing program but not a program used for chronic care management (which OEMR could be a platform for- and a most needed one), If you standardized to Innodb tables, at least for patient clinic data it'd serve future reporting needs handily.
Dusty Knobel
CDEMS.com
I don't think any of the code currently in the tree requires atomicity or isolation to correctly function, yet. Consistency and durability are fairly well guaranteed by MySQL, independent of table engine.
That covers the ACID database properties. It is possible that, in the future, the code requires atomicity or isolation. At that point, we'll probably need to do code changes, and we can move over to InnoDB at that point. We'll need atomicity if we update multiple rows or multiple tables in a single logical operation, which means turning off auto-commit at least for that connection. We'll need isolation if we start doing relative ("SET column = column + 2" ) or dependent ("SET column = (SELECT count (*) FROM table WHERE something = TRUE") updates.
I would prefer to run OpenEMR on PostgreSQL instead of MySQL. IME, the consistency and durability guarantees of PostgreSQL are better, and it is easier to reliably recover from critical failures. (I'm thinking power failure in the middle on a multi-thousand row update, for example.)
Since the table I am referencing is not InnoDB, there may be issues with the new table being InnoDB. I can try it and report back.
Is this usable for the 4.1 version or already included? In Mental Health professions tracking this is mandatory. I currently have 3 practices that cannot use this software because of this feature missing. All three practices want a switch to OSS software. (I apologize for rekindling an old post, but there is a need here, just nobody is really addressing it)
Frank, we do not see the code/feature. It was probably dropped during QA review. However, if authorization tracking is the only missing feature for the practices, without any code changes enter the payer & authorization references as an 'issue'.
You can get fancy and if the practices are not going to use a specific built-in issue type (e.g. 'Dental Issues'), use built-in translator to rename it to 'Authorizations'.
With this approach, you can -
Try it out at the demo site and then find other extra information needed by the practices. If there is budget, you can extend the functionality even further for the community.