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

[imaging_browser] Now relying on getSetting->tblScanTypes for the New Data column #2912

Merged
merged 7 commits into from
Aug 30, 2017
Merged

[imaging_browser] Now relying on getSetting->tblScanTypes for the New Data column #2912

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
774594c
Now relying on getSetting->tblScanTypes for the New Data column
gluneau Jun 28, 2017
c27e4f3
handle case where there is no selected scan types
gluneau Jul 6, 2017
a52e5ca
refactoring so new does what it should
gluneau Jul 18, 2017
d2e36cf
new new changes
gluneau Aug 28, 2017
ce43cd3
Fix variable scope
gluneau Aug 28, 2017
76f16c6
phpcs
gluneau Aug 28, 2017
c09db22
new needs single quotes in case it is a sql alias
gluneau Aug 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions modules/imaging_browser/php/NDB_Menu_Filter_imaging_browser.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ class NDB_Menu_Filter_Imaging_Browser extends NDB_Menu_Filter
as QCLastChangeTime FROM files
LEFT JOIN files_qcstatus USING (FileID)
WHERE OutputType='native' AND AcquisitionProtocolID
NOT IN (1, 2, 3, 52) GROUP BY files.SessionID) nd
IN (".
implode(",", array_keys($scan_id_types)) .") GROUP BY files.SessionID) nd
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know where $scan_id_types comes from, but this doesn't seem safe.

It looks like it's all scan id types that exist, making this in clause appear pointless.

Given that AcquisitionProtocolID 1, 2, and 3 don't exist because of the stupid default schema, and 52 in the default schema is 'scout' (which should probably be excluded at the protocol level, not in an imaging browser query), couldn't the same thing by achieved by just taking out the AcquisitionProtocolID part of the where clause?

Copy link
Contributor Author

@gluneau gluneau Jul 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

$scan_id_types comes from line 81 and got its values from the settings "tblScanTypes" at line 65

These are the protocols that are used, then its the ones that will be re-used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is based on Louis request that he only want the protocols that are being QCed to show as new in the new column.

ON (nd.SessionID=f.SessionID) ";

$left_joins = "";
Expand All @@ -145,7 +146,8 @@ class NDB_Menu_Filter_Imaging_Browser extends NDB_Menu_Filter
s.Active = 'Y' AND
f.PendingStaging=0 AND
f.FileType='mnc' AND
f.AcquisitionProtocolID not in (1, 2, 3, 52)";
f.AcquisitionProtocolID IN (".
implode(",", array_keys($scan_id_types)) .")";

$this->query .= $left_joins;
$this->query .= $where;
Expand Down