Display series specified by URL parameters in viewer by wayfarer3130 · Pull Request #2979 · OHIF/Viewers

wayfarer3130 · 2022-10-14T18:29:26Z

Display the series specified by the URL parameter seriesInstanceUID, and potentially other series parameters.

netlify · 2022-10-14T18:30:08Z

✅ Deploy Preview for ohif-platform-docs canceled.

Name	Link
🔨 Latest commit	`8e22794`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/6357e2a4a18792000c474a6e

wayfarer3130 · 2022-10-14T18:30:28Z

+
+  // const wadoDicomWebClient = new api.DICOMwebClient(wadoConfig);
+  const wadoDicomWebClient = staticWado
+    ? new StaticWadoClient(wadoConfig)


Just always create the StaticWadoClient, as it falls back to the base client if not required.

netlify · 2022-10-14T18:31:04Z

✅ Deploy Preview for ohif-platform-viewer ready!

Name	Link
🔨 Latest commit	`8e22794`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-viewer/deploys/6357e2a44f0739000919c757
😎 Deploy Preview	https://deploy-preview-2979--ohif-platform-viewer.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

wayfarer3130 · 2022-10-14T18:33:12Z

    "build": "lerna run build:viewer --stream",
    "build:dev": "lerna run build:dev --stream",
    "build:ci": "lerna run build:viewer:ci --stream",
+    "build:static": "lerna run build:static --stream && cp -rf ./platform/viewer/dist/* ~/ohif/.",


Don't want to include this line - suggest you run:
set APP_CONFIG=config/local_static.js
yarn start
which will automatically rebuild

wayfarer3130 · 2022-10-14T18:33:30Z

-        wadoUriRoot: 'http://localhost:5000/dicomweb',
-        qidoRoot: 'http://localhost:5000/dicomweb',
-        wadoRoot: 'http://localhost:5000/dicomweb',
+        wadoUriRoot: 'http://localhost:5001/dicomweb',


Definitely don't commit this

