Multiple improvements from IPPF related to layouts. #1081

sunsetsystems · 2017-09-10T23:52:23Z

A summary of what's new:

o Layout groups may be nested, i.e. each group can have sub-groups.
o Layouts and their groups have properties that may be edited in the layout editor.
o Encounter LBFs may specify sections for direct entry of services, products and diagnoses.
o "Case Management" feature including the ability to link encounter forms to issues,
and in the issue window a tab for each related encounter form.
o New layout edit option to specify initially-open groups.
o Implemented design-time ACO for LBF encounter forms.
o Improved printing of encounter LBFs.
o Feature to print a demographics profile with data filled in for a specified patient.
o Improvements to printed referrals.
o Provider attribute and selector for LBF encounter forms.
o New layout field type for a single checkbox.
o Support for demographics/history values in LBF static text fields.
o Auto-delete of shared attributes not used by any other form when a form is deleted.
o Assorted layout editor improvements.
o Added "Save and Continue" button to LBF visit forms.
o Added sorting to export of lists and layouts.
o Condition tests in LBF fields now support setting a value instead of just skipping the field.
o Added field condition support to demographics and history.
o Assorted bug fixes.

sunsetsystems · 2017-09-11T00:15:53Z

I see travis-ci reported a couple of syntax errors. I'll fix those along with whatever else comes up in the next day or two.

bradymiller · 2017-09-11T09:13:27Z

wow. that is a lot of code/features :)
Also lots of PSR2 fails in travis-ci. I'm glad to help out on this part when needed. Or if your up for it, can see here for commands:
https://gist.github.com/bradymiller/ae5069a058683b5a02c58b6afbcdace8

bradymiller · 2017-09-11T09:15:33Z

custom/demographics_print.php

+// Modified by Rod to be invoked from the custom/ directory by the normal
+// demographics_print.php script when MSI Rapid Workflow is activated.
+
+//The code below is copied from the original report, and fetches each data item in the demographic layout (if not 'unused')


See here for the new sleek headers we are using:
http://www.open-emr.org/wiki/index.php/How_to_Document_Your_Code_Properly#File-Level_DocBlock
(keep all the comments at the top of it)

bradymiller · 2017-09-11T09:15:56Z

custom/demographics_print.php

+
+//The code below is copied from the original report, and fetches each data item in the demographic layout (if not 'unused')
+
+// require_once("../interface/globals.php");


Isn't this a security issue?

Or are you using $encounter to control this (since I think there is no way to run this script unless it's included by another script that has the $encounter value set)?

Actually this entire file should be omitted. I just realized it covers an obsolete case and will never even be invoked.

bradymiller · 2017-09-11T09:17:08Z

custom/demographics_print.php

+require_once("$srcdir/formdata.inc.php");
+require_once("$srcdir/formatting.inc.php");
+
+if (empty($encounter)) die("Please select or create a visit!");


bradymiller · 2017-09-11T09:18:32Z

custom/demographics_print.php

+        $tmp = '';
+        if ($currvalue) {
+            $lrow = sqlQuery("SELECT title FROM list_options " .
+                "WHERE list_id = '$list_id' AND option_id = '$currvalue' AND activity = 1");


sql binding

bradymiller · 2017-09-11T09:19:45Z

custom/demographics_print.php

+        if ($currvalue) {
+            $lrow = sqlQuery("SELECT title FROM list_options " .
+                "WHERE list_id = '$list_id' AND option_id = '$currvalue' AND activity = 1");
+            $tmp = xl_list_label($lrow['title']);


bradymiller · 2017-09-11T09:20:24Z

custom/demographics_print.php

+            $lrow = sqlQuery("SELECT title FROM list_options " .
+                "WHERE list_id = '$list_id' AND option_id = '$currvalue' AND activity = 1");
+            $tmp = xl_list_label($lrow['title']);
+            if (empty($tmp)) $tmp = "($currvalue)";


either use text() or use $currescaped

bradymiller · 2017-09-11T09:20:41Z

custom/demographics_print.php

+    // long or multi-line text field
+    else if ($data_type == 3) {
+        $s .= "<textarea" .
+            " cols='$fld_length'" .


and would be good to put quote around this so it is cols='12' for example

bradymiller · 2017-09-11T09:20:51Z

custom/demographics_print.php

+    else if ($data_type == 3) {
+        $s .= "<textarea" .
+            " cols='$fld_length'" .
+            " rows='" . $frow['max_length'] . "'>" .


bradymiller · 2017-09-11T09:22:09Z

custom/demographics_print.php

+        $tmp = '';
+        if ($currvalue) {
+            $urow = sqlQuery("SELECT fname, lname, specialty FROM users " .
+                "WHERE id = '$currvalue'");


sql binding

bradymiller · 2017-09-11T09:23:15Z

custom/demographics_print.php

+        if ($currvalue) {
+            $urow = sqlQuery("SELECT fname, lname, specialty FROM users " .
+                "WHERE id = '$currvalue'");
+            $tmp = ucwords($urow['fname'] . " " . $urow['lname']);


best to not cap stuff since breaks with UTF8 encoding
should escpae this with text()

bradymiller · 2017-09-11T09:23:44Z

custom/demographics_print.php

+            $urow = sqlQuery("SELECT fname, lname, specialty FROM users " .
+                "WHERE id = '$currvalue'");
+            $tmp = ucwords($urow['fname'] . " " . $urow['lname']);
+            if (empty($tmp)) $tmp = "($currvalue)";


either use text() or use $currescaped

bradymiller · 2017-09-11T09:24:14Z

custom/demographics_print.php

+                if ($currvalue == $key) {
+                    $tmp = $prow['name'] . ' ' . $prow['area_code'] . '-' .
+                        $prow['prefix'] . '-' . $prow['number'] . ' / ' .
+                        $prow['line1'] . ' / ' . $prow['city'];


escape all these with text()

bradymiller · 2017-09-11T09:24:29Z

custom/demographics_print.php

+                        $prow['line1'] . ' / ' . $prow['city'];
+                }
+            }
+            if (empty($tmp)) $tmp = "($currvalue)";


either use text() or use $currescaped

bradymiller · 2017-09-11T09:26:10Z

custom/demographics_print.php

+            }
+            if (empty($tmp)) $tmp = "($currvalue)";
+        }
+        if ($tmp === '') $tmp = '&nbsp;';


the rest of this script needs adequate sql binding and html escaping

bradymiller · 2017-09-11T23:22:44Z

interface/forms/LBF/new.php

+}
+
+function warehouse_changed(sel) {
+ if (!confirm('<?php echo xl('Do you really want to change Warehouse?'); ?>')) {


bradymiller · 2017-09-11T23:23:54Z

interface/forms/LBF/new.php

+      while ($irow = sqlFetchArray($ires)) {
+        $issueid = $irow['id'];
+        $issuedate = oeFormatShortDate(empty($irow['begdate']) ? $irow['date'] : $irow['begdate']);
+        echo " <option value='$issueid'";


attr() around $issueid

It's always numeric but I'll change these to avoid anyone having to think about it.

bradymiller · 2017-09-11T23:24:09Z

interface/forms/LBF/new.php

+        $issuedate = oeFormatShortDate(empty($irow['begdate']) ? $irow['date'] : $irow['begdate']);
+        echo " <option value='$issueid'";
+        if ($issueid == $form_issue_id) echo " selected";
+        echo ">$issuedate " . text($irow['title']) . "</option>\n";


text() around $issuedate

bradymiller · 2017-09-11T23:25:46Z

interface/forms/LBF/new.php

+        if ($subtitle) {
+            // There is a group subtitle so show it.
+            echo "<tr><td class='bold' style='color:#0000ff' colspan='$CPR'>" . text($subtitle) . "</td></tr>\n";
+            echo "<tr><td class='bold' style='height:4pt' colspan='$CPR'></td></tr>\n";


attr() around $CPR in 2 lines above

bradymiller · 2017-09-11T23:27:12Z

interface/forms/LBF/new.php

+              if ($count) echo " </tr>\n";
+              echo " <tr>\n";
+            }
+            echo "  <td width='$tdpct%'>";


attr() around $tdpct

bradymiller · 2017-09-11T23:31:20Z

interface/forms/LBF/new.php

+            list($codetype, $code) = explode(':', $codestring);
+            $crow = sqlQuery("SELECT code_text FROM codes WHERE " .
+              "code_type = ? AND code = ? AND active = 1 " .
+              "ORDER BY id LIMIT 1",


What kind of codes is this for. If possibly includes external type codes, then should be using function in custom/code_types.inc.php
(in this case, the lookup_code_descriptions() function)

OK. I assume this also works for native (internal) code types?

yep, it works for all code types (internal and external) :)

bradymiller · 2017-09-11T23:32:57Z

interface/forms/LBF/new.php

+          "   <input type='checkbox' name='form_fs_bill[$lino][del]' " .
+          "value='1'" . ($li['del'] ? " checked" : "") . " />\n";
+        foreach ($li['hidden'] as $hname => $hvalue) {
+            echo "   <input type='hidden' name='form_fs_bill[$lino][$hname]' value='" . attr($hvalue) . "' />\n";


attr() around $hname

bradymiller · 2017-09-11T23:34:25Z

interface/forms/LBF/new.php

+          "   <input type='checkbox' name='form_fs_prod[$lino][del]' " .
+          "value='1'" . ($li['del'] ? " checked" : "") . " />\n";
+        foreach ($li['hidden'] as $hname => $hvalue) {
+            echo "   <input type='hidden' name='form_fs_prod[$lino][$hname]' value='" . attr($hvalue) . "' />\n";


attr() around $hname

bradymiller · 2017-09-12T08:38:01Z

interface/forms/LBF/new.php

+                if ($count) echo " </tr>\n";
+                echo " <tr>\n";
+            }
+            echo "  <td width='$tdpct%'>";


wrap $tdpct with attr()

bradymiller · 2017-09-12T08:40:42Z

interface/forms/LBF/new.php

+            list($codetype, $code) = explode(':', $codestring);
+            $crow = sqlQuery("SELECT code_text FROM codes WHERE " .
+              "code_type = ? AND code = ? AND active = 1 " .
+              "ORDER BY id LIMIT 1",


What kind of codes is this for. If possibly includes external type codes, then should be using function in custom/code_types.inc.php
(in this case, the lookup_code_descriptions() function)

Thanks (as above).

bradymiller · 2017-09-12T08:42:05Z

interface/forms/LBF/new.php

+          "   <input type='checkbox' name='form_fs_bill[$lino][del]' " .
+          "value='1'" . ($li['del'] ? " checked" : "") . " />\n";
+        foreach ($li['hidden'] as $hname => $hvalue) {
+            echo "   <input type='hidden' name='form_fs_bill[$lino][$hname]' value='" . attr($hvalue) . "' />\n";


attr() around $hname

bradymiller · 2017-09-12T08:44:41Z

interface/forms/LBF/printable.php

+td {
+ font-family: Arial;
+ font-weight: normal;
+ font-size: <?php echo $FONTSIZE; ?>pt;


text() around $FONTSIZE

bradymiller · 2017-09-13T08:16:08Z

library/ajax/code_attributes_ajax.php

+$sanitize_all_escapes  = true;
+
+require_once("../../interface/globals.php");
+require_once("$srcdir/formdata.inc.php");


remove above line

bradymiller · 2017-09-13T08:16:33Z

library/ajax/code_attributes_ajax.php

+      "WHERE list_id = 'warehouse' AND activity = 1 ORDER BY seq, title");
+    $wh .= "<option value=''></option>";
+    while ($lrow = sqlFetchArray($lres)) {
+      $wh .= "<option value='" . $lrow['option_id'] . "'";


bradymiller · 2017-09-13T08:16:46Z

library/ajax/code_attributes_ajax.php

+        // Disable this warehouse option if not selected and has no inventory.
+        if (!$has_inventory) $wh .= " disabled";
+      }
+      $wh .= ">" . xl_list_label($lrow['title']) . "</option>";


bradymiller · 2017-09-13T08:17:46Z

library/ajax/code_attributes_ajax.php

+      "WHERE c.code_type = ? AND c.code = ? LIMIT 1",
+      array($pricelevel, $code_types[$codetype]['id'], $code));
+    $desc = $crow['code_text'];
+    $price = empty($crow['pr_price']) ? 0 : (0 + $crow['pr_price']);


shouldn't this use the standard code collector mechanism in custom/code_types.inc.php

Yep another one of these. A bit concerning is that external codes do not seem to support price levels.

bradymiller · 2017-09-13T08:20:52Z

library/options.inc.php

+                if ($count) echo "</tr>";
+                echo "<tr>";
+            }
+            echo "<td width='$tdpct%'>";


bradymiller · 2017-09-13T08:23:06Z

library/options.inc.php

+              "ct.ct_key = '$codetype' AND " .
+              "c.code_type = ct.ct_id AND " .
+              "c.code = '$code' AND c.active = 1 " .
+              "ORDER BY c.id LIMIT 1";


looks like another place where should be using custom/code_types.inc.php function here in order to support external codes

bradymiller · 2017-09-13T08:30:45Z

sql/5_0_0-to-5_0_1_upgrade.sql

+  grp_products    varchar(4095)  not null default '',
+  grp_diags       varchar(4095)  not null default '',
+  PRIMARY KEY (grp_form_id, grp_group_id)
+) ENGINE=MyISAM;


use InnoDB and not MyISAM

bradymiller · 2017-09-13T08:31:20Z

sql/database.sql

+  grp_products    varchar(4095)  not null default '',
+  grp_diags       varchar(4095)  not null default '',
+  PRIMARY KEY (grp_form_id, grp_group_id)
+) ENGINE=MyISAM;


bradymiller · 2017-09-13T08:32:56Z

sql/database.sql

+INSERT INTO `list_options` (`list_id`, `option_id`, `title`, `seq`, `is_default`) VALUES ('Sexual_Orientation','Les','Lesbian'      ,40,0);
+INSERT INTO `list_options` (`list_id`, `option_id`, `title`, `seq`, `is_default`) VALUES ('Sexual_Orientation','Msm','MSM'          ,50,0);
+INSERT INTO `list_options` (`list_id`, `option_id`, `title`, `seq`, `is_default`) VALUES ('Sexual_Orientation','Oth','Other'        ,60,0);
+INSERT INTO `list_options` (`list_id`, `option_id`, `title`, `seq`, `is_default`) VALUES ('Sexual_Orientation','NS' ,'Not Specified',70,0);


Is the Gender List and Sexual_Orientation List supposed to be here. It's not in the upgrade script?

Evidently not since not in any layouts.

bradymiller · 2017-09-13T08:34:01Z

review done

sunsetsystems · 2017-09-15T05:12:19Z

I have added a commit with changes for review comments and a few other things. PSR4 for new scripts is not addressed yet, would prefer to do that afterwards to make it easier to see what's just been changed.

bradymiller · 2017-09-15T06:38:21Z

@sunsetsystems ,
When you are ready to bring this into the codebase, just let me know, and I'll do the PSR2 commit before you bring it in (this is really easy for me to do since had to do this in much more complicated settings in the past).

sunsetsystems · 2017-09-15T15:41:05Z

@bradymiller that's fine but with the master being a moving target there are now some conflicts. Do you want me to rebase my branch locally and make a new PR?

bradymiller · 2017-09-15T16:48:07Z

hi @sunsetsystems ,
Rec doing a rebase and then pushing it to the same PR (git push --force origin lbf)

A summary of what's new: o Layout groups may be nested, i.e. each group can have sub-groups. o Layouts and their groups have properties that may be edited in the layout editor. o Encounter LBFs may specify sections for direct entry of services, products and diagnoses. o "Case Management" feature including the ability to link encounter forms to issues, and in the issue window a tab for each related encounter form. o New layout edit option to specify initially-open groups. o Implemented design-time ACO for LBF encounter forms. o Improved printing of encounter LBFs. o Feature to print a demographics profile with data filled in for a specified patient. o Improvements to printed referrals. o Provider attribute and selector for LBF encounter forms. o New layout field type for a single checkbox. o Support for demographics/history values in LBF static text fields. o Auto-delete of shared attributes not used by any other form when a form is deleted. o Assorted layout editor improvements. o Added "Save and Continue" button to LBF visit forms. o Added sorting to export of lists and layouts. o Condition tests in LBF fields now support setting a value instead of just skipping the field. o Added field condition support to demographics and history. o Assorted bug fixes.

sunsetsystems · 2017-09-15T18:39:54Z

@bradymiller OK this has been done.

sunsetsystems · 2017-09-15T18:41:48Z

BTW I had to do a lot of repetitive changes for the new themes. Can't we reduce a lot of the redundancy among the different style_*.css files?

bradymiller · 2017-09-15T18:59:29Z

regarding the theme overload, hopefully will get SCSS up and running soon (so would be a central upstream script for those themes to change in 1 place, that would then spit out all the themes). @robertdown may have more to say on this.

bradymiller · 2017-09-15T19:03:45Z

btw, the recent new themes and theme cleanup are basically the first step in a complete codebase responsive bootstrap revolution that @zbig01 is working on. Very exciting and should start bringing in those changes into the codebase very soon.

bradymiller · 2017-09-16T03:31:40Z

hi @sunsetsystems ,
PSR2 changes went well. Did a code scan and things went well (note that we have found no bugs yet from when did this to entire codebase, so it's a very stable process; of course, it's best in the future to keep code PSR2 compliant via the code editor to avoid needing to do this). Since I want to ensure @zbig01 has the theme changes in his next PR, I'm going to bring this into the codebase now.
thanks for yet another awesome expansion of the layouts!
-brady

bradymiller · 2017-09-16T03:40:53Z

sql/5_0_0-to-5_0_1_upgrade.sql

@@ -617,3 +617,47 @@ UPDATE `user_settings` SET `setting_value`='style_light.css' WHERE `setting_labe
 OR `setting_value`='style_purple.css'
 OR `setting_value`='style_radiant.css'
 OR `setting_value`='style_sky_blue.css');
+
+#IfNotColumnType facility country_code varchar(30)
+ALTER TABLE `facility` CHANGE `country_code` `country_code` varchar(30) NOT NULL default '';


(right after brought into codebase noted the following :) )
It doesn't look like this change is addressed in database.sql ?
-brady

I'll push a fix for it. Thanks!

sunsetsystems added the Enhancement label Sep 10, 2017

bradymiller added the Status: Needs Review label Sep 11, 2017

bradymiller reviewed Sep 11, 2017

View reviewed changes

bradymiller reviewed Sep 12, 2017

View reviewed changes

bradymiller reviewed Sep 13, 2017

View reviewed changes

library/options.inc.php Outdated

if ($count) echo "</tr>";

echo "<tr>";

}

echo "<td width='$tdpct%'>";

Copy link

Sponsor Member

bradymiller Sep 13, 2017

Choose a reason for hiding this comment

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

attr()

bradymiller reviewed Sep 13, 2017

View reviewed changes

bradymiller added the Status: Needs Work label Sep 13, 2017

sunsetsystems removed the Status: Needs Work label Sep 15, 2017

sunsetsystems added 2 commits September 15, 2017 11:00

Some fixes for prior commit.

27f6cc4

sunsetsystems force-pushed the lbf branch from a55d6c3 to 27f6cc4 Compare September 15, 2017 18:38

sunsetsystems and others added 2 commits September 15, 2017 12:48

Fix for syntax error in prior commit.

ab8eb49

psr2 changes for lbf work

7bf7c75

bradymiller merged commit db79047 into openemr:master Sep 16, 2017

bradymiller reviewed Sep 16, 2017

View reviewed changes

sunsetsystems deleted the lbf branch October 3, 2017 19:19

mdsupport mentioned this pull request Dec 3, 2017

group_name to group_id upgrade #1268

Closed


		//The code below is copied from the original report, and fetches each data item in the demographic layout (if not 'unused')

		// require_once("../interface/globals.php");

Multiple improvements from IPPF related to layouts. #1081

Multiple improvements from IPPF related to layouts. #1081

Conversation

sunsetsystems commented Sep 10, 2017

sunsetsystems commented Sep 11, 2017

bradymiller commented Sep 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradymiller commented Sep 13, 2017

sunsetsystems commented Sep 15, 2017

bradymiller commented Sep 15, 2017

sunsetsystems commented Sep 15, 2017

bradymiller commented Sep 15, 2017

sunsetsystems commented Sep 15, 2017

sunsetsystems commented Sep 15, 2017

bradymiller commented Sep 15, 2017

bradymiller commented Sep 15, 2017

bradymiller commented Sep 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment