rachoac wrote on Friday, January 29, 2010:
Hi folks, I’ve checked in quite a few new MI2 UI enhancements. I’ve attached patch files here for easy review of what’s changed:
https://sourceforge.net/tracker/?func=detail&aid=2942035&group_id=60081&atid=1245239
Feedback/review welcome!
bradymiller wrote on Friday, January 29, 2010:
hey,
put a code review in the tracker (I put it there instead of in the forums, because there are no text formatting problems in the tracker)
-brady
rachoac wrote on Friday, January 29, 2010:
? The code review is in the tracker already. Whats in the form is the link to the tracker entry, not sure what you are referring to.
bradymiller wrote on Friday, January 29, 2010:
hey,
I placed a review of your code in the tracker artifact you linked to above (please note the unacceptable comment is only regarding the parametric stuff). Because of text formatting problems in the forums here at sourceforge, it’s easier for us to place our code reviews in the tracker for now.
-brady
rachoac wrote on Friday, January 29, 2010:
Ah ok got it. Thanks for the review. I will make a pass through all my open code reviews in the near future, incorporating suggestions.
bradymiller wrote on Friday, January 29, 2010:
hey,
Is there a reason why your globally removing htmlspecialchars($variable,
ENT_QUOTES) stuff? These are there because the values will get truncated if
they contain single or double quotes.
-brady
sunsetsystems wrote on Friday, January 29, 2010:
Aron, it appears you discarded my updates to templates/x12_partners/general_edit.html which is part of the X12 Partners form expansion that I did for Tony in November. And that’s just the first one I checked. What other changes have been zapped? Are you even using a CVS working directory?
Please review your list of changed files very carefully for conflicts. Then please spend some quality time with the CVS manual: http://ximbiot.com/cvs/manual/
tmccormi wrote on Friday, January 29, 2010:
I’ll hold off on pulling the tip over to SVN branch until issue Rod pointed out is resolved.
bradymiller wrote on Friday, January 29, 2010:
hey,
Probably good to hold off on any SF cvs commits until issue is resolved (ie. if there’s been extensive wiping out of previous developers code, possibly need to consider reverting back the code).
-brady
tmccormi wrote on Friday, January 29, 2010:
Aron say’s he’ll fix it this evening, don’t revert just yet, please
-Tony
bradymiller wrote on Friday, January 29, 2010:
In his patched files, on a quick random sampling, it appears some recent mods got incorporated and some didn’t. Since it’s so inconsistent, please ensure he checks every file for previous changes in cvs, and then fix appropriately.
thanks,
brady
rachoac wrote on Saturday, January 30, 2010:
Of course I am using CVS. I’ve used it for years.
The accidental overwrites are just that - accidental. Who here has never checked in something they didn’t regret, either immediately, or years down the line?
In any case, mea culpa, I will try to be more accurate in the future.
rachoac wrote on Saturday, January 30, 2010:
Brady:
“In his patched files, on a quick random sampling, it appears some recent mods got incorporated and some didn’t. Since it’s so inconsistent, please ensure he checks every file for previous changes in cvs, and then fix appropriately.”
Please refer to me in the first person. I am responsible for my own actions, not Tony.
Thanks,
Aron
rachoac wrote on Saturday, January 30, 2010:
These are all the files across all the patch files which were changed. I am going through them one by one now to find out which changes were stepped on. Report shortly.
– openemr/controllers/C_Document.class.php (revision 1.9)
– openemr/controllers/C_PracticeSettings.class.php (revision 1.1.1.1)
– openemr/interface/forms/newpatient/common.php (revision 1.7)
– openemr/interface/forms/newpatient/report.php (revision 1.2)
– openemr/interface/patient_file/encounter/forms.php (revision 1.25)
– openemr/interface/patient_file/history/history_full.php (revision 1.19)
– openemr/interface/patient_file/history/history.php (revision 1.13)
– openemr/interface/patient_file/summary/add_edit_issue.php (revision 1.25)
– openemr/interface/patient_file/summary/demographics.php (revision 1.61)
– openemr/interface/patient_file/summary/pnotes_fragment.php (revision 1.1)
– openemr/interface/patient_file/summary/pnotes_full_add.php Thu Jan 28 22:53:58 PST 2010
– openemr/interface/patient_file/summary/pnotes_full.php (revision 1.26)
– openemr/interface/patient_file/transaction/add_transaction.php (revision 1.17)
– openemr/interface/patient_file/transaction/transactions_full.php (revision 1.5)
– openemr/interface/patient_file/transaction/transactions.php (revision 1.13)
– openemr/interface/themes/style_sky_blue.css (revision 1.22)
– openemr/interface/usergroup/facilities_add.php Thu Jan 28 23:56:43 PST 2010
– openemr/interface/usergroup/facilities.php (revision 1.1)
– openemr/interface/usergroup/facility_admin.php (revision 1.15)
– openemr/interface/usergroup/user_admin.php (revision 1.28)
– openemr/interface/usergroup/usergroup_admin_add.php Fri Jan 29 00:34:23 PST 2010
– openemr/interface/usergroup/usergroup_admin.php (revision 1.31)
– openemr/templates/document_categories/general_list.html (revision 1.4)
– openemr/templates/documents/general_list.html (revision 1.4)
– openemr/templates/documents/general_queue.html (revision 1.3)
– openemr/templates/documents/general_upload.html (revision 1.4)
– openemr/templates/documents/general_view.html (revision 1.10)
– openemr/templates/hl7/general_parse.html (revision 1.4)
– openemr/templates/insurance_companies/general_edit.html (revision 1.4)
– openemr/templates/insurance_companies/general_list.html (revision 1.5)
– openemr/templates/insurance_numbers/general_edit.html (revision 1.4)
– openemr/templates/insurance_numbers/general_list.html (revision 1.3)
– openemr/templates/pharmacies/general_edit.html (revision 1.3)
– openemr/templates/pharmacies/general_list.html (revision 1.4)
– openemr/templates/practice_settings/general_list.html (revision 1.5)
– openemr/templates/x12_partners/general_edit.html (revision 1.4)
– openemr/templates/x12_partners/general_list.html (revision 1.3)
rachoac wrote on Saturday, January 30, 2010:
Alright. I went through each file, and it seems that I missed a grand total of *two* merges. Out of 37 changed files.
interface/usergroup/facility_admin.php
templates/x12_parnters/general_edit.html
If I am correct, then I only made a 6% error. In that case everybody can stop freaking out. Last time I checked an A- was a passing grade.
rachoac wrote on Saturday, January 30, 2010:
Attached repairs.php to the cod review.
bradymiller wrote on Saturday, January 30, 2010:
Please let him know that his recent gui refactoring work looks awesome in the demo.
-brady
sunsetsystems wrote on Saturday, January 30, 2010:
Aron, go ahead and commit your fixes. Then we can better evaluate and comment.
One thing I noticed, in the patient documents area the “Show Notes” button doesn’t work any more. It gives a JavaScript error, but also I wonder what your plan was for it, since it used to open patient notes in the “other frame”, which will be going away.
sunsetsystems wrote on Saturday, January 30, 2010:
Looking better. :-) Curiously, those 2 files you missed were among the first few I checked….
OK, some random questions and observations:
1. How come daemon_frame.php is refreshing every 5 seconds, and what is doing that? That seems excessive, it used to be 2 minutes. I’m not sure if this is your doing.
2. Is there a way to make the modal dialogs movable and resizable?
3. From the main patient page if I select Rx, Report, Documents or Transactions I get the nice cool-looking modal window, but since I can only have one of them and it does not allow any action outside of its area, it only serves to reduce the space available for doing things. Maybe we need to re-think some of this.
4. If I click History it replaces the whole frame, but the other options listed in (3) go away, and the summary info on the right is also gone. I have to click “Back to Patient” before they are available. Also in History the “To Issues” button has gone away.
5. In the Users page, if I start to add or edit a user and then click Cancel, nothing happens. Same thing with facilities.
More later. Weekend tasks are beckoning.
Rod
sunsetsystems wrote on Sunday, January 31, 2010:
Never mind the daemon_frame.php problem, I checked in a fix for it.