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.

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

feat(handler): Add handler for unsupported sopclassUIDs #3601

Merged
merged 8 commits into from
Aug 16, 2023

Conversation

rodrigobasilio2022
Copy link
Collaborator

@rodrigobasilio2022 rodrigobasilio2022 commented Aug 14, 2023

Context

Today the default OHIF's behaviour is to filter all unsupported SOPClassUIDs when opening a study. This behaviour is bad if the user wants to chekc if a particular series, not supported by OHIF, reaches the image server. This PR adds a new handler for all unsupported dicom series, adding a displaySet with an attribute, unsupported, that flags the displaySet cannot be used for viewer in OHIF. This PR is related to isssue #3362.

Changes & Results

For each unsupported series, a ThumbnailNoImage will appear with a warning icon, indicating that the particular series is unsupported and should not be used for viewing.

Testing

To test, one should alter the default.js to point to IDC test dataset and use one of the following links:

http://localhost:3000/viewer?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.308250233404174127024116250321380712318
http://localhost:3000/viewer?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.111778973144870709834087260276851352536
http://localhost:3000/viewer?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.2103.7010.237254940005841719629537222159
http://localhost:3000/viewer?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.122805535915539618236860659468790107523
http://localhost:3000/viewer?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.340031522181513869336683393500230688871

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • OS: Windows 11
  • Node version: 16.14
  • Browser: Chrome 116.0.5845.82

@netlify
Copy link

netlify bot commented Aug 14, 2023

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit cf4a27e
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/64dce2c74a6e9e0008f336c8
😎 Deploy Preview https://deploy-preview-3601--ohif-dev.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 14, 2023

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit cf4a27e
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/64dce2c7d4865a0008e8a9c6
😎 Deploy Preview https://deploy-preview-3601--ohif-platform-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rodrigobasilio2022 rodrigobasilio2022 added the IDC:priority Items that the Imaging Data Commons wants to help sponsor label Aug 14, 2023
@rodrigobasilio2022 rodrigobasilio2022 changed the title Add handler for unsupported sopclassUIds Add handler for unsupported sopclassUIDs Aug 14, 2023
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #3601 (cf4a27e) into master (ed0b01c) will not change coverage.
Report is 2 commits behind head on master.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3601   +/-   ##
=======================================
  Coverage   42.58%   42.58%           
=======================================
  Files          80       80           
  Lines        1463     1463           
  Branches      340      340           
=======================================
  Hits          623      623           
  Misses        675      675           
  Partials      165      165           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de6976d...cf4a27e. Read the comment docs.

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

see my comment. Also the PR description would benefit from some screenshots

@rodrigobasilio2022
Copy link
Collaborator Author

Code refactored

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

see my comments, thanks for the update

@rodrigobasilio2022
Copy link
Collaborator Author

Added changes required

@sedghi
Copy link
Member

sedghi commented Aug 16, 2023

isn't it supposed to show something on the thumbnail? I see nothing shown

CleanShot 2023-08-15 at 23 03 45

CleanShot 2023-08-15 at 23 04 20

@rodrigobasilio2022
Copy link
Collaborator Author

rodrigobasilio2022 commented Aug 16, 2023

Probably you forgot to set supportsWildcard: false. In IDC server if you don't set it that happens

@rodrigobasilio2022
Copy link
Collaborator Author

With supportsWildcard: false,

image

image

@sedghi
Copy link
Member

sedghi commented Aug 16, 2023

I guess that should be part of the testing section ....

@rodrigobasilio2022 rodrigobasilio2022 changed the title Add handler for unsupported sopclassUIDs Feat/Add handler for unsupported sopclassUIDs Aug 16, 2023
@rodrigobasilio2022 rodrigobasilio2022 changed the title Feat/Add handler for unsupported sopclassUIDs feat(handler): Add handler for unsupported sopclassUIDs Aug 16, 2023
Copy link
Contributor

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

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

LGTM

@rodrigobasilio2022
Copy link
Collaborator Author

Resolved the conflict in hangingprotocol.ts file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDC:priority Items that the Imaging Data Commons wants to help sponsor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants