Aggregate validation errors + Removal of pasted code

jeffersoncasimir · Nov 15, 2024 · 731b355 · 731b355
1 parent 0fb265f
commit 731b355
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 79 deletions.
diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js
@@ -26,7 +26,6 @@ class InstrumentUploadForm extends Component {
     this.getInstrumentOptions = this.getInstrumentOptions.bind(this);
     this.dataFileSelected = this.dataFileSelected.bind(this);
     this.uploadInstrumentData = this.uploadInstrumentData.bind(this);
-
   }
 
   /**
@@ -131,10 +130,17 @@ class InstrumentUploadForm extends Component {
             });
           });
         } else {
+          let message = '<div>';
+          message += `<br/># Errors: ${data.message.length}<br/><br/>`;
+          data.message.forEach((error) => {
+            message += (error + '<br/>');
+          });
+          message += '</div>';
+
           swal.fire({
             title: 'No data uploaded',
-            type: 'warn',
-            text: data.message,
+            type: 'warning',
+            html: message,
           });
         }
       })
@@ -155,16 +161,17 @@ class InstrumentUploadForm extends Component {
   }
 
 
-
   /**
    * Renders the React component.
    *
-   * @return {JSX} - React markup for the component
+   * @return {JSX.Element} - React markup for the component
    */
   render() {
-    const uploadInstrumentDisabled = () => this.state.selectedInstrumentFile === null;
+    const uploadInstrumentDisabled
+      = () => this.state.selectedInstrumentFile === null;
     const instrumentSelected = this.state.selectedInstrument !== '';
-    const uploadDataDisabled = () => !instrumentSelected || this.state.selectedDataFile === null;
+    const uploadDataDisabled
+      = () => !instrumentSelected || this.state.selectedDataFile === null;
 
     return (
       <>
@@ -225,7 +232,10 @@ class InstrumentUploadForm extends Component {
                 <a
                   className="btn btn-default"
                   disabled={!instrumentSelected}
-                  href={`${this.props.action}?instrument=${this.state.selectedInstrument}`}
+                  href={
+                    `${this.props.action}?instrument` +
+                    `=${this.state.selectedInstrument}`
+                  }
                 >
                     Download Expected Template
                 </a>

diff --git a/modules/instrument_manager/php/instrument_manager.class.inc b/modules/instrument_manager/php/instrument_manager.class.inc
@@ -276,7 +276,6 @@ class Instrument_Manager extends \DataFrameworkMenu
         if (isset($request_body['instrument'])) {
             $instrument = $request_body['instrument'];
             if ($this->instrumentExists($instrument)) {
-
                 // Parse
                 $dataParser = new InstrumentDataParser(
                     $instrument,
@@ -293,9 +292,17 @@ class Instrument_Manager extends \DataFrameworkMenu
                         ]
                     );
 
+                    if (count($validData['errors']) > 0) {
+                        return new \LORIS\Http\Response\JSON\OK(
+                            [
+                                'message' => $validData['errors'],
+                            ]
+                        );
+                    }
+
                     $result = $dataParser->insertInstrumentData(
                         $this->loris,
-                        $validData
+                        $validData['data']
                     );
 
                     return new \LORIS\Http\Response\JSON\Created($result);

diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc
@@ -226,22 +226,27 @@ class InstrumentDataParser extends CSVParser
     public function validateData(
         array $instrumentData, array $injectedColumns
     ): array {
-        try {
-            foreach ($instrumentData as &$dataRow) {
+        $errors = [];
+        foreach ($instrumentData as &$dataRow) {
+            try {
                 // Validate Session
                 $sessionID            = $this->obtainSessionID(
                     $dataRow['PSCID'],
                     $dataRow['Visit_label']
                 );
                 $dataRow['SessionID'] = $sessionID;
+
+                // Inject columns
                 $dataRow = array_merge($dataRow, $injectedColumns);
+            } catch (\Exception $e) {
+                $errors[] = $e->getMessage();
             }
-            return $instrumentData;
-        } catch (\Exception $e) {
-            throw new RuntimeException(
-                "Invalid data: {$e->getMessage()}"
-            );
         }
+
+        return [
+            'data'   => $instrumentData,
+            'errors' => $errors,
+        ];
     }
 
 

diff --git a/php/libraries/Utility.class.inc b/php/libraries/Utility.class.inc
@@ -263,10 +263,11 @@ class Utility
         }
         $query  = "SELECT DISTINCT Visit_label FROM session s
             JOIN candidate c ON (s.CandID = c.Candid)
+            JOIN psc ON (s.CenterID = psc.CenterID)
             WHERE c.Active = 'Y'
             AND s.Active = 'Y'
             AND c.Entity_type != 'Scanner'
-            AND s.CenterID!= '1'
+            AND psc.CenterID!= '1'
             $ExtraProject_Criteria ORDER BY Visit_label";
         $result = $db->pselect($query, $qparams);
         // The result has several columns; we only want the visit labels.
@@ -580,18 +581,24 @@ class Utility
             most accurate Full_name value and should be replaced by
             getDDEInstrumentNamesList() of the NDB_BVL_Instrument class."
         );
-        $Factory = \NDB_Factory::singleton();
-        $DB      = $Factory->Database();
-        $ddeInstruments = $DB->pselect(
-            "SELECT DISTINCT test_battery.Test_name, Full_name
-            FROM test_battery
-            JOIN test_names ON test_battery.Test_name = test_names.Test_name
-            WHERE DoubleDataEntryEnabled = 'Y'",
+        $Factory       = \NDB_Factory::singleton();
+        $DB            = $Factory->Database();
+        $config        = $Factory->config();
+        $instruments_q = $DB->pselect(
+            "SELECT Test_name,Full_name FROM test_names",
             []
         );
-        $instruments    = [];
-        foreach ($ddeInstruments as $instrument) {
-            $instruments[$instrument['Test_name']] = $instrument['Full_name'];
+        $doubleDataEntryInstruments = $config->getSetting(
+            'DoubleDataEntryInstruments'
+        );
+
+        $instruments = [];
+        foreach ($instruments_q as $row) {
+            if (isset($row['Test_name']) && isset($row['Full_name'])) {
+                if (in_array($row['Test_name'], $doubleDataEntryInstruments)) {
+                    $instruments[$row['Test_name']] = $row['Full_name'];
+                }
+            }
         }
         return $instruments;
     }
@@ -774,10 +781,11 @@ class Utility
             "SELECT DISTINCT t.Test_name, t.Full_name
             FROM session s
             JOIN candidate c ON (c.CandID=s.CandID)
+            JOIN psc ON (s.CenterID=psc.CenterID)
             JOIN flag f ON (f.sessionID=s.id)
             JOIN test_names t ON (f.test_name=t.Test_name)
             WHERE c.Active='Y' AND s.Active='Y' AND s.Visit_label =:vl
-            AND s.CenterID != '1' AND c.Entity_type != 'Scanner'
+            AND psc.CenterID != '1' AND c.Entity_type != 'Scanner'
             ORDER BY t.Full_name",
             ['vl' => $visit_label],
             'Test_name'

diff --git a/test/unittests/CSVParserTest.php b/test/unittests/CSVParserTest.php
@@ -25,30 +25,6 @@
  */
 class CSVParserTest extends TestCase
 {
-    protected const VALID_PASSWORD = 'correct horse battery staple';
-
-    /**
-     * Test double for NDB_Config object
-     *
-     * @var \NDB_Config | PHPUnit\Framework\MockObject\MockObject
-     */
-    private $_configMock;
-
-    /**
-     * Test double for Database object
-     *
-     * @var \Database | PHPUnit\Framework\MockObject\MockObject
-     */
-    private $_dbMock;
-
-    /**
-     * NDB_Factory used in tests.
-     * Test doubles are injected to the factory object.
-     *
-     * @var NDB_Factory
-     */
-    private $_factory;
-
     /**
      * Setup
      *
@@ -57,18 +33,6 @@ class CSVParserTest extends TestCase
     protected function setUp(): void
     {
         parent::setUp();
-
-        $configMock = $this->getMockBuilder('NDB_Config')->getMock();
-        $dbMock     = $this->getMockBuilder('Database')->getMock();
-        '@phan-var \NDB_Config $configMock';
-        '@phan-var \Database $dbMock';
-
-        $this->_configMock = $configMock;
-        $this->_dbMock     = $dbMock;
-
-        $this->_factory = NDB_Factory::singleton();
-        $this->_factory->setConfig($this->_configMock);
-        $this->_factory->setDatabase($this->_dbMock);
     }
 
     /**
@@ -80,7 +44,6 @@ protected function setUp(): void
     protected function tearDown(): void
     {
         parent::tearDown();
-        $this->_factory->reset();
     }
 
     /**
@@ -152,7 +115,7 @@ public function invalidCSVFiles(): array
         file_put_contents($invalidExtensionFilePath, '');
         $filePaths[] = [$invalidExtensionFilePath];
 
-        $incongruentData         = <<<CSV
+        $incongruentData = <<<CSV
 header1,header2,header3
 value1,value2
 value4,value5,value6
@@ -163,7 +126,7 @@ public function invalidCSVFiles(): array
         file_put_contents($incongruentDataFilePath, $incongruentData);
         $filePaths[] = [$incongruentDataFilePath];
 
-        $unexpectedHeaders         = <<<CSV
+        $unexpectedHeaders = <<<CSV
 header5,header6,header7
 value1,value2, value3
 value4,value5,value6
@@ -174,17 +137,6 @@ public function invalidCSVFiles(): array
         file_put_contents($unexpectedHeadersFilePath, $unexpectedHeaders);
         $filePaths[] = [$unexpectedHeadersFilePath];
 
-
-//        $unpairedEnclosure       = <<<CSV
-//header1,header2,header3
-//value1,value2,value3
-//value4,value5,value6
-//CSV;
-//        $unpairedEnclosureFilePath
-//            = tempnam(sys_get_temp_dir(), '__') . '.csv';
-//        file_put_contents($unpairedEnclosureFilePath, $unpairedEnclosure);
-//        $filePaths[] = [$unpairedEnclosureFilePath];
-
         return $filePaths;
     }