Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New erx 6 #1017

Closed
wants to merge 9 commits into from
Closed

New erx 6 #1017

wants to merge 9 commits into from

Conversation

juggernautsei
Copy link
Member

@juggernautsei juggernautsei commented Aug 18, 2017

Posting PR as requested

Up for grabs demo here:
http://www.open-emr.org/wiki/index.php/Development_Demo#192.168.1.135

@bradymiller
Copy link
Sponsor Member

@juggernautsei ,
I think this is getting close :)
Is this code testing well?

@juggernautsei
Copy link
Member Author

I am going to test a fresh install from scratch and see if the code works. I will update you later.

@bradymiller
Copy link
Sponsor Member

Just created an up for grabs demo for this PR here:
http://www.open-emr.org/wiki/index.php/Development_Demo#192.168.1.135

@juggernautsei
Copy link
Member Author

juggernautsei commented Aug 29, 2017

This is not testing well right now. The provider data is not populating. I feel like I am troubleshooting someone else's code. Trying to figure out how the doc info gets populated. Update you as soon as I get this fixed.

Got most everything back in working order. The clues were there just had to follow them. Waiting on Tina to update our account info. Right now the return is
Open Med Practice|2017-08-29|16:08:33|||600|IP is not registered.
Once that is corrected, will test again and report back.

…n made and the confirm script had a phpstorm suggestion that was updated to the suggestion.
@juggernautsei
Copy link
Member Author

juggernautsei commented Aug 31, 2017

The system is testing well now.
Weno Exchange LLC|2017-08-31|01:55:49.56314Z|001|Transaction successful||

Anyone that follows the instructions for testing should receive this type of message.

@bradymiller bradymiller self-assigned this Aug 31, 2017
echo text($lines). "<br>";

$i = 2;

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To drastically speed up this import of data, place the following lines here:

// Settings to drastically speed up import with InnoDB
sqlStatementNoLog("SET autocommit=0");
sqlStatementNoLog("START TRANSACTION");

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need to address this here
(also note the note I'll repost below that should be done after below code snippet)
(this will drastically increases the speed of this large import)

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juggernautsei ,
Still need to address this. see my post below for details.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

echo xlt("Inserted") . " " . text($drugName) . "<br> ";

$i++;
} while ($i < $lines);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To close out the above setting, then do the following her (after importing above data):

// Settings to drastically speed up import with InnoDB
sqlStatementNoLog("COMMIT");
sqlStatementNoLog("SET autocommit=1");

$price = $in[5];
$drugName = $in[8];

sqlStatement("INSERT INTO `erx_drug_paid` SET `drug_label_name` = ?, `NDC` = ?, `price_per_unit` = ? ", array($drugName,$ndc,$price));
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since do not need to log all these inserts, change to sqlStatementNoLog() function:

sqlStatementNoLog("INSERT INTO `erx_drug_paid` SET `drug_label_name` = ?, `NDC` = ?, `price_per_unit` = ? ", array($drugName,$ndc,$price));

} else {
sqlStatement("INSERT INTO `erx_drug_paid` SET `drug_label_name` = ?, `NDC` = ?, `price_per_unit` = ? ", array($drugName,$ndc,$price));
echo xlt("Inserted")."<br> ";
}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused on this code block from line 43-53.
There is no difference between these 2 different queries. Can this just all be changed to:

sqlStatementNoLog("INSERT INTO `erx_drug_paid` SET `drug_label_name` = ?, `NDC` = ?, `price_per_unit` = ? ", array($drugName,$ndc,$price));
echo xlt("Inserted")."<br> ";

If we can simply this, then can use the innodb strategy used above to drastically increase the speed of the update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

//See if the value is set
$check = "SELECT ntx FROM prescriptions WHERE id = ?";
$getVal = sqlStatement($check, array($e_script[1]));
$val = sqlFetchArray($getVal);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are just grabbing one item, wouldn't sqlQuery be better? (then don't need the sqlFetchArray step)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok will do.

while ($row = sqlFetchArray($res)) {
$return_arr[] = $row['drug_label_name'] . " - ". $row['price_per_unit'];
}
} catch (PDOException $e) {
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd since the database connector is not even PDO???

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left over from the original code that was PDO.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then would completely remove this try/catch (note sqlStatement outputs errors already)
as an aside, does these even work since the S in sqlstatement is not capitalized??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

while ($row = sqlFetchArray($res)) {
$return_arr[] = $row['id'] . " - ".$row['Store_name'] . " ".$row['address_line_1']." ". $row['city'] ." " . $row['state'];
}
} catch (PDOException $e) {
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd since the database connector is not even PDO???

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a left over from the original code.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then would completely remove this try/catch (note sqlStatement outputs errors already)
as an aside, does these even work since the S in sqlstatement is not capitalized??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will remove try catch blocks.


sqlInsert($sqlNarc);

return "Narcotic drugs imported<br>";
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use xlt() on the string here

*/
$sqlNarc = file_get_contents('../../contrib/weno/narc.sql');

sqlInsert($sqlNarc);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how long does this import take?
If it takes awhile, let me know, and I can show you how to drastically reduce the time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That statement was from the first round before switching to the SQL version. I don't know how long it takes now.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying this function is no longer used? If so, then would remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I am saying the words "Be patient this can take a while" can be removed.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the sqlStatementNoLog() function here instead.

return $res;
}

