Merge remote-tracking branch 'aces/24.1-release' into CBIG

ridz1208 · Sep 8, 2023 · 3799a7d · 3799a7d
2 parents 8be2d34 + b381816
commit 3799a7d
Show file tree

Hide file tree

Showing 17 changed files with 132 additions and 45 deletions.
diff --git a/modules/api/php/endpoints/candidates.class.inc b/modules/api/php/endpoints/candidates.class.inc
@@ -150,6 +150,10 @@ class Candidates extends Endpoint implements \LORIS\Middleware\ETagCalculator
 
         $candidate = \NDB_Factory::singleton()->candidate($candID);
 
+        if (!$candidate->isAccessibleBy($user)) {
+            return new \LORIS\Http\Response\JSON\Forbidden();
+        }
+
         $endpoint = new Candidate\Candidate($candidate);
 
         $pathparts = array_slice($pathparts, 2);
@@ -241,8 +245,14 @@ class Candidates extends Endpoint implements \LORIS\Middleware\ETagCalculator
                 $pscid,
                 $project->getId()
             );
-        } catch (\LorisException | \InvalidArgumentException $e) {
-            return new \LORIS\Http\Response\JSON\BadRequest($e->getMessage());
+        } catch (\ConflictException $e) {
+            return new \LORIS\Http\Response\JSON\Conflict(
+                $e->getMessage()
+            );
+        } catch (\Exception | \LorisException | \InvalidArgumentException $e) {
+            return new \LORIS\Http\Response\JSON\BadRequest(
+                $e->getMessage()
+            );
         }
 
         $candidate = \NDB_Factory::singleton()->candidate($candid);

diff --git a/modules/api/php/views/visit/flags.class.inc b/modules/api/php/views/visit/flags.class.inc
@@ -50,9 +50,9 @@ class Flags
     public function toArray(): array
     {
         $instrumentname = $this->_instrument->testName;
-        $instrumentdata = $this->_instrument->getInstanceData();
+        $commentid      = $this->_instrument->getCommentID() ?? '';
 
-        $isDDE = strpos($instrumentdata['CommentID'], 'DDE_') === 0;
+        $isDDE = strpos($commentid, 'DDE_') === 0;
 
         $meta = [
             'Candidate'  => $this->_timepoint->getCandID(),

diff --git a/modules/api/php/views/visit/instrument.class.inc b/modules/api/php/views/visit/instrument.class.inc
@@ -51,8 +51,9 @@ class Instrument
     {
         $instrumentname = $this->_instrument->testName;
         $instrumentdata = $this->_instrument->getInstanceData();
+        $commentid      = $this->_instrument->getCommentID() ?? '';
 
-        $isDDE = strpos($instrumentdata['CommentID'], 'DDE_') === 0;
+        $isDDE = strpos($commentid, 'DDE_') === 0;
 
         $meta = [
             'Candidate'  => $this->_timepoint->getCandID(),

diff --git a/modules/battery_manager/php/testoptionsendpoint.class.inc b/modules/battery_manager/php/testoptionsendpoint.class.inc
@@ -11,6 +11,7 @@
  * @link     https://www.github.com/aces/Loris/
  */
 namespace LORIS\battery_manager;
+use LORIS\VisitController;
 use \Psr\Http\Message\ServerRequestInterface;
 use \Psr\Http\Message\ResponseInterface;
 
@@ -58,11 +59,16 @@ class TestOptionsEndpoint extends \NDB_Page
      */
     private function _getOptions() : array
     {
+        $visitController = new VisitController(
+            $this->loris->getDatabaseConnection()
+        );
         return [
-            'instruments' => \Utility::getAllInstruments(),
+            'instruments' => \NDB_BVL_Instrument::getInstrumentNamesList(
+                $this->loris
+            ),
             'stages'      => $this->_getStageList(),
             'subprojects' => \Utility::getSubprojectList(null),
-            'visits'      => \Utility::getVisitList(),
+            'visits'      => $visitController->getVisitlabels(),
             'sites'       => \Utility::getSiteList(false),
             'firstVisit'  => $this->_getYesNoList(),
             'active'      => $this->_getYesNoList(),

diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc
@@ -447,11 +447,11 @@ class Edit extends \NDB_Page implements ETagCalculator
         $historyValues = $this->getChangedValues($issueValues, $issueID, $user);
 
         if (!empty($issueID)) {
-            $db->update('issues', $issueValues, ['issueID' => $issueID]);
+            $db->unsafeUpdate('issues', $issueValues, ['issueID' => $issueID]);
         } else {
             $issueValues['reporter']    = $user->getUsername();
             $issueValues['dateCreated'] = date('Y-m-d H:i:s');
-            $db->insert('issues', $issueValues);
+            $db->unsafeInsert('issues', $issueValues);
             $issueID = intval($db->getLastInsertId());
         }
 
@@ -815,7 +815,7 @@ class Edit extends \NDB_Page implements ETagCalculator
                     'issueID'      => $issueID,
                     'addedBy'      => $user->getUsername(),
                 ];
-                $db->insert('issues_history', $changedValues);
+                $db->unsafeInsert('issues_history', $changedValues);
             }
         }
     }
@@ -838,7 +838,7 @@ class Edit extends \NDB_Page implements ETagCalculator
                 'addedBy'      => $user->getUsername(),
                 'issueID'      => $issueID,
             ];
-            $db->insert('issues_comments', $commentValues);
+            $db->unsafeInsert('issues_comments', $commentValues);
         }
     }
 

diff --git a/modules/survey_accounts/js/survey_accounts_helper.js b/modules/survey_accounts/js/survey_accounts_helper.js
@@ -16,11 +16,13 @@ $(document).ready(function () {
     // Handles cases where there was an error on the page and we're resubmitting
     var email2 = $("input[name=Email2]").val();
     var email  = $("input[name=Email]").val();
-    if (email.length > 0 && email2.length > 0 && email == email2)
-    {
-            $('#email_survey').removeAttr('disabled');
-    } else {
-            $('#email_survey').attr('disabled','disabled');
+    if (email && email2) {
+      if (email.length > 0 && email2.length > 0 && email == email2)
+      {
+              $('#email_survey').removeAttr('disabled');
+      } else {
+              $('#email_survey').attr('disabled','disabled');
+      }
     }
     // Reset Test_name so that the template can be loaded by ajax below
     $("select[name=Test_name]").val("");
@@ -93,7 +95,7 @@ $(document).ready(function () {
             $("#emailContent").val(content);
         }
         );
-        
+
 
     });
 });
diff --git a/modules/survey_accounts/jsx/surveyAccountsIndex.js b/modules/survey_accounts/jsx/surveyAccountsIndex.js
@@ -112,7 +112,11 @@ class SurveyAccountsIndex extends Component {
         options: options.instruments,
       }},
       {label: 'URL', show: true},
-      {label: 'Status', show: true},
+      {label: 'Status', show: true, filter: {
+        name: 'Status',
+        type: 'select',
+        options: options.statusOptions,
+      }},
     ];
   const addSurvey = () => {
     location.href='/survey_accounts/addSurvey/';

diff --git a/modules/survey_accounts/php/addsurvey.class.inc b/modules/survey_accounts/php/addsurvey.class.inc
@@ -162,8 +162,9 @@ class AddSurvey extends \NDB_Form
                 ];
             }
         }
