I, being generally clueless, of course see nothing wrong with those mods.
I may be becoming less of a nuisance as I am gradually shifting away from being “all-questions” to where I now have some suggestions of my own. I have yet to dive into all the github/submission stuff, but do have a couple very minor mods of my own I might add to the list….
1. If your layout style global for the left_nav menu is set to treeview, when expanding the main “Reports” branch the “Visits” sub-branch will be expanded by default, yet the button to toggle expand/collapse displays the wrong icon. The image for the “Visits” button remains out-of-synch with the actual state of the branch, always showing “+” when expanded, and “-” when collapsed. My personal preference would be to leave all the branches beneath “Reports” defaulting to collapsed by changing line 1276 of left_nav.php to:
<li><a class=“collapsed_lv2”><span><?php xl(‘Visits’,‘e’) ?></span></a>
2. Could we add “$cfg = TRUE;” to the bottom of the config.inc.php file(s)?
It adds a simple link to the right side of the PhpMyAdmin home page allowing a one-click launch of the useful phpinfo() report utility.
Does a “git_for_really_dumb_dummies” document exist?
Sheesh, that seems like a lot of hoops to jump through.
Maybe if I just dive in, it won’t be as bad as it seems.
I already have a modified getproviderinfo() I’d like to submit.
I had a vague “ajax error” message pop up the other day and researching it got me off on a tangent where I compiled a list of necessary file deletions/changes to begin to sort out the jquery (AJAX library) version mess. There are currently 11 files spanning 9 jquery versions in the OpenEMR installation. There are currently 8 files referencing jquery121.js and 28 referencing jquery-1.2.2.js. Those 36 references could be changed to jquery-1.2.6.js, and the “minified” 1.2.6 version of jquery added to the \library\js folder. The only differences between 1.2.0 and 1.2.6 are bug fixes and performance enhancements, there are no “script-breaking” changes. The \library\js files jquery.js (an unreferenced copy of 1.2.1), jquery121.js, and jquery-1.2.2.js could then be deleted. Version 1.2.6 would then become the lowest-common-denominator, and the next step would be to move the floor up to version 1.3.x (currently referenced by 69 files). It requires additional work though, as the code needs to be reviewed because 1.3 contains “breaking” changes. Then up to version 1.4.x (11 references, including 7 to the 1.4.3 file named jquery.js found in \library\js\jquery.treeview-1.4.1\demo\. Then 1.5, 1.6, 1.7… checking for script-breaking changes the whole way. Ver 1.7.2 is the highest used in OpenEMR 4.1.1.9, although jquery 1.9.1 is the latest release. Redundant code or files, multiple versions, all that stuff that drives me nuts. So, I guess if I’m going to contribute, I better dig into git. I have to say though, at first glance it looks ugly. Reminds me of the stuff that ran on the Unix boxes they hung off our System 370 back in the 1980’s. (but then, sourceforge, compared to other forum software, has a bit of the same “feel”)
Just dive in with the git for dummies instructions and follow the instructions verbatim in order to get the repo going. The learning curve is relatively steep initially, but we’ve all been there and are happy to provide guidance (so feel free to post any git related questions here). If you are using Windows, then while learning suggest using the command line git interface (rather than a gui) that comes with git and then just follow along on the git for dummies instructions. Once you get the hang of git, it’s actually very intuitive and very powerful. Regarding the jquery, would be nice to get it organized better and remove redundant script. I think Julia has also posted some analysis on this on the wiki here: http://open-emr.org/wiki/index.php/Jquery
I’ve recently completed a contract adding several customizations to OpenEMR, some of which i think are improvements to the interface, and many of them touch on the jquery / gritter / fancybox version issues in the tree. To be more specific:
I’ve got a version of OpenEMR with no popups of any kind (instead using the fancybox code in the tree), and with popup notifications (hovering over the main interface, via gritter) for when someone has an active note, or expiring reminder.
I’ve also got about a hundred different minor improvements(not kidding), having touched almost every file in the concurrent interface.
I developed it all in one branch, so am now separating it (slowly, learning to use magit) into multiple branches for submission. I am placing all of the commits that i believe should be merged with mainline without effecting any features in the ‘staging’ branch, of git@gitorious.org:~juri/openemr/juris-openemr.git .
Brady, how do you want to handle several hundred small commits?
I’ll likely just merge in your commits from your staging branch (or whatever branch you have the staging commits in that you want committed). Then since keeping your commits intact, hopefully will be easier for you to keep track and organize things. Can I commit the stuff in your above posted staging branch now to sourceforge?
For the future branches, try to just keep them organized (such as the no-brainer commits lumped together, while the more controversial commits should be separate) and flat (ie. avoid branches with merging etc.), which is what you have done nicely with your above posted branch.
merge away! my current workflow is to tear the ‘master’ there into seperate branches, which should be mergable into one tree, to match up with the master. then i can throw the commits in ‘master’ away.
more to merge. please note that one of these patches does make it to where users who do not have ‘patient/notes’ access are not allowed to see the ‘messages and reminders’ page, which i consider to be a bug fix.
But the messages and reminders could potentially be non-patient related (can set a reminder to oneself etc. and at some point messages will likely be non-patient related; the goal of this screen long-term is to centralize all the messages/tasks/items/authorizations for a user here). As long as the user is only seeing their message (which they are forced; I think only admin’s can See All), then there seems to be no reason to control this with the patient/notes acl.
Quick example. If the physician sends a message to the accountant for billing matters, no reason to deny the accountant access to this. But the accountant should not be able to see all the patient/notes of that patient. So, the patient/notes acl should protect the viewing of all patient notes for a patient. Hope this clarifies things a bit.
While this reasoning makes sense, at present this interface allows anyone to send a message to anyone else regarding any patient (and requiring one to be selected). These messages will end up in the selected patient’s file.
I can understand the desire for a site wide messaging system, but at present, this is not yet there.
Hi,
But the Messages are likely being used in a way by the userbase (see my example above) that your acl will break functionality for users. I am not saying it shouldn’t go in(I still have not even looked at the code), but this commit should get it’s own branch for review/testing/discussion in order to incorporate the practical use and future trajectory of that module (especially to avoid postponing the other straightforward commits).
-brady
Makes sense to me, as your use case is likely happening in production. I’ll pull it from that branch, and let you know when the branch is ‘boring’ enough to just commit.
OK, my staging branch is now boring again, and contains many boring commits, that should be merged.
I forsee really getting to know git well, as i break up some 500 commits (which really do a few thousand things…) for submission. I’ve started on the ‘hoverover’ branch, I’ll let you know when its comitable.
Merged your staging branch in and added a quick fix after them for your commit fff1980dd0a7cfd88dfbf2d8f1acb1f4b53a929a:
http://github.com/openemr/openemr/commit/8ac0bcda74f9478b698fe575c85206a9e4e4b01e
(Note the formData function removes magic quotes (if on) and preps a variable for database insert, which is not what is happening here. Here I instead used the strip_escape_custom() function to remove magic quotes (if on). I’d suggest avoid using the formData/formDataCore function and instead use strip/add_escape-custom() instead; note that none of this is needed in scripts that have been converted (or new scripts) that follow the new security model: http://open-emr.org/wiki/index.php/Codebase_Security#Plan
(however, the scripts need to be fully converted, so the strip/add_escape-custom() functions are still very useful in scripts not yet converted)
