diff --git a/CHANGELOG.md b/CHANGELOG.md index 5653c89f0e4..1cc293c30ee 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/SQL/0000-00-00-schema.sql b/SQL/0000-00-00-schema.sql index 776634f0a16..a5e2198ed06 100644 --- a/SQL/0000-00-00-schema.sql +++ b/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; diff --git a/SQL/New_patches/2021-09-28-Unique_examiners.sql b/SQL/New_patches/2021-09-28-Unique_examiners.sql new file mode 100644 index 00000000000..e29c770cf3e --- /dev/null +++ b/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`); \ No newline at end of file diff --git a/modules/examiner/jsx/examinerIndex.js b/modules/examiner/jsx/examinerIndex.js index 5b5817ccd67..1298a8fc322 100644 --- a/modules/examiner/jsx/examinerIndex.js +++ b/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', diff --git a/modules/examiner/php/examiner.class.inc b/modules/examiner/php/examiner.class.inc index 47427f4a7d1..1238f86c9e5 100644 --- a/modules/examiner/php/examiner.class.inc +++ b/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) ); diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index ef9f426675c..a7348f93bc9 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/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'])) { diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc index 39d244cea8f..17fbbff90e3 100644 --- a/php/libraries/NDB_BVL_Instrument.class.inc +++ b/php/libraries/NDB_BVL_Instrument.class.inc @@ -1053,13 +1053,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 @@ -1068,24 +1069,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; diff --git a/tools/single_use/Remove_duplicate_examiners.php b/tools/single_use/Remove_duplicate_examiners.php new file mode 100644 index 00000000000..56e1c42bd37 --- /dev/null +++ b/tools/single_use/Remove_duplicate_examiners.php @@ -0,0 +1,85 @@ + + * @license Loris license + * @link https://www.github.com/aces/Loris-Trunk/ + */ + +require_once __DIR__."/../generic_includes.php"; +echo "##################################################################### +This script will update / delete duplicate examinerIDs caused by +user Name changes. Effected tables will be: certification, examiners, +examiner_psc_rel +#####################################################################\n\n"; + +// get examinerIDs linked to userIDs +$examiners = $DB->pselectColWithIndexKey( + "SELECT examinerID, userID FROM examiners WHERE userID IS NOT NULL", + [], + 'examinerID' +); + +// Get array of duplicate userIDs +$array_count = array_count_values($examiners); +$duplicateUIDS = array_keys( + array_filter( + $array_count, function($k){ return $k > 1;} + ) +); +$duplicates = array_filter($examiners, function($k) use(&$duplicateUIDS) { + return in_array($k, $duplicateUIDS); +}); + +if (count($duplicates) === 0) { + print_r("No duplicates found.\n"); +} else { + // Remove / update rows that are duplicate userIDs + // and do not have the correct full_name + + // The examinerIDs to be kept are the ones that have the + // same name as the current name in the users table + $correctExaminerIDs = $DB->pselectColWithIndexKey( + "SELECT e.userID, e.examinerID + FROM examiners e + JOIN users u ON u.ID=e.userID + WHERE e.userID IS NOT NULL + AND e.full_name=u.Real_name", + [], + 'userID' + ); + + foreach($duplicates as $examinerID => $userID) { + if (!in_array($examinerID, $correctExaminerIDs)) { + print_r("Updating examinerID for user $userID ". + "in certification\n"); + $DB->update( + 'certification', + [ + 'examinerID' => $correctExaminerIDs[$userID] + ], + ['examinerID' => $examinerID] + ); + + print_r("Removing row for user $userID ". + "with examinerID $examinerID in examiners and examiners_psc_rel\n\n"); + $DB->delete( + 'examiners', + ['examinerID' => $examinerID] + ); + $DB->delete( + 'examiners_psc_rel', + ['examinerID' => $examinerID] + ); + } + } +}