From 897680aacad9027144b827e7d75be66444f40461 Mon Sep 17 00:00:00 2001
From: Rida Abou-Haidar <rida.abou-haidar@mcin.ca>
Date: Mon, 16 Dec 2024 11:22:00 -0500
Subject: [PATCH] fixed issue

---
 modules/biobank/jsx/customFields.js       | 72 ++++++++++++-----------
 modules/biobank/jsx/processForm.js        | 50 ++++++++--------
 modules/biobank/jsx/specimenForm.js       |  7 +++
 modules/biobank/php/specimendao.class.inc | 59 +++++++++++--------
 4 files changed, 104 insertions(+), 84 deletions(-)

diff --git a/modules/biobank/jsx/customFields.js b/modules/biobank/jsx/customFields.js
index 6fdaadbe743..0d538b91da9 100644
--- a/modules/biobank/jsx/customFields.js
+++ b/modules/biobank/jsx/customFields.js
@@ -1,5 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import {
+  TextboxElement,
+  DateElement,
+  TimeElement,
+  CheckboxElement,
+  FileElement
+} from 'jsx/Form';
 
 /**
  * Biobank Custom Attribute Fields
@@ -7,23 +14,22 @@ import PropTypes from 'prop-types';
  * @param {object} props The component's props.
  */
 function CustomFields(props) {
-  const {options, errors, fields, object} = props;
+  const {options, errors, attributes, object} = props;
 
-  return Object.keys(fields).map(
+  return attributes.map(
     (attribute, key) => {
-      const datatype = options.specimen.attributeDatatypes[
-        fields[attribute]['datatypeId']
-      ].datatype;
+      const datatype = options.specimen.attributeDatatypes[attribute.datatypeId]
+        .datatype;
       if (datatype === 'text' || datatype === 'number') {
         return (
           <TextboxElement
             key={key}
-            name={attribute}
-            label={fields[attribute].label}
+            name={attribute.id}
+            label={attribute.label}
             onUserInput={props.setData}
-            required={fields[attribute].required}
-            value={object[attribute]}
-            errorMessage={errors[attribute]}
+            required={attribute.required}
+            value={object[attribute.id]}
+            errorMessage={errors[attribute.id]}
           />
         );
       }
@@ -31,12 +37,12 @@ function CustomFields(props) {
         return (
           <DateElement
             key={key}
-            name={attribute}
-            label={fields[attribute].label}
+            name={attribute.id}
+            label={attribute.label}
             onUserInput={props.setData}
-            required={fields[attribute].required}
-            value={object[attribute]}
-            errorMessage={errors[attribute]}
+            required={attribute.required}
+            value={object[attribute.id]}
+            errorMessage={errors[attribute.id]}
           />
         );
       }
@@ -44,42 +50,40 @@ function CustomFields(props) {
         return (
           <TimeElement
             key={key}
-            name={attribute}
-            label={fields[attribute].label}
+            name={attribute.id}
+            label={attribute.label}
             onUserInput={props.setData}
-            required={fields[attribute].required}
-            value={object[attribute]}
-            errorMessage={errors[attribute]}
+            required={attribute.required}
+            value={object[attribute.id]}
+            errorMessage={errors[attribute.id]}
           />
         );
       }
       if (datatype === 'boolean') {
-        // TODO: delete the following line.
-        // object[attribute] == null && props.setData(attribute, false);
         return (
           <CheckboxElement
             key={key}
-            name={attribute}
-            label={fields[attribute].label}
+            name={attribute.id}
+            label={attribute.label}
             onUserInput={props.setData}
-            required={fields[attribute].required}
-            value={object[attribute]}
-            errorMessage={errors[attribute]}
+            required={attribute.required}
+            value={object[attribute.id]}
+            errorMessage={errors[attribute.id]}
           />
         );
       }
       // Do not present the possibility of uploading if file is already set
       // File must instead be deleted or overwritten.
-      if (datatype === 'file' && !(props.data||{})[attribute]) {
+      if (datatype === 'file' && !(props.data||{})[attribute.id]) {
         return (
           <FileElement
             key={key}
-            name={attribute}
-            label={fields[attribute].label}
+            name={attribute.id}
+            label={attribute.label}
             onUserInput={props.setData}
-            required={fields[attribute].required}
-            value={props.current.files[object[attribute]]}
-            errorMessage={errors[attribute]}
+            required={attribute.required}
+            value={props.current.files[object[attribute.id]]}
+            errorMessage={errors[attribute.id]}
           />
         );
       }
@@ -88,7 +92,7 @@ function CustomFields(props) {
 }
 
 CustomFields.propTypes = {
-  fields: PropTypes.object.isRequired,
+  attributes: PropTypes.array.isRequired,
   options: PropTypes.object.isRequired,
   object: PropTypes.object.isRequired,
   setData: PropTypes.func.isRequired,
diff --git a/modules/biobank/jsx/processForm.js b/modules/biobank/jsx/processForm.js
index f946ee4c149..dfd9aa65dd0 100644
--- a/modules/biobank/jsx/processForm.js
+++ b/modules/biobank/jsx/processForm.js
@@ -63,7 +63,6 @@ const SpecimenProcessForm = (props) => {
   );
 
   let specimenProtocols = {};
-  let specimenProtocolAttributes = {};
   Object.entries(options.specimen.protocols).forEach(
     ([id, protocol]) => {
       // FIXME: I really don't like 'toLowerCase()' function, but it's the
@@ -73,19 +72,17 @@ const SpecimenProcessForm = (props) => {
       if (typeId == protocol.typeId && process == processStage
       ) {
         specimenProtocols[id] = protocol.label;
-        const attribute = options.specimen.protocolAttributes[id];
-        specimenProtocolAttributes[id] = attribute;
       }
     }
   );
 
   const renderProtocolFields = () => {
-    if (specimenProtocolAttributes[process.protocolId]) {
+    if (options.specimen.protocolAttributes[process.protocolId]) {
       if (process.data) {
         return <CustomFields
           options={options}
           errors={errors.data || {}}
-          fields={specimenProtocolAttributes[process.protocolId]}
+          attributes={options.specimen.protocolAttributes[process.protocolId]}
           object={process.data}
           setData={setData} />;
       } else {
@@ -178,29 +175,28 @@ const SpecimenProcessForm = (props) => {
       updateButton,
     ];
   } else if (edit === false) {
-    const protocolStaticFields = process.data &&
-    Object.keys(process.data).map(
-      (key) => {
-        let value = process.data[key];
-        if (process.data[key] === true) {
-          value = 'Yes';
-        } else if (process.data[key] === false) {
-          value = 'No';
-        }
-        // FIXME: The label used to be produced in the following way:
-        // label={options.specimen.protocolAttributes[process.protocolId][key].label}
-        // However, causes issues when there is data in the data
-        // object, but the protocolId is not associated with any attributes.
-        // This is a configuration/importing issue that should be fixed.
-        return (
-          <StaticElement
-            key={key}
-            label={options.specimen.attributes[key].label}
-            text={value}
-          />
-        );
+    const protocolAttributes = options.specimen.protocolAttributes[
+      process.protocolId
+    ] || [];
+    
+    const protocolStaticFields = protocolAttributes.map((attribute) => {
+      let value = process.data[attribute.id]; // Fetch the corresponding value from process.data
+
+      // Convert boolean values to "Yes" or "No"
+      if (value === true) {
+        value = 'Yes';
+      } else if (value === false) {
+        value = 'No';
       }
-    );
+
+      return (
+        <StaticElement
+          key={attribute.id}
+          label={attribute.label} // Use the attribute label from the ordered list
+          text={value || '—'} // Use an empty string if value is undefined
+        />
+      );
+    });
 
     const collectionStaticFields = (processStage === 'collection') && (
       <StaticElement
diff --git a/modules/biobank/jsx/specimenForm.js b/modules/biobank/jsx/specimenForm.js
index 1a4cb9b2f8b..2870460838e 100644
--- a/modules/biobank/jsx/specimenForm.js
+++ b/modules/biobank/jsx/specimenForm.js
@@ -551,6 +551,13 @@ class SpecimenBarcodeForm extends React.Component {
     this.setSpecimen = this.setSpecimen.bind(this);
   }
 
+  componentDidUpdate(prevProps, prevState) {
+    // Check if typeId has changed
+    if (prevProps.item.typeId !== this.props.item.typeId) {
+      this.props.setListItem('collection', {}, this.props.itemKey);
+    }
+  }
+
   /**
    * Set the current container.
    *
diff --git a/modules/biobank/php/specimendao.class.inc b/modules/biobank/php/specimendao.class.inc
index fef91081199..5be663f32ec 100644
--- a/modules/biobank/php/specimendao.class.inc
+++ b/modules/biobank/php/specimendao.class.inc
@@ -393,33 +393,46 @@ class SpecimenDAO extends \LORIS\Data\ProvisionerInstance
      */
     public function getProtocolAttributes() : array
     {
-        $query  = "SELECT bsp.SpecimenProtocolID as protocolId,
-                         bsa.SpecimenAttributeID as attributeId,
-                         bsa.Label as label,
-                         bsa.DatatypeID as datatypeId,
-                         bspa.Required as required,
-                         bspa.ShowInDataTable as showInDataTable
-                  FROM biobank_specimen_protocol_attribute_rel bspa
-                  LEFT JOIN biobank_specimen_protocol bsp
-                    USING (SpecimenProtocolID)
-                  LEFT JOIN biobank_specimen_attribute bsa
-                    USING (SpecimenAttributeID)";
+        $query = "
+            SELECT 
+                bsp.SpecimenProtocolID as protocolId,
+                bsa.SpecimenAttributeID as attributeId,
+                bsa.Label as label,
+                bsa.DatatypeID as datatypeId,
+                bspa.Required as required,
+                bspa.ShowInDataTable as showInDataTable,
+                bspa.OrderIndex as orderIndex
+            FROM biobank_specimen_protocol_attribute_rel bspa
+            LEFT JOIN biobank_specimen_protocol bsp
+                USING (SpecimenProtocolID)
+            LEFT JOIN biobank_specimen_attribute bsa
+                USING (SpecimenAttributeID)
+            ORDER BY bsp.SpecimenProtocolID, bspa.OrderIndex
+        ";
+    
         $result = $this->db->pselect($query, []);
-        $pA     = []; //protocolAttributes
+        $protocolAttributes = [];
+    
         foreach ($result as $row) {
-            if (!isset($pA[$row['protocolId']][$row['attributeId']])) {
-                $pA[$row['protocolId']][$row['attributeId']] = [];
+            $protocolId = $row['protocolId'];
+            // Ensure the protocol exists in the array
+            if (!isset($protocolAttributes[$protocolId])) {
+                $protocolAttributes[$protocolId] = [];
             }
-            $attrib =& $pA[$row['protocolId']][$row['attributeId']];
-
-            $attrib['label']           = $row['label'];
-            $attrib['datatypeId']      = $row['datatypeId'];
-            $attrib['required']        = (int) $row['required'];
-            $attrib['showInDataTable'] = (int) $row['showInDataTable'];
+    
+            // Add the attribute data, keeping the order intact
+            $protocolAttributes[$protocolId][] = [
+                'id'               => $row['attributeId'],
+                'label'            => $row['label'],
+                'datatypeId'       => $row['datatypeId'],
+                'required'         => (int) $row['required'],
+                'showInDataTable'  => (int) $row['showInDataTable'],
+                'orderIndex'       => (int) $row['orderIndex'],
+            ];
         }
-
-        return $pA;
-    }
+    
+        return $protocolAttributes;
+    }    
 
     /**
      * Queries all rows from the biobank_specimen_protocol_attribute_rel table