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.
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?
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 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.
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.
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.
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.
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
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?
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.
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.