[user_accounts] Use userID to update examiners table

CHANGELOG.md

@@ -20,6 +20,7 @@ database (PR #5260)
 - Fix edge-case that gave a confusing error message when changing password (PR #5956)
 - Fix bug where examiner form field was incorrectly hidden (PR #6085)
 - Fix special character double escaping in instruments (PR #6223)
+- Fix duplicate examiners created / examiners overwritten (PR #7462)
 
 ### Modules 
 #### Candidate Profile
@@ -78,6 +79,7 @@ exception). It is recommended to run this tool for existing projects (PR #5270)
 be used by projects having custom modules not in LORIS. (PR #5913)
 - Duplicate filenames in the data release module will cause an error when downloading. Make sure to remove all filename duplications before upgrading to this version. (PR #6461)
 - New tool for detecting and reporting the presence of double escaped special characters in the database instruments (PR #6477)
+- Run `tools/single_use/Remove_duplicate_examiners.php` to remove duplicate examiners that may have been created before bugfix. Make sure to run this script _before_ running the `SQL/New_patches/2021-09-28-Unique_examiners.sql`. (PR #7462)
 
 ### Notes For Developers
 - The tool `phpstan` has been added to our automated test suite. (PR #4928)

SQL/0000-00-00-schema.sql

@@ -1193,8 +1193,8 @@ CREATE TABLE `examiners` (
   `radiologist` tinyint(1) default 0 NOT NULL,
   `userID` int(10) unsigned DEFAULT NULL,
   PRIMARY KEY  (`examinerID`),
-  UNIQUE KEY `full_name` (`full_name`),
-  KEY `FK_examiners_2` (`userID`),
+  UNIQUE KEY `unique_examiner` (`full_name`,`userID`),
+  UNIQUE KEY `FK_examiners_2` (`userID`),
   CONSTRAINT `FK_examiners_2` FOREIGN KEY (`userID`) REFERENCES `users` (`ID`) ON DELETE SET NULL ON UPDATE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 

SQL/New_patches/2021-09-28-Unique_examiners.sql

@@ -0,0 +1,11 @@
+-- Note: For projects with duplicate examiners, patch must be run AFTER running
+-- tools/single_use/Remove_duplicate_examiners.php
+
+-- Remove constraint on full name
+-- Change userID to be unique
+-- Make full name / userID combined unique
+ALTER TABLE examiners
+	DROP INDEX `full_name`,
+	DROP INDEX `FK_examiners_2`,
+	ADD UNIQUE KEY `unique_examiner` (`full_name`,`userID`),
+	ADD UNIQUE KEY `FK_examiners_2` (`userID`);

modules/examiner/jsx/examinerIndex.js

@@ -237,6 +237,7 @@ class ExaminerIndex extends Component {
         name: 'examiner',
         type: 'text',
       }},
+      {label: 'Email', show: true},
       {label: 'ID', show: false},
       {label: 'Site', show: true, filter: {
         name: 'site',

modules/examiner/php/examiner.class.inc

@@ -72,6 +72,7 @@ class Examiner extends \NDB_Menu_Filter_Form
                             LEFT JOIN certification c ON 
                                 (c.examinerID=e.examinerID and c.pass = 'certified') 
                             LEFT JOIN test_names tn ON (tn.ID = c.testID) 
+                            LEFT JOIN users u ON u.ID=e.userID
                         WHERE 1=1 ";
         } else {
 
@@ -83,12 +84,14 @@ class Examiner extends \NDB_Menu_Filter_Form
                             LEFT JOIN certification c ON 
                                 (c.examinerID=e.examinerID and c.pass = 'certified') 
                             LEFT JOIN test_names tn ON (tn.ID = c.testID)
+                            LEFT JOIN users u ON u.ID=e.userID
                         WHERE FIND_IN_SET(epr.centerID, '$centerIDs')";
         }
 
         // set the class variables
         $this->columns      = array(
             'e.full_name as Examiner',
+            'u.Email',
             'e.examinerID as ID',
             'psc.Name as Site',
             'e.radiologist as Radiologist',
@@ -100,6 +103,7 @@ class Examiner extends \NDB_Menu_Filter_Form
         $this->order_by     = 'e.full_name';
         $this->headers      = array(
             'Examiner',
+            'Email',
             'ID',
             'Site',
             'Radiologist',
@@ -216,31 +220,24 @@ class Examiner extends \NDB_Menu_Filter_Form
             $radiologist = 1;
         }
 
-        //Check if examiner is already in the database 'examiners', if not add them
+        // Check if examiner is already in the database 'examiners', if not add them
+        // Assumption made that only non-users are added to the table here
+        // Otherwise, would need more information to identify which loris user
+        // the name refers to
         $examinerID = $DB->pselectOne(
             "SELECT examinerID
              FROM examiners
-             WHERE full_name=:fullName",
+             WHERE full_name=:fullName
+             AND userID IS NULL",
             array('fullName' => $fullName)
         );
 
-        //Check if examiner is already a loris user, if so add foreign key
-        $isUser = $DB->pselectOne(
-            "SELECT ID FROM users WHERE Real_name=:fullName",
-            array('fullName' => $fullName)
-        );
-
-        $user_id = null;
-        if (!empty($isUser)) {
-            $user_id = $isUser;
-        }
         if (empty($examinerID)) {
             $DB->insert(
                 'examiners',
                 array(
                     'full_name'   => $fullName,
-                    'radiologist' => $radiologist,
-                    'userID'      => $user_id,
+                    'radiologist' => $radiologist
                 )
             );
         } else {
@@ -278,7 +275,9 @@ class Examiner extends \NDB_Menu_Filter_Form
             "SELECT e.examinerID
              FROM examiners e
                   LEFT JOIN examiners_psc_rel epr ON (e.examinerID=epr.examinerID)
-             WHERE e.full_name=:fullName AND epr.centerID=:siteID",
+             WHERE e.full_name=:fullName 
+             AND epr.centerID=:siteID 
+             AND e.userID IS NULL",
             array(
                 'fullName' => $fullName,
                 'siteID'   => $siteID,
@@ -305,7 +304,7 @@ class Examiner extends \NDB_Menu_Filter_Form
         $examinerID = $DB->pselectOne(
             "SELECT examinerID
              FROM examiners
-             WHERE full_name=:fullName",
+             WHERE full_name=:fullName AND userID IS NULL",
             array('fullName' => $fullName)
         );
 

modules/user_accounts/php/edit_user.class.inc

@@ -300,7 +300,9 @@ class Edit_User extends \NDB_Form
         unset($values['ProjectIDs']);
         // END PROJECT UPDATE
 
-        // EXAMINER UPDATE
+        // SETUP EXAMINER VALUES
+        // The examiner will be updated after user is created
+        // so as to ensure an ID exists if new user
         if ($editor->hasPermission('examiner_multisite')) {
             // get all fields that are related to examiners
             $ex_curr_sites  = array();
@@ -323,13 +325,63 @@ class Edit_User extends \NDB_Form
                     $ex_pending = $v;
                 }
             }
+            foreach ($ex_curr_sites as $k => $v) {
+                unset($values[$k]);
+            }
+        }
+        unset($values['examiner_radiologist']);
+        unset($values['examiner_pending']);
+        // END SETUP EXAMINER VALUES
+
+        // make the set
+        $set = array();
+        foreach ($values as $key => $value) {
+            // Password updates are handled separately
+            if (!empty($value) && $key != 'Password_hash') {
+                $set[$key] = $value;
+            } else {
+                $set[$key] = null;
+            }
+        }
+        // update the user
+        if ($this->isCreatingNewUser()) {
+            // insert a new user
+            \User::insert($set);
+            $user = \User::factory($set['UserID']);
+            $uid  = $user->getData('ID');
+            foreach ($us_curr_sites as $site) {
+                $DB->insert(
+                    'user_psc_rel',
+                    array(
+                        "UserID"   => $uid,
+                        "CenterID" => $site,
+                    )
+                );
+            }
+            foreach ($us_curr_projects as $project) {
+                $DB->insert(
+                    'user_project_rel',
+                    array(
+                        "UserID"    => $uid,
+                        "ProjectID" => $project,
+                    )
+                );
+            }
+        } else {
+            // update the user
+            $user = \User::factory($this->identifier);
+            $user->update($set);
+        }
+
+        // START EXAMINER UPDATE
+        if ($editor->hasPermission('examiner_multisite')) {
 
             $examinerID = $DB->pselect(
                 "SELECT e.examinerID
              FROM examiners e
-             WHERE e.full_name=:fn",
+             WHERE e.userID=:uid",
                 array(
-                    "fn" => $values['Real_name'],
+                    "uid" => $uid,
                 )
             );
 
@@ -342,6 +394,7 @@ class Edit_User extends \NDB_Form
                 // If examiner not in table and radiologist, pending and current
                 // sites fields set add the examiner to the examiner table
                 $ex_radiologist = $ex_radiologist === 'Y' ? 1 : 0;
+
                 $DB->insert(
                     'examiners',
                     array(
@@ -353,8 +406,8 @@ class Edit_User extends \NDB_Form
                 $examinerID = $DB->pselectOne(
                     "SELECT examinerID
                             FROM examiners
-                            WHERE full_name=:fullName",
-                    array('fullName' => $values['Real_name'])
+                            WHERE userID=:uid",
+                    array('uid' => $uid)
                 );
             } elseif (!empty($examinerID)
                 && ((!empty($ex_radiologist)
@@ -370,10 +423,10 @@ class Edit_User extends \NDB_Form
                     'examiners',
                     array(
                         'radiologist' => $ex_radiologist,
-                        'userID'      => $uid,
+                        'full_name'   => $values['Real_name']
                     ),
                     array(
-                        "full_name" => $values['Real_name'],
+                        'userID' => $uid
                     )
                 );
                 $examinerID = $examinerID[0]['examinerID'];
@@ -428,7 +481,6 @@ class Edit_User extends \NDB_Form
                         )
                     );
                 }
-                unset($values[$k]);
             }
 
             //de-activate examiner if sites where no longer checked
@@ -444,50 +496,8 @@ class Edit_User extends \NDB_Form
                 );
             }
         }
-        unset($values['examiner_radiologist']);
-        unset($values['examiner_pending']);
         //END EXAMINER UPDATE
 
-        // make the set
-        $set = array();
-        foreach ($values as $key => $value) {
-            // Password updates are handled separately
-            if (!empty($value) && $key != 'Password_hash') {
-                $set[$key] = $value;
-            } else {
-                $set[$key] = null;
-            }
-        }
-        // update the user
-        if ($this->isCreatingNewUser()) {
-            // insert a new user
-            \User::insert($set);
-            $user = \User::factory($set['UserID']);
-            $uid  = $user->getData('ID');
-            foreach ($us_curr_sites as $site) {
-                $DB->insert(
-                    'user_psc_rel',
-                    array(
-                        "UserID"   => $uid,
-                        "CenterID" => $site,
-                    )
-                );
-            }
-            foreach ($us_curr_projects as $project) {
-                $DB->insert(
-                    'user_project_rel',
-                    array(
-                        "UserID"    => $uid,
-                        "ProjectID" => $project,
-                    )
-                );
-            }
-        } else {
-            // update the user
-            $user = \User::factory($this->identifier);
-            $user->update($set);
-        }
-
         // Now set the password. Note that this field is named incorrectly
         // and represents a plaintext password, not a hash.
         if (isset($values['Password_hash'])) {

php/libraries/NDB_BVL_Instrument.class.inc

@@ -1041,13 +1041,14 @@ abstract class NDB_BVL_Instrument extends NDB_Page
                 "SELECT ID FROM test_names WHERE Test_name=:tst_name",
                 array('tst_name' => $this->testName)
             );
-            $results = $db->pselect(
-                "SELECT c.examinerID, e.full_name
+            $results = $db->pselectWithIndexKey(
+                "SELECT c.examinerID, e.full_name, u.Email
                  FROM certification c
                      JOIN examiners e
                          ON (c.examinerID = e.examinerID)
                      JOIN examiners_psc_rel epr
                          ON (epr.examinerID=e.examinerID)
+                     LEFT JOIN users u ON u.ID=e.userID
                  WHERE c.testID =:tid
                     AND c.pass =:cert_id
                     AND epr.centerID =:cid
@@ -1056,24 +1057,41 @@ abstract class NDB_BVL_Instrument extends NDB_Page
                     'tid'     => $test_id,
                     'cert_id' => 'certified',
                     'cid'     => $centerID,
-                )
+                ),
+                'examinerID'
             );
         } else {
-            $results = $db->pselect(
-                "SELECT e.examinerID, e.full_name
+            $results = $db->pselectWithIndexKey(
+                "SELECT e.examinerID, e.full_name, u.Email
                      FROM examiners e
                          JOIN examiners_psc_rel epr
                              ON (epr.examinerID=e.examinerID)
+                     LEFT JOIN users u ON u.ID=e.userID
                  WHERE epr.centerID=:centID
                      ORDER BY full_name",
-                array('centID' => $centerID)
+                array('centID' => $centerID),
+                'examinerID'
             );
         }
-
-        $examiners = array('' => '');
+        $examiners = [];
         if (is_array($results) && !empty($results)) {
-            foreach ($results AS $row) {
-                $examiners[$row['examinerID']] = $row['full_name'];
+            foreach ($results AS $eid => $row) {
+                $name = $row['full_name'];
+                if (in_array($name, $examiners)) {
+                    // If name already in examiners, set first case of name
+                    // with email (if exists)
+                    $duplicateID    = array_search($name, $examiners);
+                    $duplicateEmail = $results[$duplicateID]['Email'];
+                    $emailString    = empty($duplicateEmail) ? "" :
+                        " ({$duplicateEmail})";
+                    $examiners[$duplicateID] = $name . $emailString;
+                    // Set new case of name with email as well
+                    $emailString     = empty($row['Email']) ? "" :
+                        " ({$row['Email']})";
+                    $examiners[$eid] = $name . $emailString;
+                } else {
+                    $examiners[$eid] = $name;
+                }
             }
         }
         return $examiners;