-
Notifications
You must be signed in to change notification settings - Fork 175
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.
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
[Dicom Archive] When File Does Not Exist, The User Is Not Told #4558
Comments
When I tested this on the test VM for a file that does not exist, I get an HTTP 404 error page (not a blank page). I agree that if the file does not exist it should not be clickable but there could technically be other reasons why the file cannot be downloaded (permission issues, file size limit, possible others...) So instead of checking all things that could go wrong, I'd be in favour of displaying the 404 page. I am open to discussions though. @HenriRabalais Let me know what you think |
@nicolasbrossard For some reason I wasn't getting that error message displayed. |
@HenriRabalais @nicolasbrossard I'm not getting the 404 page displayed on my workstation either. I'm just seeing a white page after clicking on one of the files. |
@maltheism what do you get when you click on archive 2009/DCM_2009-06-09_ImagingUpload-14-14-qM69wJ.tar? |
@nicolasbrossard On safari & firefox I'm getting a white page. On Chrome: The page can't be found with an no webpage was found for the webaddress that shows http://localhost/mri/jiv/get_file.php?file=2009/DCM_2009-06-09_ImagingUpload-14-14-qM69wJ.tar. and then it say's Search Google for localhost mri jiv get file. |
This problem still exist as of Test round 2. TestPlan Item 11. Problem: |
This seems like a Test VM issue. Now that Raisinbread contains MRI files to be downloaded the test VM seems to download these files without issue. This issue could still occur if a file is deleted from the back-end without the DB table being updated. In this case a decorated 404 page should be displayed. |
I can't replicate test this on my VM. The files are stored on a read-only filesystem and I don't believe my user account has permission to change this. |
@johnsaigle I guess if you change the path in the config module to a different path than /data-raisinbread/ for the image path, that should replicate the tarchive files not being found. A bit hacky but as long as it works. |
True, I'll try that |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
The Stale label is being removed automatically because some activity has occurred or because the developers have decided that this pull request is important and should not continue to be overlooked. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
The crashing should be resolved by #6076 but I have not actually done the verification I said I would earlier... I hope to get to it after the release. |
Item #11 of the Test Plan:
Ensure that clicking on any row in the 'Archive Location' column triggers a download of the corresponding archived DICOM study. Make sure that the copy of the file downloaded to your system is prepended with the Patient Name field. [Manual Testing]
When a file does not exist and the user attempts to download it, they are led to a blank page and are not told that the file could not be found. Ideally, files that do not exist should not be hyperlinked, or the user should be warned after click on the link that the file could not be found.
The text was updated successfully, but these errors were encountered: