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

[gluneau]

The new column is now in line with the tblScanTypes from the settings.

This removes hard coded AcquisitionProtocolID values from this module.

[driusan]

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?

[gluneau]

$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.

[gluneau]

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

[johnsaigle]

Moving this to 17.2 as it was stated in the LORIS meeting that Cleanup PRs should not be going to 17.1 this close to the release.

[gluneau]

@johnsaigle This PR is important to the IBIS project since if it is merged in 17.1, this module will no longer need to be overridden. Thank you for considering projects requirements as well.

[johnsaigle]

Cool works for me.

[xlecours]

This is effectively filtering out the 2 dti files from the raisinbread dataset if the only tblScanTypes are t1 and t2. If I add 'dti' in the tblScanTypes config list, the files are shown.

[xlecours]

@driusan I am moving this into the Senior dev review lane because it is your call :)

[MounaSafiHarab]

@xlecours @gluneau

I removed the passed manual test because what @xlecours noted is indeed happening and it is not the intended behavior.

@gluneau

1. if you select in the Imaging Module tab in Configuration a scan type type that you have no entries for in the files table for (for example, pick mrs or scout), this will remove all the "rows" in imaging browser (so something wrong with the query as a result of the addition of f.AcquisitionProtocolID IN ... statement)

(rows should not be impacted by what scan types you select; just columns)

2. if you select no mri_scan_type at all; then you will get an "error loading data" in imaging browser (and an error in SQL in the error_log)

[gluneau]

@MounaSafiHarab @xlecours I've modified the code to allow for no selected scan type.

I've also removed the scan type from the where clause since we do not want to auto filter out stuff

[xlecours]

@gluneau I am not sure what to test regarding the New Data column. The behaviour I see is an additional column in the table when a new tblScanTypes in the Configs.

[MounaSafiHarab]

@gluneau

Let me know when the New Data column filter is implemented so I can re-test

[MounaSafiHarab]

It is all good, except if you choose no scan types at all where you get an error loading data (it is an unlikely/impractical situation)

[MounaSafiHarab]

tested it, and it works well now!
@driusan I added the new feature/add to release notes because projects somehow need to know that filtering for NEW now only returns session entries that have non-QC'ed imaging modalities coming from the Configuration module.