From d74665492d5b0b3f5d4f17ebf1da72a98ff7bd56 Mon Sep 17 00:00:00 2001 From: charlottesce <75381352+charlottesce@users.noreply.github.com> Date: Tue, 7 Nov 2023 13:30:55 -0500 Subject: [PATCH 01/11] [issue_tracker] Do not display inactive users in issue form (#8841) Inactive users are not displayed in dropdowns to assign / watch an issue. --- modules/issue_tracker/php/edit.class.inc | 49 ++++++++++++++----- modules/issue_tracker/php/issue.class.inc | 17 +++++++ .../issue_tracker/php/issue_tracker.class.inc | 4 +- 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc index 0e3a0680d52..3c79e1d65f0 100644 --- a/modules/issue_tracker/php/edit.class.inc +++ b/modules/issue_tracker/php/edit.class.inc @@ -66,7 +66,8 @@ class Edit extends \NDB_Page implements ETagCalculator $inactive_users = []; if ($user->hasPermission('access_all_profiles')) { $assignee_expanded = $db->pselect( - "SELECT Real_name, UserID FROM users", + "SELECT Real_name, UserID FROM users + WHERE Active='Y' AND Pending_approval='N'", [] ); @@ -84,7 +85,8 @@ class Edit extends \NDB_Page implements ETagCalculator $assignee_expanded = $db->pselect( "SELECT DISTINCT u.Real_name, u.UserID FROM users u LEFT JOIN user_psc_rel upr ON (upr.UserID=u.ID) - WHERE FIND_IN_SET(upr.CenterID,:CenterID) OR (upr.CenterID=:DCC)", + WHERE FIND_IN_SET(upr.CenterID,:CenterID) OR (upr.CenterID=:DCC) + AND Active='Y' AND Pending_approval='N'", [ 'CenterID' => $CenterID, 'DCC' => $DCCID, @@ -113,7 +115,8 @@ class Edit extends \NDB_Page implements ETagCalculator $otherWatchers = []; $potential_watchers_expanded = $db->pselect( - "SELECT Real_name, UserID FROM users", + "SELECT Real_name, UserID FROM users + WHERE Active='Y' AND Pending_approval='N'", [] ); foreach ($potential_watchers_expanded as $w_row) { @@ -189,7 +192,7 @@ class Edit extends \NDB_Page implements ETagCalculator ->withDataFrom($provisioner) ->toArray($user); - $isWatching = $db->pselectOne( + $isWatching = $db->pselectOne( "SELECT userID, issueID FROM issues_watching WHERE issueID=:issueID AND userID=:userID", [ @@ -197,11 +200,28 @@ class Edit extends \NDB_Page implements ETagCalculator 'userID' => $user->getUsername(), ] ); - $issueData['watching'] = $isWatching === null ? 'No' : 'Yes'; + + // Add current assignee in assignees dropdown even if not active + if (!isset($assignees[$issueData['assignee']])) { + $assignees[$issueData['assignee']] = $db->pselectOne( + "SELECT Real_name FROM users + WHERE UserID=:userID", + [ + 'userID' => $issueData['assignee'] + ] + ); + } + + $othersWatching = $this->getWatching($issueID); + // add current watchers to others watching, even if not active + $otherWatchers = array_merge($otherWatchers, $othersWatching); + unset($otherWatchers[$user->getUserName()]); + + $issueData['watching'] = $isWatching === null ? 'No' : 'Yes'; $issueData['commentHistory'] = $this->getComments($issueID); $issueData['attachments'] = $attachments; $issueData['whoami'] = $user->getUsername(); - $issueData['othersWatching'] = $this->getWatching($issueID); + $issueData['othersWatching'] = array_keys($othersWatching); // We need to unescape the string here: // React is escaping the string in the template @@ -340,20 +360,23 @@ class Edit extends \NDB_Page implements ETagCalculator * * @param int $issueID the relevant issue * - * @return array those who are watching + * @return array [Real_name => userID] of + * those who are watching */ function getWatching(int $issueID): array { $db = $this->loris->getDatabaseConnection(); $watching = $db->pselect( - "SELECT userID from issues_watching WHERE issueID=:issueID", + "SELECT Real_name, UserID from users + WHERE UserID IN + (SELECT UserID FROM issues_watching WHERE issueID=:issueID)", ['issueID' => $issueID] ); $whoIsWatching = []; foreach ($watching as $watcher) { - $whoIsWatching[] = $watcher['userID']; + $whoIsWatching[$watcher['UserID']] = $watcher['Real_name']; } return $whoIsWatching; } @@ -823,13 +846,13 @@ class Edit extends \NDB_Page implements ETagCalculator /** * Puts updated fields into the issues_comments table. * - * @param string $comment new issue comment - * @param int $issueID the issue ID - * @param \User $user the user + * @param ?string $comment new issue comment + * @param int $issueID the issue ID + * @param \User $user the user * * @return void */ - function updateComments(string $comment, int $issueID, \User $user) + function updateComments(?string $comment, int $issueID, \User $user) { $db = $this->loris->getDatabaseConnection(); if (isset($comment) && $comment != "null") { diff --git a/modules/issue_tracker/php/issue.class.inc b/modules/issue_tracker/php/issue.class.inc index 2b3c5fb5734..6f1239370fd 100644 --- a/modules/issue_tracker/php/issue.class.inc +++ b/modules/issue_tracker/php/issue.class.inc @@ -159,4 +159,21 @@ class Issue extends \NDB_Form // the ajax/EditIssue.php hack. return parent::handle($request); } + + /** + * Generate a breadcrumb trail for this page. + * + * @return \LORIS\BreadcrumbTrail + */ + public function getBreadcrumbs(): \LORIS\BreadcrumbTrail + { + $label = ucwords(str_replace('_', ' ', $this->name)); + return new \LORIS\BreadcrumbTrail( + new \LORIS\Breadcrumb($label, "/$this->name"), + new \LORIS\Breadcrumb( + 'Issue', + "/issue_tracker/issue/$this->issueID" + ) + ); + } } diff --git a/modules/issue_tracker/php/issue_tracker.class.inc b/modules/issue_tracker/php/issue_tracker.class.inc index 20de6785fae..379052802b8 100644 --- a/modules/issue_tracker/php/issue_tracker.class.inc +++ b/modules/issue_tracker/php/issue_tracker.class.inc @@ -115,7 +115,7 @@ class Issue_Tracker extends \NDB_Menu_Filter_Form //reporters $reporters = []; $reporter_expanded = $db->pselect( - "SELECT u.UserID, + "SELECT DISTINCT u.UserID, u.Real_name FROM issues i INNER JOIN users u @@ -129,7 +129,7 @@ class Issue_Tracker extends \NDB_Menu_Filter_Form //assignees $assignees = []; $assignee_expanded = $db->pselect( - "SELECT u.UserID, + "SELECT DISTINCT u.UserID, u.Real_name FROM issues i INNER JOIN users u From c1f25c921cb4779758b93dc6cafb8a56dc892a1b Mon Sep 17 00:00:00 2001 From: Zaliqa Date: Fri, 24 Nov 2023 13:59:58 -0500 Subject: [PATCH 02/11] [LINST] skip metadatafields for surveys (#8961) Skip over metadatafields and calculate ages in surveys This is not a big change, one way or another the fields were either being skipped with an empty check, through a project override or through code in the module itself handling the submission. this is just an explicit check to make everything clearer --- php/libraries/NDB_BVL_Instrument.class.inc | 11 +++++++---- php/libraries/NDB_BVL_Instrument_LINST.class.inc | 4 +++- test/unittests/NDB_BVL_Instrument_Test.php | 12 +++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc index f767c0f97ef..52f89196824 100644 --- a/php/libraries/NDB_BVL_Instrument.class.inc +++ b/php/libraries/NDB_BVL_Instrument.class.inc @@ -298,6 +298,12 @@ abstract class NDB_BVL_Instrument extends NDB_Page $base . "project/instruments/$instrument.meta" ); } + // Set DataEntryType to normal or DirectEntry + if (isset($_REQUEST['key'])) { + $obj->DataEntryType = 'DirectEntry'; + } else { + $obj->DataEntryType = 'normal'; + } // Adds all of the form element and form rules to the page after // having instantiated the form above $obj->loadInstrumentFile( @@ -477,10 +483,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page { // ALWAYS INCLUDE THESE! - if (isset($_REQUEST['key'])) { - $this->DataEntryType = 'DirectEntry'; - } else { - $this->DataEntryType = 'normal'; + if ($this->DataEntryType === 'normal') { // These are required for Save Data to work properly, but not when it's // a direct data entry page $this->addHidden( diff --git a/php/libraries/NDB_BVL_Instrument_LINST.class.inc b/php/libraries/NDB_BVL_Instrument_LINST.class.inc index 1c196e3753d..ff8f50e0e05 100644 --- a/php/libraries/NDB_BVL_Instrument_LINST.class.inc +++ b/php/libraries/NDB_BVL_Instrument_LINST.class.inc @@ -556,7 +556,9 @@ class NDB_BVL_Instrument_LINST extends \NDB_BVL_Instrument 'instrument_title', $pieces[1] ); - $this->_addMetadataFields(); + if ($this->DataEntryType!=="DirectEntry") { + $this->_addMetadataFields(); + } } break; case 'begingroup': diff --git a/test/unittests/NDB_BVL_Instrument_Test.php b/test/unittests/NDB_BVL_Instrument_Test.php index e7570e52149..31946dbd1e6 100644 --- a/test/unittests/NDB_BVL_Instrument_Test.php +++ b/test/unittests/NDB_BVL_Instrument_Test.php @@ -1335,10 +1335,11 @@ function testSaveValueAndSave() { $this->_setUpMockDB(); $this->_setTableData(); - $this->_instrument->commentID = 'commentID1'; - $this->_instrument->table = 'medical_history'; - $this->_instrument->testName = 'Test'; - $this->_instrument->formType = "XIN"; + $this->_instrument->commentID = 'commentID1'; + $this->_instrument->table = 'medical_history'; + $this->_instrument->testName = 'Test'; + $this->_instrument->formType = "XIN"; + $this->_instrument->DataEntryType = "normal"; $values = ['Date_taken' => '2005-06-06', 'arthritis_age' => 2, 'arthritis_age_status' => 'status' @@ -1791,7 +1792,8 @@ function testDisplay() $this->_setUpMockDB(); $this->_setTableData(); $this->_instrument->setup("commentID1", "page"); - $this->_instrument->table = 'medical_history'; + $this->_instrument->table = 'medical_history'; + $this->_instrument->DataEntryType = "normal"; $this->assertStringContainsString( "\n", $this->_instrument->display() From 47637c87c1dddc51549c5e50d40a6260b831a438 Mon Sep 17 00:00:00 2001 From: CamilleBeau <51176779+CamilleBeau@users.noreply.github.com> Date: Fri, 1 Dec 2023 14:51:16 -0500 Subject: [PATCH 03/11] [tools] Fix efficiency of fix_candidate_age script (#8992) This fixes a bug in the fix_candidate_age.php script where it is getting CommentIDs from every instrument for each instrument, instead of just getting commentIDs for that instrument. This is making the script take n times as long where n = the number of instruments on the project. --- tools/data_integrity/fix_candidate_age.php | 41 ++++++---------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/tools/data_integrity/fix_candidate_age.php b/tools/data_integrity/fix_candidate_age.php index ab80693a233..2f45f100f1a 100644 --- a/tools/data_integrity/fix_candidate_age.php +++ b/tools/data_integrity/fix_candidate_age.php @@ -82,37 +82,16 @@ "SELECT f.CommentID FROM flag f JOIN session s ON s.ID=f.SessionID JOIN candidate c ON c.CandID=s.CandID - WHERE c.Active='Y' AND s.Active='Y'", - [] + WHERE c.Active='Y' AND s.Active='Y' + AND f.Test_name=:tn", + ['tn' => $testName] ); - foreach ($CommentIDs as $commentID) { - // Get Instrument Instance with commentID - try { - $instrument = NDB_BVL_Instrument::factory( - $lorisInstance, - $testName, - $commentID, - '', - false - ); - } catch (Exception $e) { - echo "\t$testName instrument row with CommentID: ".$commentID. - " was ignored for one of the following reasons:\n". - " - The candidate is inactive.\n". - " - The session is inactive.\n\n"; - continue; - } - - if (!$instrument) { - // instrument does not exist - echo "\t" - . "$testName for CommentID:$commentID could not be instantiated." - . "\n"; - continue; - } + $instrumentInstances = $instrument->bulkLoadInstanceData($CommentIDs); - $data = $instrument->getInstanceData(); + foreach ($instrumentInstances as $instrumentInstance) { + $data = $instrumentInstance->getInstanceData(); + $commentID = $instrumentInstance->getCommentID(); $dateTaken = $data['Date_taken'] ?? null; // Flag for problem with date @@ -125,8 +104,8 @@ $trouble =true; } else { // get Age from instrument class - $calculatedAge = $instrument->getCandidateAge(); - $calculatedAgeMonths = $instrument->calculateAgeMonths( + $calculatedAge = $instrumentInstance->getCandidateAge(); + $calculatedAgeMonths = $instrumentInstance->calculateAgeMonths( $calculatedAge ); //Compare age to value saved in the instrument table @@ -144,7 +123,7 @@ //Fix the saved values if confirm and trouble flags enabled if ($trouble && $confirm) { echo "\tFixing age for CommentID: ".$commentID."\n"; - $instrument->_saveValues(['Date_taken' => $dateTaken]); + $instrumentInstance->_saveValues(['Date_taken' => $dateTaken]); } } } From d7a599b292ab7f6e25de56ef7de82b00550bdf51 Mon Sep 17 00:00:00 2001 From: racostas <37309344+racostas@users.noreply.github.com> Date: Tue, 5 Dec 2023 10:53:19 -0500 Subject: [PATCH 04/11] [media] fixes issue when media module try to instantiate a non existing instrument. (#8903) A try catch block was added to prevent the module to break if for any reason one of the instruments could not be instantiated. Resolves #8902 --- .../media/php/mediafileprovisioner.class.inc | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/modules/media/php/mediafileprovisioner.class.inc b/modules/media/php/mediafileprovisioner.class.inc index 0f30249bba0..158a0070d61 100644 --- a/modules/media/php/mediafileprovisioner.class.inc +++ b/modules/media/php/mediafileprovisioner.class.inc @@ -91,10 +91,19 @@ class MediaFileProvisioner extends \LORIS\Data\Provisioners\DBRowProvisioner // make sure not to call the factory when $testname is null or '' if (!empty($testname) && !isset($this->_instrumentnames[$testname])) { - $this->_instrumentnames[$testname] = \NDB_BVL_Instrument::factory( - $this->lorisinstance, - $testname, - )->getFullname(); + try { + $this->_instrumentnames[$testname] = \NDB_BVL_Instrument::factory( + $this->lorisinstance, + $testname, + )->getFullname(); + } catch (\Exception $e) { + error_log( + "There was a problem instantiating the instrument ". + "'$testname'. Make sure the instrument is valid and ". + "functional in order for it to be displayed in the media ". + "module." + ); + } } $row['fullName'] = $this->_instrumentnames[$testname] ?? null; From bffcae307029e468da4b3cec6b47444c6bc7eec4 Mon Sep 17 00:00:00 2001 From: Rida Abou-Haidar Date: Tue, 12 Dec 2023 12:04:56 -0500 Subject: [PATCH 05/11] [LINST] fix survey and date issues (#8858) Add fields to XIN validation which are not parts of groups --- .../NDB_BVL_Instrument_LINST.class.inc | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/php/libraries/NDB_BVL_Instrument_LINST.class.inc b/php/libraries/NDB_BVL_Instrument_LINST.class.inc index ff8f50e0e05..12ea564aaf7 100644 --- a/php/libraries/NDB_BVL_Instrument_LINST.class.inc +++ b/php/libraries/NDB_BVL_Instrument_LINST.class.inc @@ -677,7 +677,10 @@ class NDB_BVL_Instrument_LINST extends \NDB_BVL_Instrument // The question should be added to the LinstQuestions in this // order, before the _date is stripped below for standard // dates to allow XINValidation to recognize the field name - $this->LinstQuestions[$pieces[1]] = ['type' => 'date']; + $this->LinstQuestions[$pieces[1]] = [ + 'type' => 'date', + 'dateFormat' => $dateFormat, + ]; if ($dateFormat === 'MonthYear') { // Shows date without day of month @@ -980,15 +983,27 @@ class NDB_BVL_Instrument_LINST extends \NDB_BVL_Instrument $q['UserRules'] = true; switch ($q['type']) { + // Selects (including multiselects), Basic Dates and MonthYears + // are the only type of rules that aren't part of a group + // the rest include a _status element case 'select': - // Selects are the only type of rules that aren't part - // of a group, the rest include a _status element $this->XINRegisterRule($question, $rules, $message); break; + case 'date': + if ($q['dateFormat'] === 'BasicDate' + || $q['dateFormat'] === 'MonthYear' + ) { + $this->XINRegisterRule($question, $rules, $message); + break; + } default: + $rules_array = array_merge( + $rules, + [$question.'_status{@}=={@}'] + ); $this->XINRegisterRule( $question, - $rules, + $rules_array, $message, $question . "_group" ); From 6b672723550ae8fdfb866fa7e0092bd0e997ab06 Mon Sep 17 00:00:00 2001 From: Saagar Arya <51128536+skarya22@users.noreply.github.com> Date: Tue, 12 Dec 2023 12:37:37 -0500 Subject: [PATCH 06/11] Add width prop to Modal (#8893) - Adds a width prop to Modal so that it can be changed from the default 700px - Backport from main branch. This would be used in CCNA v24.1 upgrade. --- jsx/Modal.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jsx/Modal.js b/jsx/Modal.js index 5e5ce1f6d30..2b22bf274ca 100644 --- a/jsx/Modal.js +++ b/jsx/Modal.js @@ -62,7 +62,7 @@ class Modal extends Component { * @return {JSX} - React markup for the component */ render() { - const {show, children, onSubmit, title} = this.props; + const {show, children, onSubmit, title, width} = this.props; const headerStyle = { display: 'flex', @@ -109,7 +109,7 @@ class Modal extends Component { margin: 'auto', padding: 0, border: '1px solid #888', - width: '700px', + width: width || '700px', boxShadow: '0 4px 8px 0 rbga(0,0,0,0.2), 0 6px 20px 0 rgba(0,0,0,0.19)', transition: 'top 0.4s, opacity 0.4s', }; @@ -175,6 +175,8 @@ Modal.propTypes = { onClose: PropTypes.func.isRequired, show: PropTypes.bool.isRequired, throwWarning: PropTypes.bool, + children: PropTypes.node, + width: PropTypes.string, }; Modal.defaultProps = { From 5b7ddb265302608260f54ba44dc3bca0f3f52441 Mon Sep 17 00:00:00 2001 From: CamilleBeau <51176779+CamilleBeau@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:08:10 -0500 Subject: [PATCH 07/11] [libraries] Get examiner sites by UserID (#8994) Fix bug where if a user has the same full name as another user, the examiner sites from the other user will show up in the edit user page. --- php/libraries/User.class.inc | 4 ++-- test/unittests/UserTest.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/php/libraries/User.class.inc b/php/libraries/User.class.inc index f41bbea209f..46a5a3dba8b 100644 --- a/php/libraries/User.class.inc +++ b/php/libraries/User.class.inc @@ -122,12 +122,12 @@ class User extends UserPermissions implements epr.pending_approval FROM examiners e JOIN examiners_psc_rel epr ON (e.examinerID=epr.examinerID) - WHERE e.full_name=:fn + WHERE e.userID=:uid AND (epr.active='Y' OR (epr.active='N' AND epr.pending_approval='Y') )", [ - "fn" => $row['Real_name'], + "uid" => $row['ID'], ] ); diff --git a/test/unittests/UserTest.php b/test/unittests/UserTest.php index 1a57b15ad1c..5101543117a 100644 --- a/test/unittests/UserTest.php +++ b/test/unittests/UserTest.php @@ -105,6 +105,7 @@ class UserTest extends TestCase * @var array */ private $_examinerInfo = [0 => ['full_name' => 'John Doe', + 'userID' => '1', 'examinerID' => 1, 'radiologist' => 1 ] From 18707d196b40082a19b36d313585deca3614f45c Mon Sep 17 00:00:00 2001 From: Rida Abou-Haidar Date: Tue, 12 Dec 2023 13:12:19 -0500 Subject: [PATCH 08/11] [genomic_browser] Download button not showing up in the Files tab (#8480) Made the name column a link for downloading. Note: This module needs refactoring of the upload and download logic to be more robust. Resolves #7594 --- .../genomic_browser/jsx/tabs_content/files.js | 12 +++++++-- .../genomic_browser/php/filemanager.class.inc | 27 +++++++++++++++++++ .../php/uploading/genomicfile.class.inc | 6 +++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/modules/genomic_browser/jsx/tabs_content/files.js b/modules/genomic_browser/jsx/tabs_content/files.js index 0aa6d446690..3e80b07deaa 100644 --- a/modules/genomic_browser/jsx/tabs_content/files.js +++ b/modules/genomic_browser/jsx/tabs_content/files.js @@ -35,6 +35,7 @@ class Files extends Component { }, }; this.fetchData = this.fetchData.bind(this); + this.formatColumn = this.formatColumn.bind(this); this.openFileUploadModal = this.openFileUploadModal.bind(this); this.closeFileUploadModal = this.closeFileUploadModal.bind(this); this.renderFileUploadForm = this.renderFileUploadForm.bind(this); @@ -151,9 +152,16 @@ class Files extends Component { formatColumn(column, cell, rowData, rowHeaders) { let reactElement; switch (column) { + case 'Name': + const fileName = rowData.Name.split('/').pop(); + const url = + `${this.props.baseURL + }/genomic_browser/FileManager?filename=${fileName}`; + reactElement = {fileName}; + break; case 'PSCID': - const url = `${this.props.baseURL}/${rowData.DCCID}/`; - reactElement = {rowData.PSCID}; + const urlPscid = `${this.props.baseURL}/${rowData.DCCID}/`; + reactElement = {rowData.PSCID}; break; case 'Subproject': reactElement = {this.state.data.subprojects[parseInt(cell)]}; diff --git a/modules/genomic_browser/php/filemanager.class.inc b/modules/genomic_browser/php/filemanager.class.inc index 83de23c925a..cde651bb333 100644 --- a/modules/genomic_browser/php/filemanager.class.inc +++ b/modules/genomic_browser/php/filemanager.class.inc @@ -54,6 +54,33 @@ class FileManager extends \NDB_Page implements ETagCalculator */ private function _handleGET(ServerRequestInterface $request) : ResponseInterface { + // Parse GET query params. + $values = $request->getQueryParams(); + // GET request for downloading file by ID. + if ($values['filename']) { + $factory = \NDB_Factory::singleton(); + $config = $factory->config(); + $filesDir = \Utility::appendForwardSlash( + $config->getSetting('GenomicDataPath') + ); + try { + $downloadhandler = new \LORIS\FilesDownloadHandler( + new \SplFileInfo($filesDir) + ); + $request = $request->withAttribute( + 'filename', + $values['filename'] + ); + return $downloadhandler->handle($request); + } catch (\LorisException $e) { + // FilesUploadHandler throws an exception if there's a problem with + // the downloaddir. + return new \LORIS\Http\Response\JSON\InternalServerError( + $e->getMessage() + ); + } + } + $provisioner = new FilesProvisioner(); $user = $request->getAttribute('user'); diff --git a/modules/genomic_browser/php/uploading/genomicfile.class.inc b/modules/genomic_browser/php/uploading/genomicfile.class.inc index f220a0cb836..46a9e209219 100644 --- a/modules/genomic_browser/php/uploading/genomicfile.class.inc +++ b/modules/genomic_browser/php/uploading/genomicfile.class.inc @@ -468,8 +468,10 @@ class Genomicfile function setFullPath(&$fileToUpload) : void { $config = \NDB_Config::singleton(); - $genomic_data_dir = rtrim($config->getSetting('GenomicDataPath'), '/') - . '/genomic_uploader/'; + $genomic_data_dir = rtrim( + $config->getSetting('GenomicDataPath'), + '/' + ) . '/'; $fileToUpload->full_path = $genomic_data_dir . $fileToUpload->file_name; From 32ef64d7c915d06927cc5a0299d7316f15e375c9 Mon Sep 17 00:00:00 2001 From: Zaliqa Date: Thu, 11 Jan 2024 09:06:33 -0500 Subject: [PATCH 09/11] [issue_tracker - raisinbread/tools] Fix issue tracker history wrong module ID (#8677) This single use tool https://github.com/aces/Loris/blob/main/tools/single_use/Convert_LorisMenuID_to_ModuleID.php failed to convert the now deprecated LorisMenuID to the current modules table ModuleID for rows in issues_history table. The histories now refer to old LorisMenuIDs which map to the wrong module in the new modules table. This fixes this discrepancy by providing a patch, fixing the raisinbread dataset , and updating the conversion tool. How this should be used for existing projects: - If project upgraded to 23.0 and so already ran the Convert_LorisMenuID_to_ModuleID.php script, the project should run the patch SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql --- SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql | 8 ++++++++ raisinbread/RB_files/RB_issues_history.sql | 9 ++++----- 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql diff --git a/SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql b/SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql new file mode 100644 index 00000000000..ed2469b6403 --- /dev/null +++ b/SQL/New_patches/2023-04-25-FixIssueWrongModuleID.sql @@ -0,0 +1,8 @@ +-- NOTE: This SQL patch follows up the running of single use tool `tools/single_use/Convert_LorisMenuID_to_ModuleID.php` +-- that was necessary to upgrade the `issues` table from LORIS version 22 to version 23. The tool forgot +-- to include an upgrade of the `issues_history` table, which is now tackled by this SQL patch. + +-- delete from issues_history any orphaned module IDs +DELETE FROM issues_history WHERE fieldChanged='module' AND issueID IN (SELECT issueID FROM issues WHERE module IS NULL); +-- set issues history module ID to correct moduleID, replacing old LorisMenu ID +UPDATE issues_history ih SET newValue=(SELECT i.module FROM issues i WHERE i.issueID=ih.issueID) WHERE fieldChanged='module'; diff --git a/raisinbread/RB_files/RB_issues_history.sql b/raisinbread/RB_files/RB_issues_history.sql index 314b3ec9cb8..bd12f73d592 100644 --- a/raisinbread/RB_files/RB_issues_history.sql +++ b/raisinbread/RB_files/RB_issues_history.sql @@ -22,7 +22,7 @@ INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldC INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (19,'admin','2016-09-02 20:30:38','assignee',16,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (20,'new issue to test assignee notification','2016-09-02 20:30:38','title',16,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (21,'Examiners','2016-09-02 20:30:38','category',16,'tester'); -INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (22,'21','2016-09-02 20:30:39','module',16,'tester'); +INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (22,'16','2016-09-02 20:30:39','module',16,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (23,'tester','2016-09-02 20:30:39','comment',16,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (24,'2016-09-02 12:30:38','2016-09-02 20:30:39','comment',16,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (25,'tester','2016-09-02 20:31:45','assignee',17,'tester'); @@ -39,13 +39,12 @@ INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldC INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (36,'3','2016-09-02 21:59:56','centerID',19,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (37,'testing if assignee gets email - new ticket','2016-09-02 21:59:56','title',19,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (38,'Examiners','2016-09-02 21:59:56','category',19,'tester'); -INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (39,'11','2016-09-02 21:59:56','module',19,'tester'); +INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (39,'17','2016-09-02 21:59:56','module',19,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (40,'tester','2016-09-02 22:02:29','assignee',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (41,'feedback','2016-09-02 22:02:29','status',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (42,'urgent','2016-09-02 22:02:30','priority',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (43,'testing if assignee gets email upon ticket update+re-assign','2016-09-02 22:02:30','title',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (44,'Imaging','2016-09-02 22:02:30','category',20,'tester'); -INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (45,'12','2016-09-02 22:02:30','module',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (46,'admin','2016-09-02 22:04:38','assignee',20,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (47,'tester','2016-09-02 23:45:40','assignee',21,'tester'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (48,'ticket to end all tickets ','2016-09-02 23:45:40','title',21,'tester'); @@ -62,12 +61,12 @@ INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldC INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (59,'2','2016-09-07 01:19:21','centerID',26,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (60,'testing site population','2016-09-07 01:19:21','title',26,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (61,'Behavioural Battery','2016-09-07 01:19:21','category',26,'admin'); -INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (62,'11','2016-09-07 01:19:21','module',26,'admin'); +INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (62,'17','2016-09-07 01:19:21','module',26,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (63,'test','2016-09-07 01:38:06','assignee',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (64,'3','2016-09-07 01:38:06','centerID',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (65,'test ticket with visit label','2016-09-07 01:38:06','title',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (66,'Behavioural Instruments','2016-09-07 01:38:06','category',27,'admin'); -INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (67,'10','2016-09-07 01:38:07','module',27,'admin'); +INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (67,'9','2016-09-07 01:38:07','module',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (68,'300002','2016-09-07 01:38:07','comment',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (69,'2','2016-09-07 01:39:23','sessionID',27,'admin'); INSERT INTO `issues_history` (`issueHistoryID`, `newValue`, `dateAdded`, `fieldChanged`, `issueID`, `addedBy`) VALUES (70,'2','2016-09-07 02:19:59','sessionID',27,'admin'); From 03d209275380664143fe79a04b82d3f1d5cd60ff Mon Sep 17 00:00:00 2001 From: Saagar Arya <51128536+skarya22@users.noreply.github.com> Date: Thu, 11 Jan 2024 06:12:47 -0800 Subject: [PATCH 10/11] [Issue Tracker] Issue Change Notifications (#8885) Fixed issue where the assignee dropdown did not show the current assignee Emails users who were added as watching When a comment is added, it adds it to the email to everyone watching --- modules/issue_tracker/jsx/IssueForm.js | 2 +- modules/issue_tracker/php/edit.class.inc | 139 ++++++++++++++---- .../test/issue_tracker_test_plan.md | 1 + .../email/issue_assigned_modified.tpl | 14 ++ smarty/templates/email/issue_change.tpl | 5 +- 5 files changed, 133 insertions(+), 28 deletions(-) create mode 100644 smarty/templates/email/issue_assigned_modified.tpl diff --git a/modules/issue_tracker/jsx/IssueForm.js b/modules/issue_tracker/jsx/IssueForm.js index 90b7a972617..13748be7fd9 100644 --- a/modules/issue_tracker/jsx/IssueForm.js +++ b/modules/issue_tracker/jsx/IssueForm.js @@ -26,7 +26,7 @@ class IssueForm extends Component { super(props); this.state = { - Data: [], + Data: {}, formData: {}, submissionResult: null, errorMessage: null, diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc index 3c79e1d65f0..b4705d6f839 100644 --- a/modules/issue_tracker/php/edit.class.inc +++ b/modules/issue_tracker/php/edit.class.inc @@ -222,6 +222,10 @@ class Edit extends \NDB_Page implements ETagCalculator $issueData['attachments'] = $attachments; $issueData['whoami'] = $user->getUsername(); $issueData['othersWatching'] = array_keys($othersWatching); + $issueData['assignee'] = $db->pselectOne( + "SELECT assignee FROM issues WHERE issueID=:issueID", + ['issueID' => $issueID] + ); // We need to unescape the string here: // React is escaping the string in the template @@ -370,7 +374,12 @@ class Edit extends \NDB_Page implements ETagCalculator $watching = $db->pselect( "SELECT Real_name, UserID from users WHERE UserID IN - (SELECT UserID FROM issues_watching WHERE issueID=:issueID)", + (SELECT userID from issues_watching + WHERE issueID=:issueID) + AND UserID NOT IN + (SELECT assignee FROM issues + WHERE issueID=:issueID + AND assignee IS NOT NULL)", ['issueID' => $issueID] ); @@ -454,6 +463,11 @@ class Edit extends \NDB_Page implements ETagCalculator $issueValues['lastUpdatedBy'] = $user->getUsername(); + $assignee = $db->pselectOne( + "SELECT assignee FROM issues WHERE issueID=:ID", + ['ID' => $issueID] + ); + $validatedInput = $this->validateInput($validateValues, $user); if (!is_array($validatedInput)) { // Error exists. return $validatedInput; @@ -504,18 +518,6 @@ class Edit extends \NDB_Page implements ETagCalculator } } - // Adding new assignee to watching - if (isset($issueValues['assignee'])) { - $nowWatching = [ - 'userID' => $issueValues['assignee'], - 'issueID' => $issueID, - ]; - $db->replace('issues_watching', $nowWatching); - - // sending email - $this->emailUser($issueID, $issueValues['assignee'], '', $user); - } - // Adding others from multiselect to watching table. if (isset($values['othersWatching'])) { @@ -542,19 +544,34 @@ class Edit extends \NDB_Page implements ETagCalculator ) { continue; } - $this->emailUser($issueID, '', $usersWatching, $user); + $this->emailUser( + $issueID, + '', + $usersWatching, + $user, + 'false', + $values + ); } } } // Add editor to the watching table unless they don't want to be added. - if (isset($values['watching']) && $values['watching'] == 'Yes') { + if (isset($values['watching']) + && $values['watching'] == 'Yes' + && (!isset($issueValues['assignee']) + || $issueValues['assignee'] !== $user->getUsername()) + ) { $nowWatching = [ 'userID' => $user->getUsername(), 'issueID' => $issueID, ]; $db->replace('issues_watching', $nowWatching); - } else if (isset($values['watching']) && $values['watching'] == 'No') { + } else if (isset($values['watching']) + && $values['watching'] == 'No' + && (!isset($issueValues['assignee']) + || $issueValues['assignee'] !== $user->getUsername()) + ) { $db->delete( 'issues_watching', [ @@ -563,6 +580,46 @@ class Edit extends \NDB_Page implements ETagCalculator ] ); } + // Adding new assignee to watching + if (isset($issueValues['assignee']) + && $issueValues['assignee'] !== $assignee + ) { + $nowWatching = [ + 'userID' => $issueValues['assignee'], + 'issueID' => $issueID + ]; + $db->replace('issues_watching', $nowWatching); + // sending email + $this->emailUser( + $issueID, + $issueValues['assignee'], + isset($usersWatching) ? $usersWatching : '', + $user, + 'false', + $values + ); + } else if (isset($issueValues['assignee']) + && $issueValues['assignee'] === $assignee + ) { + // sending email + $this->emailUser( + $issueID, + $issueValues['assignee'], + isset($usersWatching) ? $usersWatching : '', + $user, + 'true', + $values + ); + } else { + $this->emailUser( + $issueID, + '', + isset($usersWatching) ? $usersWatching : '', + $user, + 'false', + $values + ); + } return new \LORIS\Http\Response\JsonResponse( ['issueID' => $issueID] ); @@ -575,11 +632,13 @@ class Edit extends \NDB_Page implements ETagCalculator * @param string $changed_assignee changed assignee * @param string $changed_watcher changed watcher * @param \User $user the user requesting the change + * @param string $new_assignee_tag boolean of whether it is a new assignee + * @param array $values the values the user entered in the form * * @return void */ function emailUser(int $issueID, string $changed_assignee, - string $changed_watcher, \User $user + string $changed_watcher, \User $user, string $new_assignee_tag, array $values ) { $db = $this->loris->getDatabaseConnection(); $baseurl = \NDB_Factory::singleton()->settings()->getBaseURL(); @@ -596,17 +655,36 @@ class Edit extends \NDB_Page implements ETagCalculator $msg_data['issueID'] = $issueID; $msg_data['currentUser'] = $user->getUsername(); $msg_data['title'] = $title; + $msg_data['comment'] = $values['comment']; - if (isset($changed_assignee)) { + if (isset($changed_assignee) && $new_assignee_tag == 'false') { $issueChangeEmailsAssignee = $db->pselect( "SELECT - u.Email AS Email, - u.First_name AS firstname - FROM - users u - WHERE - u.UserID = :assignee - AND u.UserID != :currentUser", + u.Email AS Email, + u.First_name AS firstname + FROM + users u + WHERE + u.UserID = :assignee + AND u.UserID != :currentUser", + [ + 'assignee' => $changed_assignee, + 'currentUser' => $user->getUserName(), + ] + ); + if (isset($issueChangeEmailsAssignee[0])) { + $msg_data['firstname'] = $issueChangeEmailsAssignee[0]['firstname']; + \Email::send( + $issueChangeEmailsAssignee[0]['Email'], + 'issue_assigned.tpl', + $msg_data + ); + } + } else if (isset($changed_assignee) && $new_assignee_tag === 'true') { + $issueChangeEmailsAssignee = $db->pselect( + "SELECT u.Email as Email, u.First_name as firstname " . + "FROM users u WHERE u.UserID=:assignee + AND u.UserID<>:currentUser", [ 'assignee' => $changed_assignee, 'currentUser' => $user->getUserName(), @@ -618,7 +696,7 @@ class Edit extends \NDB_Page implements ETagCalculator \Email::send( $issueChangeEmailsAssignee[0]['Email'], - 'issue_assigned.tpl', + 'issue_assigned_modified.tpl', $msg_data ); } @@ -634,11 +712,13 @@ class Edit extends \NDB_Page implements ETagCalculator WHERE w.issueID = :issueID AND u.UserID = :watcher + AND u.UserID != :assignee AND u.UserID != :currentUser", [ 'issueID' => $issueID, 'watcher' => $changed_watcher, 'currentUser' => $user->getUserName(), + 'assignee' => $changed_assignee, ] ); @@ -829,6 +909,10 @@ class Edit extends \NDB_Page implements ETagCalculator function updateHistory(array $values, int $issueID, \User $user) { $db = $this->loris->getDatabaseConnection(); + $originalAssignee = $db->pselectOne( + 'SELECT assignee FROM issues where issueID = :issueID', + ['issueID' => $issueID] + ); foreach ($values as $key => $value) { // centerID is allowed to be NULL if (!empty($value) || $key === 'centerID') { @@ -838,6 +922,9 @@ class Edit extends \NDB_Page implements ETagCalculator 'issueID' => $issueID, 'addedBy' => $user->getUsername(), ]; + if ($key === 'assignee' && $originalAssignee === $value) { + continue; + } $db->unsafeInsert('issues_history', $changedValues); } } diff --git a/modules/issue_tracker/test/issue_tracker_test_plan.md b/modules/issue_tracker/test/issue_tracker_test_plan.md index 3aef36d8cb1..2f56604eed3 100644 --- a/modules/issue_tracker/test/issue_tracker_test_plan.md +++ b/modules/issue_tracker/test/issue_tracker_test_plan.md @@ -40,6 +40,7 @@ 10. Test if users assigned to issues can upload attachments. 11. Test if users can delete their own uploaded attachments. 12. Test if user assigned to issue cannot delete attachments of issue owner. +13. Test that emails are sent to users that are watching the issue. ## Permissions [Automation Testing] 1. Remove `access_all_profiles` permission. diff --git a/smarty/templates/email/issue_assigned_modified.tpl b/smarty/templates/email/issue_assigned_modified.tpl new file mode 100644 index 00000000000..8fa7792263a --- /dev/null +++ b/smarty/templates/email/issue_assigned_modified.tpl @@ -0,0 +1,14 @@ +Subject: Issue Assigned - # {$issueID} +{$firstname}, + +The issue "{$title}" that is assigned to you has been modified. + +{if $comment !== "null"} + {$currentUser} commented: "{$comment}" + +{/if} +Please see the issue here: {$url} + +Thank you, + +LORIS Team \ No newline at end of file diff --git a/smarty/templates/email/issue_change.tpl b/smarty/templates/email/issue_change.tpl index 8a8b53a9188..9559382c328 100644 --- a/smarty/templates/email/issue_change.tpl +++ b/smarty/templates/email/issue_change.tpl @@ -1,9 +1,12 @@ Subject: Change to Issue # - {$issueID} - {$firstname}, {$currentUser} has updated an issue "{$title}" you are watching. +{if $comment !== "null"} + {$currentUser} commented: "{$comment}" +{/if} + Please view the changes here: {$url} Thank you, From 7d8f81d7c8e6e76e246c64e3afe2732cf50fd05f Mon Sep 17 00:00:00 2001 From: racostas <37309344+racostas@users.noreply.github.com> Date: Thu, 11 Jan 2024 09:15:38 -0500 Subject: [PATCH 11/11] [behavioural_qc] fixes visitLevel feedback not showing up. (#8900) Fix visit level feedback not showing up in behavioural_qc module. The visit Level feedback is now included as part of the formatColumn() function in the BehaviouralFeedback class. --- .../jsx/tabs_content/behaviouralFeedback.js | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/modules/behavioural_qc/jsx/tabs_content/behaviouralFeedback.js b/modules/behavioural_qc/jsx/tabs_content/behaviouralFeedback.js index ba353331bae..c72ea30c980 100644 --- a/modules/behavioural_qc/jsx/tabs_content/behaviouralFeedback.js +++ b/modules/behavioural_qc/jsx/tabs_content/behaviouralFeedback.js @@ -109,23 +109,32 @@ class BehaviouralFeedback extends Component { ); break; case 'Feedback Level': - rowData['Instrument'] ? reactElement = ( + let bvlLink = ''; + let bvlLevel = ''; + if (rowData['Instrument']) { + bvlLink = this.props.baseURL + + '/instruments/' + + rowData['Test Name'] + + '/?candID=' + + rowData['DCCID'] + + '&sessionID=' + + rowData['sessionID'] + + '&commentID=' + + rowData['commentID']; + bvlLevel ='Instrument : ' + rowData['Instrument']; + } else if (rowData['Visit']) { + bvlLink = this.props.baseURL + + '/instrument_list/' + + '?candID=' + + rowData['DCCID'] + + '&sessionID=' + + rowData['sessionID']; + bvlLevel ='Visit : ' + rowData['Visit']; + } + reactElement = ( - - {'Instrument : ' + rowData['Instrument']} - + {bvlLevel} - ) : reactElement = ( - {''} ); break; default: