[ImgBrowser] Add DICOM Download #2791

christinerogers · 2017-05-08T23:46:53Z

This pull request adds DICOM download to the Imaging Browser View Session page, via a button in the navigation panel.
All files for the given session (specifically, tarchiveID) will be downloaded.
get_file.php is also modified to identify DICOM tarchives as type of file for download.

Note that your Dicom tarchives (and above directories) must be Executable* (unix permission-wise) for the download (get_file.php) to work.

mohadesz · 2017-05-10T16:01:41Z

modules/imaging_browser/php/Imaging_Session_ControlPanel.class.inc

+        // To enable DICOM download: Add pair: tarchiveID, ArchiveLocation
+        $tarchiveIDLoc = $DB->pselect(
+            "SELECT TarchiveID, ArchiveLocation
+            FROM tarchive


This should be a prepared statement

thanks for the catch 👍

mohadesz · 2017-05-10T16:07:28Z

Clicking on the download link doesn't do anything.

@mohadesz -- Are your Dicom tarchives (and above directories) Executable? (will only work if so, i'll amend the PR blurb at top)

MounaSafiHarab · 2017-05-12T17:03:57Z

@christinerogers

I meant to push to your branch not this pull request; I did not know it would make it directly here without you approving it first. Sorry!

christinerogers · 2017-05-23T14:50:00Z

htdocs/mri/jiv/get_file.php

@@ -136,10 +136,10 @@
    $MimeType         = 'image/vnd.nrrd';
    $DownloadFilename = basename($File);
    break;
-case 'DICOMTAR':   
+case 'tar':   


@MounaSafiHarab - good improvement to just handle all Tars.

if using 'tar' over DICOMTAR, let's also remove the code at that sets $FileExt = 'DICOMTAR'

I specified dicomtar, envisioning that Media Module etc might store other tars of stimulus files, or other non-imaging tars stored elsewhere (even genetics). i.e. Will all future tars in Loris be images accessible on the Imaging path?

@christinerogers

In the last commit,

I switched it back to have the option to allow other tar to be downloaded (so I went back to dicomtar), but I changed the "trachive" to "DCM_" which is automatically appended by the pipeline (tarchive no longer exists in the archivelocation as it is now a relative location).

ran phpcs

MounaSafiHarab · 2017-05-26T14:10:52Z

htdocs/mri/jiv/get_file.php

@@ -131,6 +137,12 @@
    $MimeType         = 'image/vnd.nrrd';
    $DownloadFilename = basename($File);
    break;
+case 'DICOMTAR':
+    // ADD case for DICOMTAR
+    $FullPath         = $imagePath . '/' . $File;


@christinerogers

$imagePath can eventually be changed to get the tarchiveLibraryDir from Config table (once #2649 is merged).

MounaSafiHarab · 2017-05-26T14:12:29Z

modules/imaging_browser/templates/imaging_session_controlpanel.tpl

+          <li><a href="{$baseurl}/dicom_archive/viewDetails/?tarchiveID={$tarchive}&backURL={$backURL|escape:"url"}">DICOM Archive(s) {$tarchive}</a></li>
+        {/foreach}
+        {foreach from=$subject.tarchiveidLoc item=tarchiveLoc}
+              <li><a href="/mri/jiv/get_file.php?file=tarchive/{$tarchiveLoc}" class="btn btn-primary btn-small">


@christinerogers

once this is merged (#2649); and get_file.php is made to use the Config variable tarchiveLibraryDir, the hardcoded tarchive in file=tarchive/{$tarchiveLoc} can be removed

MounaSafiHarab · 2017-05-26T14:13:58Z

modules/imaging_browser/php/Imaging_Session_ControlPanel.class.inc

+        // To enable DICOM download: Add pair: tarchiveID, ArchiveLocation
+        $qresult = $DB->pselectRow(
+            "SELECT TarchiveID, ArchiveLocation
+            FROM tarchive
            WHERE PatientName LIKE $ID",


@christinerogers

not sure how to use a prepared query with $ID being defined a few lines earlier with 2 cases and the second case being a concat of 3 different variables)...

I confirmed with @AnyhowStep, and he did not see the query the way it is as a security risk.

I think it's just not obvious in the diff that $ID is safe since GitHub is hiding that part of the code unless you manually expand it..

@christinerogers

discussed at the Imaging meeting of July 18th, 2017:

pselectRow should be pselect (we might have more than one tarchiveID per session)

Have the download DICOM button next to the Dicom Archive Link; and /or have the download cloud list the Dicom Tarchive ID next to it

add the download cloud in the dicom archive module as well (RM #9865); @davidblader

johnsaigle · 2017-06-14T18:42:11Z

~~Please see my PR as it touches the same file.~~

~~Since my PR is being sent to 17.0, the changes in this file should be updated to match the new formatting.~~

~~Please let me know if you have questions.~~

I'm using a different approach now; these changes shouldn't cause any issues. Removing the Blocked tag.

MounaSafiHarab · 2017-07-20T23:50:31Z

@christinerogers

for item 1. above, please see this:
christinerogers@c00366a
(and the snapshot in that pull request to your branch to have an idea of what it looks like in the front end)

MounaSafiHarab · 2017-12-21T17:26:20Z

@christinerogers
any news on this PR? what is the status? which release is it going to?

MounaSafiHarab · 2018-01-04T16:08:44Z

@christinerogers

let me know if you will update this or if you prefer that someone else to work on this so we can finalise it.
also now that the design specs are there for this module (depending on which release this pull request will go to), the README.md needs to be updated to account for the new feature introduced in this pull request.

christinerogers · 2018-01-19T17:06:12Z

Closing.

[ImgBrowser] Add DICOM Download link

8c44d0b

christinerogers added the Category: Feature PR or issue that aims to introduce a new feature label May 8, 2017

christinerogers added this to the 17-1 milestone May 8, 2017

christinerogers requested a review from MounaSafiHarab May 8, 2017 23:46

mohadesz reviewed May 10, 2017

View reviewed changes

jstirling91 modified the milestones: 17.1, 17-1 May 10, 2017

mohadesz added the State: Needs work PR awaiting additional work by the author to proceed label May 10, 2017

Christine Rogers and others added 2 commits May 10, 2017 18:21

updated w prepared query

ad0477a

Fixes that make the changes work; no prepared query though

642e46e

christinerogers commented May 23, 2017

View reviewed changes

phpcs and Christine comments

c652b3c

MounaSafiHarab reviewed May 26, 2017

View reviewed changes

johnsaigle added State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed and removed State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed labels Jun 14, 2017

MounaSafiHarab self-assigned this Jul 5, 2017

samirdas modified the milestones: 17.1, 17.2 Jul 6, 2017

davidblader self-assigned this Jul 18, 2017

davidblader mentioned this pull request Jul 19, 2017

[DICOM Archive] Add download tar functionality #2967

Merged

driusan removed this from the 18.1 milestone Oct 2, 2017

driusan added the [branch] minor label Oct 2, 2017

ridz1208 changed the base branch from 17.1-dev to minor October 21, 2017 17:24

MounaSafiHarab added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Jan 4, 2018

christinerogers closed this Jan 19, 2018

MounaSafiHarab mentioned this pull request Feb 15, 2018

Imaging Browser: Make Links to instruments project customizable and add the DICOM download: Redmines 12500 and 12580 #3496

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ImgBrowser] Add DICOM Download #2791

[ImgBrowser] Add DICOM Download #2791

christinerogers commented May 8, 2017 •

edited

Loading

mohadesz May 10, 2017

christinerogers May 10, 2017

mohadesz commented May 10, 2017 •

edited by christinerogers

Loading

MounaSafiHarab commented May 12, 2017

christinerogers May 23, 2017

MounaSafiHarab May 26, 2017

MounaSafiHarab May 26, 2017

MounaSafiHarab May 26, 2017 •

edited

Loading

MounaSafiHarab May 26, 2017

MounaSafiHarab Jun 8, 2017

driusan Jun 8, 2017

MounaSafiHarab Jul 19, 2017 •

edited

Loading

johnsaigle commented Jun 14, 2017 •

edited

Loading

MounaSafiHarab commented Jul 20, 2017 •

edited

Loading

MounaSafiHarab commented Dec 21, 2017

MounaSafiHarab commented Jan 4, 2018 •

edited

Loading

christinerogers commented Jan 19, 2018

[ImgBrowser] Add DICOM Download #2791

[ImgBrowser] Add DICOM Download #2791

Conversation

christinerogers commented May 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohadesz commented May 10, 2017 • edited by christinerogers Loading

MounaSafiHarab commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab May 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

johnsaigle commented Jun 14, 2017 • edited Loading

MounaSafiHarab commented Jul 20, 2017 • edited Loading

MounaSafiHarab commented Dec 21, 2017

MounaSafiHarab commented Jan 4, 2018 • edited Loading

christinerogers commented Jan 19, 2018

christinerogers commented May 8, 2017 •

edited

Loading

mohadesz commented May 10, 2017 •

edited by christinerogers

Loading

MounaSafiHarab May 26, 2017 •

edited

Loading

MounaSafiHarab Jul 19, 2017 •

edited

Loading

johnsaigle commented Jun 14, 2017 •

edited

Loading

MounaSafiHarab commented Jul 20, 2017 •

edited

Loading

MounaSafiHarab commented Jan 4, 2018 •

edited

Loading