-
-        if ($_REQUEST['fire_away'] !== 'Create survey') {
+        if (!isset($_REQUEST['fire_away'])
+            || ($_REQUEST['fire_away'] !== 'Create survey')
+        ) {
             if (!filter_var(
                 $values['Email'],
                 FILTER_VALIDATE_EMAIL
@@ -241,11 +242,11 @@ class AddSurvey extends \NDB_Form
                     'CommentID'       => $commentID,
                 ]
             );
+            $this->tpl_data['success'] = true;
         } catch (\DatabaseException $e) {
             error_log($e->getMessage());
             $this->tpl_data['success'] = false;
         }
-        $this->tpl_data['success'] = true;
 
         if ($email && ($values['send_email'] == 'true')) {
             $config   = \NDB_Config::singleton();
@@ -291,7 +292,7 @@ class AddSurvey extends \NDB_Form
             "Instrument",
             array_merge(
                 ['' => ''],
-                \Utility::getDirectInstruments()
+                \NDB_BVL_Instrument::getDirectEntryInstrumentNamesList($this->loris)
             )
         );
         $this->addBasicText("Email", "Email address");

diff --git a/modules/survey_accounts/php/survey_accounts.class.inc b/modules/survey_accounts/php/survey_accounts.class.inc
@@ -74,14 +74,22 @@ class Survey_Accounts extends \DataFrameworkMenu
      */
     public function getFieldOptions() : array
     {
+        $statusOptions = [
+            'Created'     => 'Created',
+            'Sent'        => 'Sent',
+            'In Progress' => 'In Progress',
+            'Complete'    => 'Complete',
+        ];
+
         $instruments
             = \NDB_BVL_Instrument::getDirectEntryInstrumentNamesList(
                 $this->loris
             );
 
         return [
-            'visits'      => \Utility::getVisitList(),
-            'instruments' => $instruments,
+            'visits'        => \Utility::getVisitList(),
+            'instruments'   => $instruments,
+            'statusOptions' => $statusOptions,
         ];
     }
 

diff --git a/php/exceptions/ConflictException.class.inc b/php/exceptions/ConflictException.class.inc
@@ -0,0 +1,11 @@
+<?php
+/**
+ * This file contains the Conflict exception type.
+ *
+ * PHP Version 7
+ *
+ * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
+ */
+class ConflictException extends LorisException
+{
+}
diff --git a/php/libraries/BVL_Feedback_Panel.class.inc b/php/libraries/BVL_Feedback_Panel.class.inc
@@ -101,7 +101,15 @@ class BVL_Feedback_Panel
         $summary = $this->feedbackThread->getSummaryOfThreads();
         $this->tpl_data['thread_summary_headers'] = json_encode($summary);
 
-        $field_names = Utility::getSourcefields($_REQUEST['test_name'] ?? '');
+        $test_name = '';
+        if (array_key_exists('test_name', $_REQUEST)) {
+            $test_name = $_REQUEST['test_name'];
+        } else if (array_key_exists('lorispath', $_REQUEST)) {
+            $test_name = preg_split("#/#", $_REQUEST['lorispath'])[1] ?? '';
+        }
+
+        // Get field names
+        $field_names = Utility::getSourcefields($test_name);
         $fields      = [];
         $fields['Across All Fields'] = 'Across All Fields';
         foreach ($field_names as $field_name) {

diff --git a/php/libraries/Candidate.class.inc b/php/libraries/Candidate.class.inc
@@ -223,6 +223,7 @@ class Candidate implements \LORIS\StudyEntities\AccessibleResource,
         ProjectID $registrationProjectID = null
     ): CandID {
         $factory = NDB_Factory::singleton();
+        $db      = $factory->database();
 
         $site = \Site::singleton($centerID);
 
@@ -264,6 +265,25 @@ class Candidate implements \LORIS\StudyEntities\AccessibleResource,
                 );
             }
 
+            // check pscid uniqueness
+            $existing = $db->pselectOne(
+                'SELECT
+                  COUNT(*)
+                 FROM candidate
+                 WHERE PSCID = :v_pscid
+                 GROUP BY
+                   PSCID
+                ',
+                ['v_pscid' => $PSCID]
+            );
+
+            if ($existing > 0) {
+                throw new \ConflictException(
+                    "PSCID must be unique",
+                    PSCID_NOT_UNIQUE
+                );
+            }
+
             // check pscid structure
             if (!Candidate::validatePSCID(
                 $PSCID,

diff --git a/php/libraries/LorisForm.class.inc b/php/libraries/LorisForm.class.inc
@@ -1592,6 +1592,7 @@ class LorisForm
         $checked  = '';
         $value    = '';
         $disabled = '';
+        $required = '';
         if (!empty($val)) {
             $checked = 'checked="checked"';
         }
@@ -1601,6 +1602,9 @@ class LorisForm
         if (isset($el['disabled']) || $this->frozen) {
             $disabled = 'disabled';
         }
+        if (isset($el['required'])) {
+            $required = 'required';
+        }
         /// XXX: There seems to be a problem when using &nbsp; to separate the
         //  checkbox from the label. Both Firefox and Chrome will still put a
         //  linebreak between the space and the checkbox. Instead, we wrap use
@@ -1609,7 +1613,7 @@ class LorisForm
         //  label it's still allowed to have linebreaks.
         return "<span style=\"white-space: nowrap\"><input name=\"$el[name]\""
             . " type=\"checkbox\" $checked $value "
-            . "$disabled/>"
+            . "$disabled $required/>"
             . " </span>$el[label]";
     }
 

diff --git a/php/libraries/NDB_BVL_Instrument_LINST.class.inc b/php/libraries/NDB_BVL_Instrument_LINST.class.inc
@@ -734,14 +734,14 @@ class NDB_BVL_Instrument_LINST extends \NDB_BVL_Instrument
                 case 'numeric':
                     if ($addElements) {
                         $this->addNumericElement($pieces[1], $pieces[2]);
-                        $this->dictionary[] = new DictionaryItem(
-                            $this->testName."_".$pieces[1],
-                            $pieces[2],
-                            $scope,
-                            new IntegerType(),
-                            new Cardinality(Cardinality::SINGLE),
-                        );
                     }
+                    $this->dictionary[] = new DictionaryItem(
+                        $this->testName."_".$pieces[1],
+                        $pieces[2],
+                        $scope,
+                        new IntegerType(),
+                        new Cardinality(Cardinality::SINGLE),
+                    );
                     if ($firstFieldOfPage) {
                         $this->_requiredElements[] = $fieldname;
                         $firstFieldOfPage          = false;

diff --git a/php/libraries/VisitController.class.inc b/php/libraries/VisitController.class.inc
@@ -54,22 +54,31 @@ class VisitController
         return array_map(
             function ($row) {
                 return new \LORIS\Visit(
-                    $row['name'],
-                    $row['ID']
+                    $row['VisitName'],
+                    $row['VisitLabel']
                 );
             },
             $this->database->pselect(
-                'SELECT
-                 v.VisitID as "ID", v.VisitName as "name"
-                FROM
-                 visit v
-ORDER BY ID
-                ',
+                'SELECT VisitName, VisitLabel FROM visit ORDER BY VisitName',
                 []
             )
         );
     }
 
+    /**
+     * Get an associative array of the type
+     *
+     * @return array
+     */
+    public function getVisitlabels(): array
+    {
+        $visitLabels = [];
+        foreach ($this->getAllVisits() as $visitObj) {
+            $visitLabels[$visitObj->getName()] = $visitObj->getLabel();
+        }
+        return $visitLabels;
+    }
+
     /**
      * Retruns a VisitID by name
      *

diff --git a/test/unittests/LorisForms_Test.php b/test/unittests/LorisForms_Test.php
@@ -1336,7 +1336,7 @@ function testCheckboxHTMLWithNoAttributes()
         $this->form->addCheckbox("abc", "Hello", []);
         $this->assertEquals(
             "<span style=\"white-space: nowrap\"><input name=\"abc\" " .
-            "type=\"checkbox\"   /> </span>Hello",
+            "type=\"checkbox\"    /> </span>Hello",
             $this->form->checkboxHTML($this->form->form['abc'])
         );
     }
@@ -1358,7 +1358,7 @@ function testCheckboxHTMLWithAttributesSet()
         $this->assertEquals(
             "<span style=\"white-space: nowrap\"><input name=\"abc\" " .
             "type=\"checkbox\" checked=\"checked\"" .
-            " value=\"value1\" disabled/> </span>Hello",
+            " value=\"value1\" disabled /> </span>Hello",
             $this->form->checkboxHTML($this->form->form['abc'])
         );
     }