Locating files using Smarty-Template-Engine's Syntax for refactoring

[nileshprasad137]

This issue is created to list down all the files using Smarty Template's syntax, and for discussion.

Other than all the files contained inside templates directory, there are several other files which need to looked after. List of files ::

• [x] Administration/Practice Complete Section

• [x] interface/forms/vitals/

• [ ] interface/forms/vitalsM/(not used anywhere, has to be deleted)

• [x] interface/forms/soap/

• [x] interface/forms/ros/

• [x] interface/forms/prior_auth/templates/prior_auth/

• [x] interface/clickmap/

• [x] interface/annotate_diagram/map_diagram/

• [ ] GACL Section

• [ ] Billing Contact Controller and template(not used anywhere, has to be deleted)

• [x] Prescription Controller and template

• [ ] Patient Finder controller and template(not used anywhere, has to be deleted)

• interface/main/calendar Ujjwal's project

Edit PR for the issue - https://github.com/LibreHealthIO/LibreEHR/pull/598 #614 #665

[aethelwulffe]

/soap /ros can be dropped completely and replaced with modern forms later. They are despicable.

[nileshprasad137]

Just to update you all here, I will maintain MVC pattern which is currently followed in the forms. I will make appropriate changes in the controller files in order to achieve this.

Also, I intend to use Bootstrap in new forms.

[nileshprasad137]

I will leave current forms as it is, so that current functionality doesn't break

[nileshprasad137]

UPDATE: Administration/Practice Section is being almost made independent of Smarty class. Only Documents tab is left. Would complete that mostly by tomorrow. Currently, I have made changes in Controller class to make it work without Smarty class. After I am done removing Smarty from Documents tab , I'll proceed towards UI refactoring.

[aethelwulffe]

Nice.

[nileshprasad137]

I am done removing Smarty from Admin/Practice . You may test it by downloading this branch https://github.com/nileshprasad137/FormsMVC-Application/tree/admin_prac in a folder and change the path /controller.php?practice_settings&pharmacy&action=list to /[new_folder]/controller.php?practice_settings&pharmacy&action=list from Edit-menu

[nileshprasad137]

I haven't changed the UI part currently. Just removed Smarty class and replaced its various functions.

[aethelwulffe]

Gimmie a minute to break this first :)

[nileshprasad137]

Sure! And if possible please test Documents tab functionality. I am not fully aware of its deep functionalities. The document tree was getting displayed correctly , so I presumed that it might be working well.

[aethelwulffe]

Crud. I don't even remember what version of that document_tree stuff we are actually using at the moment. The file manger stuff is a mess too....

[nileshprasad137]

As far as I know, it is working the same way as the Documents tab before removing smarty.

[aethelwulffe]

Seems to be so. I haven't cracked anything yet. Terry...Tony...Ujj...Pri...you guys want to check this out? Looks like a good stage to push this to me. -Assuming the configuration for production is added to this.

[teryhill]

I will be working it into my system shortly.

[teryhill]

I got it in I think. I just downloaded the zip and unziped it into controllers_2. It has various directories in it Is that correct? @nileshprasad137

[nileshprasad137]

yes

[nileshprasad137]

But that is not the master branch of that repository. Note that.

[teryhill]

I changed the url to point to the directory controllers_2 Directory structure below

I get these errors in my phperror log

[15-Jun-2017 16:14:41 America/New_York] PHP Warning: mysqli_real_connect(): (HY000/1045): Access denied for user 'libreehr'@'localhost' (using password: YES) in C:\xampp\htdocs\LibreEHR\controllers_2\library\adodb\drivers\adodb-mysqli.inc.php on line 123

[15-Jun-2017 16:14:41 America/New_York] PHP custom error: from librehealth ehr library/sql.inc - Unable to set up UTF8 encoding with mysql database: Access denied for user 'libreehr'@'localhost' (using password: YES)

[15-Jun-2017 16:14:41 America/New_York] PHP Fatal error: Call to undefined function text() in C:\xampp\htdocs\LibreEHR\controllers_2\library\sql.inc on line 510