wayfarer3130 · 2022-10-14T18:34:20Z

 */
 function defaultRouteInit(
-  { servicesManager, studyInstanceUIDs, dataSource },
+  { servicesManager, studyInstanceUIDs, dataSource, seriesInstanceUID },


Grab the filters here direct, as there might be more than just seriesInstanceUID

codecov · 2022-10-14T18:35:05Z

Codecov Report

Merging #2979 (8e22794) into v3-stable (9f1e813) will increase coverage by 12.35%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           v3-stable    #2979       +/-   ##
==============================================
+ Coverage      25.15%   37.51%   +12.35%     
==============================================
  Files            119      101       -18     
  Lines           2862     2154      -708     
  Branches         555      440      -115     
==============================================
+ Hits             720      808       +88     
+ Misses          1856     1103      -753     
+ Partials         286      243       -43

Impacted Files	Coverage Δ
...tform/core/src/services/CineService/CineService.js	`12.50% <0.00%> (-6.25%)`	⬇️
.../services/MeasurementService/MeasurementService.js	`47.23% <0.00%> (-1.96%)`	⬇️
platform/core/src/classes/CommandsManager.js	`90.69% <0.00%> (-1.14%)`	⬇️
platform/core/src/classes/MetadataProvider.js	`4.54% <0.00%> (-0.72%)`	⬇️
...core/src/services/ToolBarService/ToolBarService.js	`1.36% <0.00%> (-0.13%)`	⬇️
platform/viewer/src/index.js	`0.00% <0.00%> (ø)`
platform/viewer/src/appInit.js	`0.00% <0.00%> (ø)`
platform/core/src/utils/index.js	`100.00% <0.00%> (ø)`
platform/core/src/utils/isImage.js	`100.00% <0.00%> (ø)`
platform/core/src/services/ServicesManager.js	`100.00% <0.00%> (ø)`
... and 48 more

Continue to review full report at Codecov.

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

wayfarer3130 · 2022-10-14T18:46:13Z


-
    const setupRouteInit = async () => {
+      const seriesInstanceUID = query.get('SeriesInstaceUID');


Grab from the query the actual full list of parameters as the filters, and then exclude the studyInstanceUIDs parameter from it, as that one shouldn't be present. Something like:
const filters = Object.fromEntries(query.values.filter(([key]) => key!='StudyInstanceUIDs'))
You should play with that in a nodejs command line tool - I may well not have it quite right there.

…re' of https://github.com/TFRadicalImaging/Viewers into feat/add-series-instance-uid-query-parameter-to-ohif-core

wayfarer3130 · 2022-10-17T18:14:46Z

    if (!valueElem) return false;
-    if (valueElem.vr == 'DA') return this.compareDateRange(testValue, valueElem.Value[0]);
+    if (valueElem.vr == 'DA')
+      return this.compareDateRange(testValue, valueElem.Value[0]);


If this is a separate line, it needs { around it

wayfarer3130 · 2022-10-17T18:15:02Z

    sortFunction
  );
  const data = await retrieveMetadataLoader.execLoad();
-


Please revert empty changes

wayfarer3130 · 2022-10-17T20:18:19Z

  const query = useQuery();
  const params = useParams();
-
+  // console.log('\n\n\n\n\n\n', { params, query }, '\n\n\n\n\n\n');


Please delete the console.log message

wayfarer3130 · 2022-10-17T20:19:15Z

+  };
+
+  static seriesFilterKeys = {
+    SeriesInstanceUID: '0020000E',


Can you add SeriesNumber to this list just to be able to test with an extra parameter for the filters.

wayfarer3130 · 2022-10-17T20:20:11Z

@sedghi - other than two small changes I think this one is ready to go - it adds the ability to choose which series to display on going to the viewer page Otherwise doesn't affect functionality.

wayfarer3130 · 2022-10-18T20:14:10Z

    : new api.DICOMwebClient(qidoConfig);
-  const wadoDicomWebClient = new api.DICOMwebClient(wadoConfig);
+
+  const wadoDicomWebClient = new StaticWadoClient(wadoConfig);


ALireza requested with have the staticWado ? .... style instead of replacing the thing entirely

sedghi · 2022-10-19T01:43:08Z

+      const filters = Array.from(query.keys()).reduce(
+        (acc: Record<string, string>, val: string) => {
+          if (val !== 'StudyInstanceUIDs') {
+            return { ...acc, [val]: query.get(val) };
+          }
+        },
+        {}
+      );


can you please write down what this reduce is making as a comment? like as an example in the top of it

sedghi · 2022-10-19T01:43:31Z

          servicesManager,
          studyInstanceUIDs,
          dataSource,
+          filters,


this might need to be passed to the custom init above as well

sedghi

This is great addition. one major comments

Currently the filter only works if it is lower case seresInstanceUID and it does not work with SeriesInstanceUID. Since we have StudyInstanceUIDs right now as a param in URL this will be super confusing. We need to fix this before merge, and also possibly somewhere add some documentation

…th StudyInstanceUID

… right change on Mode.tsx

sedghi · 2022-10-25T14:15:54Z

+      /**
+       * The next line should get all the query parameters provided by the URL
+       * - except the StudyInstaceUIDs - and create an object called filters
+       * used to filtering the study as the user wants otherwise it will return
+       * a empty object.
+       *
+       * Example:
+       * const filters = {
+       *   seriesInstaceUID: 1.2.276.0.7230010.3.1.3.1791068887.5412.1620253993.114611
+       * }
+       */


Nice comment, please keep it up always with details. Grat job

sedghi

This looks great, eventually we should have a page on doc (makes sense when we support more filters?) that demonstrate this feature. But for now good to go

WIP

9b85336

wayfarer3130 commented Oct 14, 2022

View reviewed changes

Thomas Forster added 3 commits October 14, 2022 16:16

WIP

1290691

Merge branch 'feat/add-series-instance-uid-query-parameter-to-ohif-co…

243acca

…re' of https://github.com/TFRadicalImaging/Viewers into feat/add-series-instance-uid-query-parameter-to-ohif-core

WIP

2a9239c

wayfarer3130 commented Oct 17, 2022

View reviewed changes

fixed Bill comments on PR - #2979

5a85e98

wayfarer3130 commented Oct 17, 2022

View reviewed changes

wayfarer3130 changed the title ~~WIP Display series specified by URL parameters in viewer~~ Display series specified by URL parameters in viewer Oct 17, 2022

added series number to series filter and removed extra console.log

c966b54

wayfarer3130 commented Oct 18, 2022

View reviewed changes

sedghi reviewed Oct 19, 2022

View reviewed changes

sedghi requested changes Oct 19, 2022

View reviewed changes

Thomas Forster and others added 4 commits October 21, 2022 11:09

changed the filter of seriesFilterKeys to studyFilterKeys

7e227f9

changed the seriesInstaceUID to SeriesInstanceUID to make coerence wi…

40f9431

…th StudyInstanceUID

reverted change on extension retrieveMetadataLoaderAsync and make the…

ea4ddf6

… right change on Mode.tsx

fixed PR comments and add small information about the filters

8e22794

sedghi reviewed Oct 25, 2022

View reviewed changes

sedghi approved these changes Oct 25, 2022

View reviewed changes

sedghi merged commit 2507a4b into OHIF:v3-stable Oct 25, 2022

TFRadicalImaging deleted the feat/add-series-instance-uid-query-parameter-to-ohif-core branch November 1, 2022 17:36



		const setupRouteInit = async () => {
		const seriesInstanceUID = query.get('SeriesInstaceUID');

Uh oh!

Uh oh!

Conversation

wayfarer3130 commented Oct 14, 2022

Uh oh!

netlify Bot commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for ohif-platform-docs canceled.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netlify Bot commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for ohif-platform-viewer ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 commented Oct 17, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sedghi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sedghi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify Bot commented Oct 14, 2022 •

edited

Loading

netlify Bot commented Oct 14, 2022 •

edited

Loading

codecov Bot commented Oct 14, 2022 •

edited

Loading

sedghi left a comment •

edited

Loading