Improving Adjustment Reasons

sunsetsystems wrote on Friday, November 19, 2010:

I have a client using 3.2 that’s often asking me to add new entries to the list of “adjustment reasons” which is referenced by the manual EOB entry page.  This becomes awkward because there’s some hard-coded logic in there that decides if an adjustment affects the invoice balance, or if it just shifts payment responsibility to the patient.

So I changed this list to include a “type” attribute for this purpose.  The associated 3.2.0 patch can be found here:

https://github.com/sunsetsystems/openemr/tree/adj320

I’ll port this to current code if there are no objections, but mainly I want to ask if it should be applied to the rel-320 branch since that is already released and stable.  What do y’all think?

Rod
www.sunsetsystems.com

bradymiller wrote on Friday, November 19, 2010:

Rod,

Since your modifying the database, I’d rec. not putting this into the rel-320 branch (at least for now). I’d post it in the tracker as a patch (use the same mechanism as the official patches; ie. files placed in correct directories so when user opens it in the openemr directory the new modified files then overwrite it) with instructions on how to install it. These type of patches are nice because any OS and person can unzip a file; it becomes much more complicated when you need to have the user use patches or modify the sql tables. In the official patches, I did break this rule for the lang_custom table, but I made the code failsafe (works with and without the lang_custom table), and placed very dumbed down instructions for users on how to implement/install this feature, if wanted:
http://www.openmedsoftware.org/wiki/Configure_Language_Editor_Improvements_For_3.2

-brady

sunsetsystems wrote on Friday, November 19, 2010:

Hmm.  Well there is no schema change, the only database mod is to assign nonzero values for list_options.option_value for a particular list where that column was previously unused.  And the code will in fact work if that is not done, though you will get warning messages.  Does that affect your opinion?

Rod
www.sunsetsystems.com

bradymiller wrote on Saturday, November 20, 2010:

hey,
If the column(the reason type I assume) is still set to zero, then is there a way to go back to previous behavior without any warnings? Then it would be ok if also included dumbed down instructions on modifying the database (for those who want this feature) with the next official patch release. However, if its gonna throw new warnings and expect users to then need to figure out how to upgrade their database, then do not recommend putting in rel-320 (or the next official patch release). The current patch mechanism is very simple (download most recent official patch zip and extract it); not a good idea to demand other mods with our current “patch release” mechanism. Another important issue is how vital this patch is; ie. is it worth yours (and my time) to get it in rel-320 in an appropriate manner.
-brady

sunsetsystems wrote on Saturday, November 20, 2010:

Not vital, so I’ll not worry about it for 3.2.0 unless there is demand.  Thanks for the input!

Rod
www.sunsetsystems.com

sunsetsystems wrote on Tuesday, November 23, 2010:

This has been ported to current code and committed.

Rod
www.sunsetsystems.com

bradymiller wrote on Tuesday, November 23, 2010:

hey,
You brought in a trivial merge commit. Did you stray from the commit instructions(if not, then may need to optimize the instructions a bit)?
-brady

sunsetsystems wrote on Tuesday, November 23, 2010:

Yeah I thought that was a bit weird.  I’m pretty sure I did it all by the book.  But I think Paul did a push shortly before mine but after my working branch was created.  My last steps were:

git pull origin master  (this pulled in new stuff)
git fetch origin
git rev-list master -not origin/master
git checkout master
git pull origin master
git merge work6
git log | less
git push origin master

Rod
www.sunsetsystems.com

sunsetsystems wrote on Tuesday, November 23, 2010:

Oh, guess I forgot the rebase.  Sorry.

Rod
www.sunsetsystems.com

bradymiller wrote on Tuesday, November 23, 2010:

lol,
Just a trivial commit, but avoiding them makes our history more clear. Ignore github’s diff of the merge commits (not accurate). A nice way to visualize these merge commits is to use qgit; simply run it from the working directory (after installing qgit, of course). qgit is very helpful in understanding what git is doing with these merges, and it will also tell you if the merge introduced any changes (or if it was simply a trivial merge).
-brady