I am using the role based menus.

[aethelwulffe]

I only checked it from his branch...

[nileshprasad137]

Couldn't connect to database! why ... @aethelwulffe any idea? Do you face the same error while downloading and running? I'll look at it in the morning.

[aethelwulffe]

Collation?

[teryhill]

Got it working Guy's

[teryhill]

Never mind still using the old yunk

[nileshprasad137]

Yep, I previously said ,

I haven't changed the UI part currently. Just removed Smarty class and replaced its various functions

But , is the same functionality being maintained after removing Smarty?

[nileshprasad137]

Is this a bad approach? I asked it on the forums few days back, if I should proceed with a new MVC and use PDO fordatabase connections and Kevin said

A switch to PDO (or other database layer) from ADODB would require a re-implementation of the audit logging mechanisms currently provided by sql.inc.php which is no small task and would ultimately require dual maintenance of the audit code (current code which relies on ADODB is going to be with us for a long time.)

I am open to any suggestions you have . Otherwise, I can proceed to remove smarty in the same way and later revamp the UI..

EDITED:

Link to that forums post

New MVC I was working on -> https://github.com/nileshprasad137/FormsMVC-Application

Please clarify which approach I should be using before I proceed forward..

[nileshprasad137]

`[15-Jun-2017 16:14:41 America/New_York] PHP Warning: mysqli_real_connect(): (HY000/1045): Access denied for user 'libreehr'@'localhost' (using password: YES) in C:\xampp\htdocs\LibreEHR\controllers_2\library\adodb\drivers\adodb-mysqli.inc.php on line 123

[15-Jun-2017 16:14:41 America/New_York] PHP custom error: from librehealth ehr library/sql.inc - Unable to set up UTF8 encoding with mysql database: Access denied for user 'libreehr'@'localhost' (using password: YES)

[15-Jun-2017 16:14:41 America/New_York] PHP Fatal error: Call to undefined function text() in C:\xampp\htdocs\LibreEHR\controllers_2\library\sql.inc on line 510`

This would have happened because of my database configurations in sqlconf.php. In order to test please replace this file with yours.

[tmccormi]

Art is a big fan of PDO, but I have no idea what the implications for using PDO are and by passing ADODB. Logging is important and it needs to be maintained one way or the other

[nileshprasad137]

The method I am currently using is safer I think so!

[tmccormi]

What do you mean by "safer" ? be specific.

[nileshprasad137]

I meant it would be less error prone as I have already tested it and it is working fine. But, I have not tested Documents tab functionality in depth.

[tmccormi]

Add/Change/Del operations all need to be logged via internal calls thru our mysql query (ADODB) functions. You can't bypass that. An argument could be made that "configuration" related changes like in the practice tools you've updated could be exempt from that, but it makes them outliers in the code base.

[aethelwulffe]

We already have sqlStatementNoLog($query) function that is used...and really not used enough in SELECT statements. I don't think that any UPDATE/INSERT/DELETE statements should be immune from logging (without intentional global configuration changes, which should then be logged) However:

         $conn =& NewADConnection('pdo');
         $conn->Connect('mysql:host=localhost',$user,$pwd,$mydb);
         $conn->Connect('mysql:host=localhost;dbname=mydb',$user,$pwd);
         $conn->Connect("mysql:host=localhost;dbname=mydb;username=$user;password=$pwd");

Or with DSN mechanism: $conn =& NewADConnection("pdo_mysql://user:pwd@localhost/mydb?persist"); # persist is optional Reference: http://phplens.com/lens/adodb/docs-adodb.htm

[nileshprasad137]

@teryhill @aethelwulffe Hope you've tested Administration/Practice. I found that templates in templates/patient_finder are not used anywhere at this moment.. Am i right.?

I don't think I would need to change any query right now using the approach i'm following at this moment.Above two comments apply if I make database related changes, right?

All I am doing now is replacing smarty based functions like fetch() , display() and constants defined using assign() with native PHP. Currently removing Smarty from vitals form.

[yehster]

Instead of switching to PDO, (which isn't really that different than ADODB), what would ultimately be quite valuable would be to implement and ORM layer using Eloquent/Laravel.

Eloquent has a "listener" event to which a hook for logging every query as we do with ADODB could be attached. So the logging issue should be relatively simple to address.

https://medium.com/headerlabs-india/logging-every-eloquent-queries-in-laravel-5-2-f812c6a4165e

[nileshprasad137]

Smarty from Vitals is being removed :) For reference :: LatestCommit on my repo

[nileshprasad137]

I have some confusions and questions . Please help me if you get some time.

1. Should I not remove Smarty from SOAP and ROS? Should I drop it completely?
2. What is the difference between vitals and vitalsM? Is vitalsM used anywhere in application.
3. Where are the files used in interface/clickmap being used in application ? Couldn't find it. Please help.
4. Templates inside templates/prescription and templates/patient finder are also not being used anywhere , right? Please correct me if I'm wrong.
5. When should I start working over UI? GACL is using Smarty and its quite big and would take more time. I think, after removing Smarty from other than this section, I would start to work over UI and would later come back to this again.. Suggestions?

[teryhill]

Yes remove smarty from ROS and SOAP. VitalsM appears to be a metric version. VitalsM can be loaded like any form so some one may be using it.

[nileshprasad137]

What about interface/clickmap..? Where is it used in application. I couldn't find so far.

[teryhill]

It is used in pain map and annotate diagram

[yehster]

VitalsM should just be removed. The standard vitals form can currently be configured to just show one or the other type of units, and handles conversion between them.

[aethelwulffe]

Anyone else particularly want to put in the commit on VitalsM? Nilesh, would you like to kill it, or is it OK for someone else to do it...?

[teryhill]

I say let @nileshprasad137 remove it as part of the overall effort.

[aethelwulffe]

Agreed.

[nileshprasad137]

I am done removing Smarty from almost everywhere except GACL, and a few other files. I would now like to jump over UI improvements. I would come back to remove Smarty from GACL after some time. I think it would be better to create a new thread for UI.

[tmccormi]

Is there a pull request for us to do code reviews and merge? I don't see one.

[nileshprasad137]

@tmccormi I had put the branch of the repository where I was developing earlier in this thread. https://github.com/nileshprasad137/FormsMVC-Application/tree/admin_prac

I thought that I'll send a PR after I am done removing smarty from GACL as well. But anyways, I can send a PR today itself. For now , I'll keep all the new code of Administration/Practice section and other Smarty pieces in a different folder, basically all the code you see in that branch of the repository above. So, that you all can test it by changing path of Admin/Prac from menu.

Will that be fine??

[aethelwulffe]

Just see if it is practical to do smaller commits...and if not, let's do piece-by-piece reviews on your branch and fork, integrating each piece as best we can into a final branch for merge into the main repo. We all know why we like to keep the pieces small when we can.

[nileshprasad137]

@aethelwulffe I completely agree with your point above , but if I send small pieces one-by-one, it will break the system in meanwhile. That is because , I have made changes in Controller class and calender and many other things depend on that.

So according to me, I'll keep my things in a different folder, I'll make a new branch over my repo, so that you can test it by changing path from menu.. After you are done reviewing that branch, we can merge that . You can have a look at my repository above.You may have a look over my commits to see what exactly I've been doing.

I'll update you soon on this.

[aethelwulffe]

Yeah, we understand this is a pretty darn integrated spaghetti-code thing, but doing a little as-we-go review well before we do integration (and yes, it will have to be one huge commit) should help ease the process.

[nileshprasad137]

https://github.com/nileshprasad137/LibreEHR/tree/smarty-removal

I thought calender was using Controller class but I was wrong. Everything is working fine(according to me). I think I can send a PR.

[nileshprasad137]

@teryhill Where in the application are Billing contact Controller and Patient Finder controllers are being used? Couldn't find.

I had no idea where Prescription controllers were being used until I noticed the error today.