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

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

Closed
nileshprasad137 opened this issue May 25, 2017 · 75 comments
Assignees

Comments

@nileshprasad137
Copy link
Member

@nileshprasad137 nileshprasad137 commented May 25, 2017

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 ::

  • Administration/Practice Complete Section

  • interface/forms/vitals/

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

  • interface/forms/soap/

  • interface/forms/ros/

  • interface/forms/prior_auth/templates/prior_auth/

  • interface/clickmap/

  • interface/annotate_diagram/map_diagram/

  • GACL Section

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

  • 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 - #598 #614 #665

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented May 30, 2017

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

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 6, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 6, 2017

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

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 14, 2017

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

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 14, 2017

Nice.

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 15, 2017

Gimmie a minute to break this first :)

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jun 15, 2017

I will be working it into my system shortly.

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

yes

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jun 15, 2017

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

image

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

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 15, 2017

I only checked it from his branch...

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 15, 2017

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

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jun 15, 2017

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jun 16, 2017

Got it working Guy's

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jun 16, 2017

Never mind still using the old yunk

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 16, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 16, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 16, 2017

`[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

This comment has been minimized.

Copy link
Contributor

@tmccormi tmccormi commented Jun 16, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 16, 2017

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

@tmccormi

This comment has been minimized.

Copy link
Contributor

@tmccormi tmccormi commented Jun 16, 2017

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

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jun 27, 2017

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

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jul 8, 2017

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

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Jul 8, 2017

I don't see them being used any where

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jul 8, 2017

They are not used. The billing bit was in an old billing system, and the patient finder I believe was used in the old patient search.

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Jul 8, 2017

Hey, rip them out, then see if anything breaks!

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jul 10, 2017

Yes, nothing bad happens on ripping them. So only GACL left. 😌 👍

@azwreith

This comment has been minimized.

Copy link
Member

@azwreith azwreith commented Jul 14, 2017

Hi! Can someone confirm if smarty is being used in Patient Summary, i.e, summary/demographics.php.
I didn't give write permissions to web user to write compiled templates in pntemplates and it ends up in a smarty error and shows an endless loading cricle in the stats_div which is trying to load stats.php 😕

EDIT: The exact error for reference:

[Fri Jul 14 00:51:44.387005 2017] [:error] [pid 14975] [client 127.0.0.1:38160] PHP Fatal error:  Smarty error: unable to write to $compile_dir '/home/azwreith/Documents/GitHub/LibreEHR/interface/main/calendar/modules/PostCalendar/pntemplates/compiled'. Be sure $compile_dir is writable by the web server user. in /home/azwreith/Documents/GitHub/LibreEHR/library/Smarty/Smarty.class.php on line 1094, referer: http://localhost/LibreEHR/interface/patient_file/summary/demographics.php?set_pid=1
@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jul 14, 2017

@azwreith No,there aren't any Smarty functions being used there but.. I saw that in function image_widget($doc_id,$doc_catg) inside demographics.php , a call to Controller.class.php is made .
Controller.class.php was extending Smarty class before but later I made it independent in PR #598 . However, I don't know exactly why is that error coming up.. 😞

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Jul 14, 2017

@azwreith I think I got an idea why that error comes up :: (Refer #598 in Controller.class.php)
err

@azwreith

This comment has been minimized.

Copy link
Member

@azwreith azwreith commented Jul 14, 2017

Oh so it was a call to the controller.class! Thanks for the help!

Don't worry about the error, it was because I haven't given write permissions to Apache for the that directory you highlighted folder. I'll give it the permission for now and this whole smarty nonsense will go away when your PR is merged ^^"

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 3, 2017

GACL should be tackled or should be left for now?

@tmccormi

This comment has been minimized.

Copy link
Contributor

@tmccormi tmccormi commented Aug 4, 2017

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 4, 2017

Great then... 😄 There are still HTML templates residing there which are of no use now. If everything seems to be working fine, that can be deleted.

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 13, 2017

Should I delete the html templates as they aren't used now or should it be left there for reference which can anyway be seen through git history.?

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Aug 13, 2017

DELETE!!! DELEEEET! 🙂

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 13, 2017

Sure 👍

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 18, 2017

I also noticed some smarty references (calendar and non-calendar ones scattered throughout the repo)

@azwreith Can you tell me where so that I can get them removed? (Note - I have not worked on GACL.)

@azwreith

This comment has been minimized.

Copy link
Member

@azwreith azwreith commented Aug 20, 2017

@nileshprasad137 I just searched repo for smarty and a whole lot of references popped up. Help->About about.php uses SMARTY, for instance. Not sure if it's being used anywhere though!

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 21, 2017

@azwreith Yep, about.php uses it. I had to remove smarty connection from GACL if about.php had to not use smarty. It was a huge piece , so we decided to leave it for now.
Everything else seems to be safe !

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Aug 21, 2017

What are you using to indicate that about.php is using a smarty template?

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Aug 21, 2017

@teryhill Note that on line 108 on about.php, there is a smarty function display() which is used to render the smarty template (about.tpl) .

@teryhill

This comment has been minimized.

Copy link
Contributor

@teryhill teryhill commented Aug 21, 2017

Okay. I was just curious as to what the current criteria is.

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Aug 23, 2017

Looks like the criteria is "stuff that looks at templates". Good method. 😸

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Oct 19, 2017

Is this issue wrapped up yet?

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Feb 4, 2018

\interface\reports\index.php still has smarty class call.

@nileshprasad137

This comment has been minimized.

Copy link
Member Author

@nileshprasad137 nileshprasad137 commented Feb 4, 2018

Hi @aethelwulffe

Ya, I saw its having a Smarty Class call. However, I am not sure if it is being used anywhere.
The file \interface\reports\index.php is being called from interface/main/main_navigation.php. But perhaps main_navigation.php itself is not being used ..(I checked if anything breaks after deleting main_navigation).

I may be wrong. I'm looking at the repo after a long time.

Other than that, GACL has a call to Smarty class. Not by anything other than that perhaps.

@aethelwulffe

This comment has been minimized.

Copy link
Contributor

@aethelwulffe aethelwulffe commented Feb 5, 2018

@nileshprasad137 Maybe the best thing to pursue is to move GACL to /assets along with Smarty and have it ready for a cleaner, tighter 1:1 replacement or whatever libraries may be used by laravel. All I know is that for the few things we ask GACL to do, we should have a very simple and clean UI for it. As is, it is mostly an inexplicable mess. Between GACL, TAGS, user settings, authorizations, esign and a number of other things, I believe we actually have 6 different systems that handle access control. That is...too many.

I will start an issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.