New Developer + Immunization Changes

OpenEMR Development
1

jansta23 wrote on Thursday, March 14, 2013:

Hi everyone,

I am Jan and I recently signed up as  a developer.

I am new to the open source community so please bear with me as I learn the ins and out of the community.

I recently worked with Brady to implement the Immunization Information changes.

Just a quick summary about the project and what I have changed:

1. /interface/patient_file/summary/immunizations.php
    - I added the new fields specified in the requirement and populate the fields during edit
    - I implemented the “added erroneously” section
              - I added a checkbox to specify if the record is an error, doing so update the table and puts strikethrough on the entry. The main screen containing the immuniz info will also display a message in bold red stating that the record is an error.
    - I am currently storing the unit of amount of immunization as a string in the database, as opposed to the index/id of the unit. My reasoning here is that the I was not able to put a relationship between immunization table and list_options table. Therfore, if the list_options changed, the immunization unit does not get affected.
2. /interface/patient_file/summary/shot_record.php
    - I am displaying the records (that were not added erroneously), on the pdf or html when user chooses to print the records.
3. /interface/patient_file/summary/stats.php
    - I added a condition to only display the records that were not added erroneously.
4. Immunizations Table
    - still trying to find out how to push the changes to the mysql table. 

Repository:
https://github.com/jajalla23/openemr/tree/jajalla23

Let me know if you guys have any suggestions or comments. Thanks in advance!

2

yehster wrote on Thursday, March 14, 2013:

Looks good in general, but I haven’t had time to test it out yet.
If you get a chance, take a look at
http://www.open-emr.org/wiki/index.php/Codebase_Security#SQL-Injection_and_Cross-Scripting_Prevention
Specifically step 4 regarding XSS, and take a look at the documentation in the htmlspecialchars php file.

Try not to include your modified version of sqlconf.php in the commit.  It makes it harder for others to just pull and test your changes.  The best way to do that depends on your development environment.  Not a big deal for now.

3

yehster wrote on Thursday, March 14, 2013:

This is what code looks like to update tables for patching.

#IfMissingColumn procedure_questions tips
ALTER TABLE `immunizations`
  ADD COLUMN `amount_administered_unit` varchar(255) NOT NULL DEFAULT '';
#EndIf

It will go into
https://github.com/yehster/openemr/blob/master/sql/4_1_1-to-4_1_2_upgrade.sql

Table creation also needs to be updated in:
https://github.com/yehster/openemr/blob/master/sql/database.sql#L2221

4

jansta23 wrote on Friday, March 15, 2013:

Thanks Kevin. The link is really helpful.

Small question about Git, do I need to create a new branch every time I wish to push a change?  Seems like my new changes are not showing up to the first branch I created

5

yehster wrote on Friday, March 15, 2013:

No you don’t need a new branch.  The sequence you need to share changes would be add, commit, push and most likely will look something like this:

git add .
git commit -m "Description of my changes"
git push git@github.com:jajalla23/openemr jajalla23
6

bradymiller wrote on Saturday, March 16, 2013:

Hi Jan,

I placed a code review on github.

thanks,
-brady
OpenEMR

7

jansta23 wrote on Monday, March 18, 2013:

Thanks so much Kevin and Brady for the help!

I have checked in my changes as per your comments and suggestions. Please let me know if they are good.

Also, I discovered a bug when a new immunization has been added to a new patient and the print pdf has been clicked. When back is clicked, another entry with similar value to earlier entry gets added. I am not sure if this is a legitimate bug or it’s due to myself click the back button on the browser.

Another bug I noticed is that the administered date does not get displayed on the pdf (but it does on print html). Seems like when the entry is too long, the pdf has an issue displaying all the columns.When i change the format to yyyy-mm-dd, it appears but when i include the hour and minutes, the administration  date disappears

8

yehster wrote on Tuesday, March 19, 2013:

Jan,
I would consider the duplicate posting a “legitimate bug.”  I see something similar when I “refresh the frame”  and there isn’t a warning about resending post data as is often seen on other pages. 

9

bradymiller wrote on Friday, March 22, 2013:

Hi Jan,

The above link/commit is wrong. Sorry. Here is the correct link:
http://github.com/bradymiller/openemr/commit/f61a348fefd4170dcfec26ed4b11b44644e9c52f
(to avoid confusion, I will delete the previous post)

To  make the review easier for us, I have squashed your branch into one commit (and rebased it to most recent main codebase) and placed it in above commit.

I should have time over next several days to review (I’ll let you know when I finish the review).

thanks,
-brady
OpenEMR

10

bradymiller wrote on Saturday, March 23, 2013:

Hi Jan,

My review is done. Placed it here:
http://github.com/bradymiller/openemr/commit/f61a348fefd4170dcfec26ed4b11b44644e9c52f

To see my comments, use the web browser to search for ‘bradymiller’ . Let me know your thoughts on the issues in the interface/patient_file/summary/shot_record.php script or if any other issues/questions. Looking very forward to the next revision.

thanks,
-brady
OpenEMR

11

jansta23 wrote on Thursday, April 11, 2013:

Brady/Kevin,

I have checked in my changes as per your suggestions. I created a common code that is being used by immunizations and shot_record (to generate pdf and html). Named the file immunizations_helper.php under library.

Please let me know how the code looks when you get the chance.

12

bradymiller wrote on Friday, April 12, 2013:

Hi Jan,

Thanks for submitting the next revision; look very forward to testing and reviewing it. I’ve squashed your code into one commit and rebased it to the most recent codebase to make it easier to review/test. Plan to review it over the weekend:

thanks,
OpenEMR

13

bradymiller wrote on Friday, April 12, 2013:

oops,

Here is the link to the commit:

I’ll let you know when I finish the review (or if anybody else reviews it remember to let Jan know since he won’t get the emails from github since it is in my repo).

thanks,
-brady
OpenEMR

14

bradymiller wrote on Saturday, April 13, 2013:

Hi Jan,

Finished the review. Very nice progress. On the commit, search for bradymiller to ensure you see all of my comments:

Recommend you now work from this branch, which is:

To get this branch in your repo, do this:
git remote add brady git://github.com/bradymiller/openemr.git
git fetch brady
git checkout brady/jajalla23-bundle-commits_04_11_2013
(ignore the warning message)
git checkout -b jajalla23-immuniz

And at that point you will now have your own local repo branch jajalla23-immuniz to work from.

Looking very forward to the next revision.

-brady
OpenEMR

15

bradymiller wrote on Sunday, April 28, 2013:

Hi,

Here’s is Jan’s recent revision (with a small amount of integration work by myself), which I think is ready to be committed into the official codebase. This will also fulfill criteria for MU item 170.314(f1):

-brady
OpenEMR

16

bradymiller wrote on Wednesday, May 01, 2013:

Hi,

Just pushed above commit to sourceforge.

-brady
OpenEMR

17

yehster wrote on Wednesday, May 01, 2013:

It seems a little odd that you can toggle the “added_erroneously” flag back and forth, and delete should probably no longer be an option. I suspect the certification process will view the existence of the delete button in a negative light.

18

bradymiller wrote on Monday, May 06, 2013:

Hi,
Agree a bit awkward, although perhaps Delete is an Admin (superuser) function while the flag to erroneous could require less access to do since not as potentially as harmful as a full delete. Regarding the certification process, if it’s like the last time, they don’t seem to care about anything else as long as the testing scripts pass(although Tony and Visolve know much more about the testing process).
-brady
OpenEMR