public function checkList($send)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't appear to be used anywhere and is odd (to start with a sqlFetchArray). If it's not used, then would remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

@@ -256,10 +265,24 @@
{$ENDING_JAVASCRIPT}
</script>
</form>
<!-- Has to be loaded after the form -->
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move these to the bottom? Rec moving them to the top where include the css stuff. This is likely why getting the following js errors when open the prescription Add gui:
screenshot from 2017-09-02 21-18-38

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not move anything in this area.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AAAARRRRRRR!!! I keep fixing this and it keeps getting broken :-(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jquery-ui.min.css
This file is missing.
Two version back, I placed this file in the library/js folder and now it is no longer there and the code needs it to run properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the top and it works still so leaving and removing the note.

$(function() {

//Drug form autocomplete to text box.
$(".wDrug").autocomplete({
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this function is specific to weno rx, then would only output this if he weno rx feature is on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to do this. I have tried and

{php} if ($GLOBALS['weno_rx_enable']) { {/php}

{php} } {/php}

I think because of the {literal} function no matter what is typed in there will be output.

<td COLSPAN="1" class="text" ALIGN="right" VALIGN="MIDDLE" >{xl t='Starting Date'}</td>
<td COLSPAN="2" ALIGN="LEFT" VALIGN="MIDDLE" >
{html_select_date start_year="-10" end_year="+5" time=$prescription->start_date prefix="start_date_"}
<input type="hidden" name="start_date" value="{$prescription->start_date}">
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add the above line for a bug fix? This is a new line of code. Note it's easier to see these things when look at this code while ignoring whitespace which can do with following link(note the ?w=1 at end of link):
https://github.com/openemr/openemr/pull/1017/files?w=1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I did not add that line of code. I have not recently edited this file.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git is clearly showing the above line is not in main codebase, but is in your code. If you did not add that line, then rec removing it.
If you want to get a clearer idea of what code you added to this file, check out this diff which ignores whitespace changes:
https://github.com/openemr/openemr/pull/1017/files?w=1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my proof that I did not add the lines 159-162. In the image below, I went back to where I did the dragndrop mod which has been untouched since then. The window on the left shows the file date. Look at the top of the editor, you can see that I am in the dragdrop folder. In the editor are the same lines from April of this year. This is from the main code base. I always fetch before creating a new branch. That code has been there long before April and I did not add it. Can you be clear for me, you want me to remove lines 159-162?
not_me

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only 1 line at 162. don't worry about this; it's minor and i'll fix it up when get this into the codebase (have fingers crossed that it will get into the codebase tonight :) )

@bradymiller
Copy link
Sponsor Member

Hi @juggernautsei ,
Just finished code review and testing. This is getting very close. Let me know when it's ready for the next review (guessing it will be ready for the main codebase at that point).
-brady

@juggernautsei
Copy link
Member Author

New updates are ready for inspection.

echo xlt("Inserted") . " " . text($drugName) . "<br> ";

$i++;
} while ($i < $lines);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

below while code, do the following:

// Settings to drastically speed up import with InnoDB
sqlStatementNoLog("COMMIT");
sqlStatementNoLog("SET autocommit=1");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually just do what you ask no questions and try to understand later. Sometimes it is obvious after I read the instructions and I add it to my skills.
This one is over my head enough that I want to ask a question. Placing the statements after the while loop means they will only execute after the loop is finished. How is that going to speed up the insert process?

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, we are wrapping the large insert with settings to increase the innodb performance. Note you also need to place the settings above the while loop in my other post above. For inserts, innodb touches the hard drive after every insert for log/robust issues. This can be very expensive when inserting hundreds of records(for example, the setup script in openemr could take up to 45 minutes because of 100,000+ translations), so we are basically turning this off temporarily before the insert, and then turning it back on after the insert.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood.

*
*/
$sqlNarc = file_get_contents('../../contrib/weno/narc.sql');

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above the sqlStatementNoLog, do the following:

// Settings to drastically speed up import with InnoDB
sqlStatementNoLog("SET autocommit=0");
sqlStatementNoLog("START TRANSACTION");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

$sqlNarc = file_get_contents('../../contrib/weno/narc.sql');

sqlInsert($sqlNarc);

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

below the sqlStatementNoLog(), do the following:

// Settings to drastically speed up import with InnoDB
sqlStatementNoLog("COMMIT");
sqlStatementNoLog("SET autocommit=1");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bradymiller
Copy link
Sponsor Member

hi @juggernautsei ,

I just brought in your feature into the codebase. thank you for your awesome contribution! I made a couple minor changes.

Here's your original code:
049d45a

Here are a couple PSR2 changes:
cec9566

And here were a couple of other changes/fixes. (Mostly needed to fix the autocomplete feature so it didn't break the original autocomplete from the drugs table. So, when not using weno, it will use original drugs table autocomplete, and when using weno, it will autocomplete from your formulary table):
04b6d41

-brady

@bradymiller bradymiller closed this Sep 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants