Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Imaging Browser] Fix caveat bug not showing when there are pipeline protocol violations #8661

Merged
merged 6 commits into from
May 2, 2023
Merged

[Imaging Browser] Fix caveat bug not showing when there are pipeline protocol violations #8661

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8454b1f
add drop down for caveat only when there are no other violations in M…
cmadjar Apr 21, 2023
7f70f03
Fixed Caveat link
cmadjar Apr 21, 2023
02659dd
modify test plan for the caveat link
cmadjar Apr 24, 2023
4b2f7fb
link to MRI violation restricted to filename and seriesUID
cmadjar Apr 24, 2023
c2c3408
php sniffer
cmadjar Apr 24, 2023
1605dcc
fix test plan
cmadjar May 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 17 additions & 19 deletions modules/imaging_browser/jsx/ImagePanel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ class ImageQCDropdown extends Component {
} else {
dropdown = (
<div className="col-xs-12">
{this.props.defaultValue}
{this.props.options[this.props.defaultValue]}
</div>
);
}
Expand Down Expand Up @@ -491,24 +491,18 @@ class ImagePanelQCCaveatSelector extends Component {
render() {
// Link caveat to MRI Violations if set true
let mriViolationsLink = null;
if (this.props.SeriesUID && this.props.Caveat === '1') {
// If there is a manual caveat that was set, the link
// will take you to it, even though there might also
// be a caveat that was set by the MRI piepline (i.e
// not manual). Note that manual caveat are always
// resolved
sruthymathew123 marked this conversation as resolved.
Show resolved Hide resolved
if (this.props.CaveatViolationsResolvedID) {
mriViolationsLink = '/mri_violations/resolved_violations/?' +
'SeriesUID=' + this.props.SeriesUID + '&filter=true';
}
if (this.props.FullName && this.props.Caveat === '1') {
mriViolationsLink = '/mri_violations/?' +
'mincFile=' + this.props.FullName +
'&seriesUID=' + this.props.SeriesUID;
}

return (
<ImageQCDropdown
Label="Caveat"
FormName="caveat"
FileID={this.props.FileID}
editable={this.props.HasQCPerm}
editable={this.props.HasQCPerm && this.props.EditableCaveat}
options={
{
'': '',
Expand All @@ -527,7 +521,8 @@ ImagePanelQCCaveatSelector.propTypes = {
HasQCPerm: PropTypes.bool,
SeriesUID: PropTypes.string,
Caveat: PropTypes.string,
CaveatViolationsResolvedID: PropTypes.string,
EditableCaveat: PropTypes.bool,
FullName: PropTypes.string,
};


Expand Down Expand Up @@ -605,7 +600,8 @@ class ImagePanelQCPanel extends Component {
HasQCPerm={this.props.HasQCPerm}
Caveat={this.props.Caveat}
SeriesUID={this.props.SeriesUID}
CaveatViolationsResolvedID={this.props.CaveatViolationsResolvedID}
EditableCaveat={this.props.EditableCaveat}
FullName={this.props.FullName}
/>
<ImagePanelQCSNRValue
FileID={this.props.FileID}
Expand All @@ -624,7 +620,8 @@ ImagePanelQCPanel.propTypes = {
Caveat: PropTypes.string,
SeriesUID: PropTypes.string,
SNR: PropTypes.string,
CaveatViolationsResolvedID: PropTypes.string,
EditableCaveat: PropTypes.bool,
FullName: PropTypes.string,
};


Expand Down Expand Up @@ -924,10 +921,11 @@ class ImagePanelBody extends Component {
HasQCPerm={this.props.HasQCPerm}
QCStatus={this.props.QCStatus}
Caveat={this.props.Caveat}
CaveatViolationsResolvedID={this.props.CaveatViolationsResolvedID}
Selected={this.props.Selected}
SNR={this.props.SNR}
SeriesUID={this.props.SeriesUID}
EditableCaveat={this.props.EditableCaveat}
FullName={this.props.Fullname}
/>
</div>
</div>
Expand Down Expand Up @@ -971,8 +969,8 @@ ImagePanelBody.propTypes = {
JsonFile: PropTypes.string,
OtherTimepoints: PropTypes.string,
HeadersExpanded: PropTypes.bool,
CaveatViolationsResolvedID: PropTypes.string,
HeaderInfo: PropTypes.object,
EditableCaveat: PropTypes.bool,
};


Expand Down Expand Up @@ -1049,7 +1047,7 @@ class ImagePanel extends Component {
HasQCPerm={this.props.HasQCPerm}
QCStatus={this.props.QCStatus}
Caveat={this.props.Caveat}
CaveatViolationsResolvedID={this.props.CaveatViolationsResolvedID}
EditableCaveat={this.props.EditableCaveat}
Selected={this.props.Selected}
SNR={this.props.SNR}

Expand Down Expand Up @@ -1092,7 +1090,7 @@ ImagePanel.propTypes = {
HeaderInfo: PropTypes.object,
HeadersExpanded: PropTypes.string,
APIFile: PropTypes.string,
CaveatViolationsResolvedID: PropTypes.string,
EditableCaveat: PropTypes.bool,
};

let RImagePanel = React.createFactory(ImagePanel);
Expand Down
139 changes: 84 additions & 55 deletions modules/imaging_browser/php/viewsession.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,57 +317,63 @@ class ViewSession extends \NDB_Form
(int)$FileObj->getParameter('AcquisitionProtocolID')
);

$caveatViolationsResolvedID
$manualCaveatViolationsResolvedID
= $this->_getManualCaveatViolationsResolvedID($FileID);
$caveat = $caveatViolationsResolvedID ? 1 : 0;
$file = [
'FileID' => $FileID,
'Filename' => $Filename,
'APIFile' => $APIFile,
'FullFilename' => $FullFilename,
'New' => $New,
'OutputType' => $OutputType,
'AcquisitionProtocol' => $AcquisitionProtocol,
'CoordinateSpace' => $CoordinateSpace,
'AcquisitionDate' => $AcquisitionDate,
'ProcDate' => $ProcDate,
'FileInsertDate' => $FileInsertDate,
'SeriesDescription' => $SeriesDescription,
'SeriesNumber' => $SeriesNumber,
'SeriesUID' => $SeriesUID,
'EchoTime' => $EchoTime,
'InversionTime' => $InversionTime,
'RepetitionTime' => $RepetitionTime,
'SliceThickness' => $SliceThickness,
'Time' => $Time,
'ImageType' => $ImageType,
'PhaseEncodingDirection' => $PhaseEncodingDirection,
'EchoNumber' => $EchoNumber,
'Comment' => $Comment,
'ProcessingPipeline' => $ProcessingPipeline,
'TotalRejected' => $TotalRejected,
'SourceFile' => $SourceFile,
'Tool' => $Tool,
'SlicewiseRejected' => $SlicewiseRejected,
'InterlaceRejected' => $InterlaceRejected,
'IntergradientRejected' => $IntergradientRejected,
'Xstep' => $Xstep,
'Ystep' => $Ystep,
'Zstep' => $Zstep,
'Selected' => $Selected,
'QCStatus' => $QCStatus,
'QCDate' => $QCDate,
'Caveat' => $caveat,
'SNR' => $SNR,
'XMLreport' => $XMLreport,
'XMLprotocol' => $XMLprotocol,
'NrrdFile' => $NrrdFile,
'NiiFile' => $NiiFile,
'BvalFile' => $BvalFile,
'BvecFile' => $BvecFile,
'JsonFile' => $JsonFile,
'OtherTimepoints' => $OtherTimepoints,
'CaveatViolationsResolvedID' => $caveatViolationsResolvedID,
$caveat = $manualCaveatViolationsResolvedID ? 1 : 0;
$foundProtocolViolations = $this->_getProtocolViolations($FileID);
$editableCaveat = true;
if ($foundProtocolViolations) {
$caveat = 1;
$editableCaveat = false;
}
$file = [
'FileID' => $FileID,
'Filename' => $Filename,
'APIFile' => $APIFile,
'FullFilename' => $FullFilename,
'New' => $New,
'OutputType' => $OutputType,
'AcquisitionProtocol' => $AcquisitionProtocol,
'CoordinateSpace' => $CoordinateSpace,
'AcquisitionDate' => $AcquisitionDate,
'ProcDate' => $ProcDate,
'FileInsertDate' => $FileInsertDate,
'SeriesDescription' => $SeriesDescription,
'SeriesNumber' => $SeriesNumber,
'SeriesUID' => $SeriesUID,
'EchoTime' => $EchoTime,
'InversionTime' => $InversionTime,
'RepetitionTime' => $RepetitionTime,
'SliceThickness' => $SliceThickness,
'Time' => $Time,
'ImageType' => $ImageType,
'PhaseEncodingDirection' => $PhaseEncodingDirection,
'EchoNumber' => $EchoNumber,
'Comment' => $Comment,
'ProcessingPipeline' => $ProcessingPipeline,
'TotalRejected' => $TotalRejected,
'SourceFile' => $SourceFile,
'Tool' => $Tool,
'SlicewiseRejected' => $SlicewiseRejected,
'InterlaceRejected' => $InterlaceRejected,
'IntergradientRejected' => $IntergradientRejected,
'Xstep' => $Xstep,
'Ystep' => $Ystep,
'Zstep' => $Zstep,
'Selected' => $Selected,
'QCStatus' => $QCStatus,
'QCDate' => $QCDate,
'Caveat' => $caveat,
'SNR' => $SNR,
'XMLreport' => $XMLreport,
'XMLprotocol' => $XMLprotocol,
'NrrdFile' => $NrrdFile,
'NiiFile' => $NiiFile,
'BvalFile' => $BvalFile,
'BvecFile' => $BvecFile,
'JsonFile' => $JsonFile,
'OtherTimepoints' => $OtherTimepoints,
'EditableCaveat' => $editableCaveat,
];

$this->tpl_data['files'][] = $file;
Expand Down Expand Up @@ -402,6 +408,29 @@ class ViewSession extends \NDB_Form
return $this->_DB->pselectOneInt($query, ['fileID' => $fileID]);
}

/**
* Gets the ID of the record in table violations_resolved associated
* to the manual caveat set on a given file.
*
* @param $fileID int the ID of the record in table files.
*
* @return ?array array of IDs of the associated records in table
* mri_violations_log or null if there is none.
*/
function _getProtocolViolations(int $fileID): ?array
{
$query = "
SELECT mvl.LogID
FROM files f
JOIN mri_violations_log mvl
ON (mvl.MincFile = f.File)
WHERE f.FileID =:fileID
AND mvl.Header NOT LIKE 'Manual Caveat Set by %';
";

return $this->_DB->pselect($query, ['fileID' => $fileID]);
}

/**
* Gets a rejected parameter according to its type
*
Expand Down Expand Up @@ -537,11 +566,11 @@ class ViewSession extends \NDB_Form
['FileID' => $curFileID]
);

$caveatViolationsResolvedID
$manualCaveatViolationsResolvedID
= $this->_getManualCaveatViolationsResolvedID($curFileID);
// If Caveat was set to true and there is not already a manual
// caveat for that scan, then the user wants to create one
if ($curCaveat === '1' && !$caveatViolationsResolvedID) {
if ($curCaveat === '1' && !$manualCaveatViolationsResolvedID) {
//----------------------------------------------------------//
// Insert a record in mri_violations_log to indicate that //
// there is an MRI violation for that scan. The Header //
Expand Down Expand Up @@ -622,9 +651,9 @@ class ViewSession extends \NDB_Form
['Caveat' => 1],
['FileID' => $curFileID],
);
} elseif ($curCaveat === '0' && $caveatViolationsResolvedID) {
} elseif ($curCaveat === '0' && $manualCaveatViolationsResolvedID) {
$quotedViolationsResolvedID
= $this->_DB->quote("$caveatViolationsResolvedID");
= $this->_DB->quote("$manualCaveatViolationsResolvedID");
$query = "
DELETE FROM mri_violations_log
WHERE LogID = (
Expand All @@ -638,7 +667,7 @@ class ViewSession extends \NDB_Form

$this->_DB->delete(
'violations_resolved',
['ID' => $caveatViolationsResolvedID]
['ID' => $manualCaveatViolationsResolvedID]
);

$quotedFileID = $this->_DB->quote($curFileID);
Expand Down
2 changes: 1 addition & 1 deletion modules/imaging_browser/templates/form_viewSession.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"Selected" : "{if $files[file].Selected}{$files[file].Selected}{/if}",

"Caveat" : "{$files[file].Caveat}",
"CaveatViolationsResolvedID" : "{$files[file].CaveatViolationsResolvedID}",
"EditableCaveat": "{$files[file].EditableCaveat}",
"SNR" : "{if $files[file].SNR}{$files[file].SNR}{/if}",
'HeaderInfo' : {
"SeriesUID" : "{$files[file].SeriesUID}",
Expand Down
36 changes: 19 additions & 17 deletions modules/imaging_browser/test/TestPlan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,37 +31,39 @@
### Main panel: Per Acquisition
16. Files can be downloaded (links clickable) if and only if user has at least one of the module permissions. Ensure that DICOM downloads are
prepended with the Patient Name.
17. Scan-level QC flags (Selected, pass/fail, Caveat emptor) are viewable to all, modifiable if and only if user has permission `imaging_browser_qc`.
18. There will be a Caveat link just above the Caveat drop-down in the image panel of a given scan if and only if there is an MRI violation with:
17. Scan-level QC flags (Selected, pass/fail) are viewable to all, modifiable if and only if user has permission `imaging_browser_qc`.
18. Scan-level Caveat emptor are viewable to all, modifiable if and only if user has permission `imaging_browser_qc` and there are no
pipeline protocol violations associated to the image in the MRI violation module (`Manual Caveat Set by...` protocol violation
with no other protocol violations can be modified). Example in raisinbread: `OTT170_300170_V1` `fieldmapBOLD` images have
pipeline protocol violations associated to the image in the MRI violation module and therefore should Caveat should not be editable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar: "...module and therefore should Caveat should not be editable."
Not part of your PR but I noticed that step 19 mentions the (old) resolved tab...

19. There will be a Caveat link just above the Caveat drop-down in the image panel of a given scan if and only if there is an MRI violation with:
i) the same path as the image file
ii) the same SeriesUID as the image's SeriesUID
iii) header set to 'Manual Caveat set by <username>'
iv) resolution type set to 'Inserted with flag'
Search in the MRI violations' Resolved tab using the scan's SeriesUID and image file path to verify this. Note that you can see the header
of an MRI violation by clicking on the 'Protocol Violation' link in the 'Problem' column.
19. In the imaging browser, pick a scan that does not have a Caveat link. Use the drop-down to set the Caveat to true and save your modification.
20. In the imaging browser, pick a scan that does not have a Caveat link. Use the drop-down to set the Caveat to true and save your modification.
Ensure that there is now a link to your newly created caveat and verify that when you click on it it redirects to the MRI violations' violations
resolved submenu with your caveat displayed in the search result table.
20. In the imaging browser, pick a scan that has a Caveat link. Use the drop-down to set the Caveat to false and save your modification.
21. In the imaging browser, pick a scan that has a Caveat link. Use the drop-down to set the Caveat to false and save your modification.
Ensure that there is no Caveat link for that scan now. Using the MRI violation module and the technique described in 17, validate that there
are no MRI violations that would justify the presence of a link for that scan.
21. Selected: can be set back to Null (blank).
22. Clicking on an image launches the BrainBrowser.
23. Clicking on the "QC Comments" button opens the QC comments window.
24. Longitudinal View button launches the BrainBrowser with images of the chosen modality for that specific candidate across visits/timepoints.
22. Selected: can be set back to Null (blank).
23. Clicking on an image launches the BrainBrowser.
24. Clicking on the "QC Comments" button opens the QC comments window.
25. Longitudinal View button launches the BrainBrowser with images of the chosen modality for that specific candidate across visits/timepoints.

### MRI-QC : Scan-level (QC Comments) dialog window
25. With the permission `imaging_browser_qc`, edit comments, checkboxes, and dropdown values.
26. Clicking Save should update the values.
27. Data should not be editable without this permission.
26. With the permission `imaging_browser_qc`, edit comments, checkboxes, and dropdown values.
27. Clicking Save should update the values.
28. Data should not be editable without this permission.

### Visit-level QC feedback dialog window
28. On the view session page, click the button "Visit Level Feedback".
29. The entries in this dialog should be editable when a user has the permission `imaging_browser_qc`.
30. Try editing comments (adding new ones, deleting old ones). Click Save and ensure the data is saved.
29. On the view session page, click the button "Visit Level Feedback".
30. The entries in this dialog should be editable when a user has the permission `imaging_browser_qc`.
31. Try editing comments (adding new ones, deleting old ones). Click Save and ensure the data is saved.

### Test the Candidate Dashboard widget
31. Go to the candidate dashboard in the candidate module and check the Imaging QC Summary widget.
32. Go to the candidate dashboard in the candidate module and check the Imaging QC Summary widget.
- For each visit, check that the QC status displayed matches the Imaging Browser Module by hovering over any visit to see detailed modality breakdown for visit.
- Click on a visit from the graph to access the imaging browser. Check that the link redirects to the correct scans in the imaging browser.
- For a few candidate/visits, check that all the files from the Imaging Browser Module appear in the widget for all the visits.
Expand Down