From e7cc735c03d02eeb0d3af4ba02c15ed4f81bbec2 Mon Sep 17 00:00:00 2001 From: James Petts Date: Mon, 13 Jul 2020 13:51:33 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20Fix=20RT=20Panel=20hide/s?= =?UTF-8?q?how=20and=20Fix=20looping=20load=20errors=20(#1877)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 Fix RT Panel hide/show * Prevent infinite loop by persisting load errors on displaySet. * fix: 🐛 Fix infinite error messages * Fix unit tests. * comment out reamining MRSTUDY line --- .../src/OHIFDicomRTStructSopClassHandler.js | 5 +- .../src/components/RTPanel/RTPanel.js | 136 +++++++++++------- .../src/tools/modules/rtStructModule.js | 13 +- .../src/loadSegmentation.js | 1 + .../src/classes/metadata/StudyMetadata.js | 1 + .../utils/loadAndCacheDerivedDisplaySets.js | 7 + .../integration/common/OHIFRoutes.spec.js | 23 +-- .../integration/common/OHIFStudyList.spec.js | 12 +- .../PercyCheckOHIFRoutes.spec.js | 5 +- platform/viewer/public/config/idc.js | 3 +- 10 files changed, 127 insertions(+), 79 deletions(-) diff --git a/extensions/dicom-rt/src/OHIFDicomRTStructSopClassHandler.js b/extensions/dicom-rt/src/OHIFDicomRTStructSopClassHandler.js index d0a67d113..6561cf312 100644 --- a/extensions/dicom-rt/src/OHIFDicomRTStructSopClassHandler.js +++ b/extensions/dicom-rt/src/OHIFDicomRTStructSopClassHandler.js @@ -14,7 +14,7 @@ const OHIFDicomRTStructSopClassHandler = { id: 'OHIFDicomRTStructSopClassHandler', type: MODULE_TYPES.SOP_CLASS_HANDLER, sopClassUIDs, - getDisplaySetFromSeries: function ( + getDisplaySetFromSeries: function( series, study, dicomWebClient, @@ -67,13 +67,14 @@ const OHIFDicomRTStructSopClassHandler = { } } - rtStructDisplaySet.load = function (referencedDisplaySet, studies) { + rtStructDisplaySet.load = function(referencedDisplaySet, studies) { return loadRTStruct( rtStructDisplaySet, referencedDisplaySet, studies ).catch(error => { rtStructDisplaySet.isLoaded = false; + rtStructDisplaySet.loadError = true; throw new Error(error); }); }; diff --git a/extensions/dicom-rt/src/components/RTPanel/RTPanel.js b/extensions/dicom-rt/src/components/RTPanel/RTPanel.js index 80eb785d2..879c45e7b 100644 --- a/extensions/dicom-rt/src/components/RTPanel/RTPanel.js +++ b/extensions/dicom-rt/src/components/RTPanel/RTPanel.js @@ -38,7 +38,7 @@ const RTPanel = ({ isOpen, onContourItemClick, activeContexts = [], - contexts = {} + contexts = {}, }) => { const isVTK = () => activeContexts.includes(contexts.VTK); const isCornerstone = () => activeContexts.includes(contexts.CORNERSTONE); @@ -76,7 +76,10 @@ const RTPanel = ({ document.addEventListener('extensiondicomrtrtloaded', updateStructureSets); return () => { - document.removeEventListener('extensiondicomrtrtloaded', updateStructureSets); + document.removeEventListener( + 'extensiondicomrtrtloaded', + updateStructureSets + ); }; }, []); @@ -89,7 +92,9 @@ const RTPanel = ({ activeViewport.SeriesInstanceUID ); - const studyMetadata = studyMetadataManager.get(activeViewport.StudyInstanceUID); + const studyMetadata = studyMetadataManager.get( + activeViewport.StudyInstanceUID + ); const referencedDisplaysets = studyMetadata.getDerivedDatasets({ referencedSeriesInstanceUID: activeViewport.SeriesInstanceUID, Modality: 'RTSTRUCT', @@ -100,7 +105,7 @@ const RTPanel = ({ setState({ referencedDisplaysets, selectedSet: defaultSet, - sets: viewportSets + sets: viewportSets, }); } else { setState(DEFAULT_STATE); @@ -116,7 +121,10 @@ const RTPanel = ({ setShowSettings(showSettings && !isOpen); }, [isOpen]); - const toContourItem = ({ ROINumber, ROIName, RTROIObservations, colorArray, visible }, loadedSet) => { + const toContourItem = ( + { ROINumber, ROIName, RTROIObservations, colorArray, visible }, + loadedSet + ) => { let interpretedType = ''; if (RTROIObservations && RTROIObservations.RTROIInterpretedType) { interpretedType = `(${RTROIObservations.RTROIInterpretedType})`; @@ -149,14 +157,20 @@ const RTPanel = ({ ); const frameIndex = imageIds.indexOf(imageId); - const SOPInstanceUID = cornerstone.metaData.get('SOPInstanceUID', imageId); - const StudyInstanceUID = cornerstone.metaData.get('StudyInstanceUID', imageId); + const SOPInstanceUID = cornerstone.metaData.get( + 'SOPInstanceUID', + imageId + ); + const StudyInstanceUID = cornerstone.metaData.get( + 'StudyInstanceUID', + imageId + ); onContourItemClick({ StudyInstanceUID, SOPInstanceUID, frameIndex, - activeViewportIndex: activeIndex + activeViewportIndex: activeIndex, }); } }} @@ -166,7 +180,10 @@ const RTPanel = ({ visible={visible} onVisibilityChange={() => { const module = cornerstoneTools.getModule('rtstruct'); - module.setters.toggleROIContour(state.selectedSet.SeriesInstanceUID, ROINumber); + module.setters.toggleROIContour( + state.selectedSet.SeriesInstanceUID, + ROINumber + ); }} /> ); @@ -202,49 +219,70 @@ const RTPanel = ({ onClick={() => setShowSettings(true)} /> - {!state.referencedDisplaysets.length && } - {state.sets && state.referencedDisplaysets.map(displaySet => { - const { SeriesInstanceUID, metadata, isLoaded } = displaySet; + {!state.referencedDisplaysets.length && ( + + )} + {state.sets && + state.referencedDisplaysets.map(displaySet => { + const { SeriesInstanceUID, metadata, isLoaded } = displaySet; - const module = cornerstoneTools.getModule('rtstruct'); - const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid(viewports[activeIndex].SeriesInstanceUID); + const module = cornerstoneTools.getModule('rtstruct'); + const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid( + viewports[activeIndex].SeriesInstanceUID + ); - const loadedSet = sets.find(set => set.SeriesInstanceUID === SeriesInstanceUID); - return ( - { - const module = cornerstoneTools.getModule('rtstruct'); - loadedSet.ROIContours.forEach(({ ROINumber }) => { - module.setters.toggleROIContour(loadedSet.SeriesInstanceUID, ROINumber); - }); - const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid(viewports[activeIndex].SeriesInstanceUID); - setState(state => ({ ...state, sets })); - refreshViewport(); - }} - onExpandChange={async () => { - if (!isLoaded) { - await displaySet.load(viewports[activeIndex], studies); - const module = cornerstoneTools.getModule('rtstruct'); - const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid(viewports[activeIndex].SeriesInstanceUID); - const selectedSet = sets.find(set => set.SeriesInstanceUID === SeriesInstanceUID); - setState(state => ({ ...state, selectedSet, sets })); + const loadedSet = sets.find( + set => set.SeriesInstanceUID === SeriesInstanceUID + ); + return ( + - - - {isLoaded && loadedSet.ROIContours.map(c => toContourItem(c, loadedSet))} - - - - ); - })} + onVisibilityChange={newVisibility => { + const module = cornerstoneTools.getModule('rtstruct'); + + if (newVisibility) { + module.setters.showStructureSet(loadedSet.SeriesInstanceUID); + } else { + module.setters.hideStructureSet(loadedSet.SeriesInstanceUID); + } + const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid( + viewports[activeIndex].SeriesInstanceUID + ); + setState(state => ({ ...state, sets })); + refreshViewport(); + }} + onExpandChange={async () => { + if (!isLoaded) { + await displaySet.load(viewports[activeIndex], studies); + const module = cornerstoneTools.getModule('rtstruct'); + const sets = module.getters.structuresSetsWhichReferenceSeriesInstanceUid( + viewports[activeIndex].SeriesInstanceUID + ); + const selectedSet = sets.find( + set => set.SeriesInstanceUID === SeriesInstanceUID + ); + setState(state => ({ ...state, selectedSet, sets })); + } + }} + > + + + {isLoaded && + loadedSet.ROIContours.map(c => toContourItem(c, loadedSet))} + + + + ); + })} ); }; diff --git a/extensions/dicom-rt/src/tools/modules/rtStructModule.js b/extensions/dicom-rt/src/tools/modules/rtStructModule.js index e00ed1074..ccd94ac56 100644 --- a/extensions/dicom-rt/src/tools/modules/rtStructModule.js +++ b/extensions/dicom-rt/src/tools/modules/rtStructModule.js @@ -81,7 +81,9 @@ function _setStructureSetVisible(SeriesInstanceUID, visible = true) { const StructureSet = getStructureSet(SeriesInstanceUID); if (StructureSet) { - StructureSet.visible = visible; + StructureSet.ROIContours.forEach(ROIContour => { + ROIContour.visible = visible; + }); refreshViewport(); } @@ -154,15 +156,10 @@ function setToggleROIContour(SeriesInstanceUID, ROINumber) { * Returns an array of StructureSets which reference the given SeriesInstanceUID. * @param {string} SeriesInstanceUID The SeriesInstanceUID to check. */ -function getStructuresSetsWhichReferenceSeriesInstanceUid( - SeriesInstanceUID -) { +function getStructuresSetsWhichReferenceSeriesInstanceUid(SeriesInstanceUID) { const { StructureSets } = state; return StructureSets.filter(StructureSet => - structureSetReferencesSeriesInstanceUid( - StructureSet, - SeriesInstanceUID - ) + structureSetReferencesSeriesInstanceUid(StructureSet, SeriesInstanceUID) ); } diff --git a/extensions/dicom-segmentation/src/loadSegmentation.js b/extensions/dicom-segmentation/src/loadSegmentation.js index 388d562b6..052126180 100644 --- a/extensions/dicom-segmentation/src/loadSegmentation.js +++ b/extensions/dicom-segmentation/src/loadSegmentation.js @@ -41,6 +41,7 @@ export default async function loadSegmentation( results = _parseSeg(segArrayBuffer, imageIds); } catch (error) { segDisplaySet.isLoaded = false; + segDisplaySet.loadError = true; reject(error); } diff --git a/platform/core/src/classes/metadata/StudyMetadata.js b/platform/core/src/classes/metadata/StudyMetadata.js index a1550c504..459a0e441 100644 --- a/platform/core/src/classes/metadata/StudyMetadata.js +++ b/platform/core/src/classes/metadata/StudyMetadata.js @@ -135,6 +135,7 @@ export class StudyMetadata extends Metadata { study, sopClassUIDs ); + if (displaySet) { displaySet.sopClassModule = true; diff --git a/platform/core/src/utils/loadAndCacheDerivedDisplaySets.js b/platform/core/src/utils/loadAndCacheDerivedDisplaySets.js index 3ce929ca5..e11462c7b 100644 --- a/platform/core/src/utils/loadAndCacheDerivedDisplaySets.js +++ b/platform/core/src/utils/loadAndCacheDerivedDisplaySets.js @@ -82,12 +82,17 @@ const loadAndCacheDerivedDisplaySets = (referencedDisplaySet, studies) => { // For each type, see if any are loaded, if not load the most recent. Object.keys(displaySetsPerModality).forEach(key => { const displaySets = displaySetsPerModality[key]; + const isLoaded = displaySets.some(displaySet => displaySet.isLoaded); if (isLoaded) { return; } + if (displaySets.some(displaySet => displaySet.loadError)) { + return; + } + // find most recent and load it. let recentDateTime = 0; let recentDisplaySet = displaySets[0]; @@ -102,6 +107,8 @@ const loadAndCacheDerivedDisplaySets = (referencedDisplaySet, studies) => { } }); + recentDisplaySet.isLoading = true; + promises.push(recentDisplaySet.load(referencedDisplaySet, studies)); }); diff --git a/platform/viewer/cypress/integration/common/OHIFRoutes.spec.js b/platform/viewer/cypress/integration/common/OHIFRoutes.spec.js index b225d3c47..a7f590c72 100644 --- a/platform/viewer/cypress/integration/common/OHIFRoutes.spec.js +++ b/platform/viewer/cypress/integration/common/OHIFRoutes.spec.js @@ -5,17 +5,18 @@ describe('OHIF Routes', function() { cy.openStudyList(); }); - it('checks TEST json url study route', function() { - cy.visit( - '/viewer?url=https://ohif-viewer.s3.eu-central-1.amazonaws.com/JSON/testJSON.json' - ); + // TODO -> Bring back when testJSON is hosted again. + // it('checks TEST json url study route', function() { + // cy.visit( + // '/viewer?url=https://ohif-viewer.s3.eu-central-1.amazonaws.com/JSON/testJSON.json' + // ); - cy.server(); - cy.route('GET', '**/MRStudy/**').as('getTESTStudy'); + // cy.server(); + // cy.route('GET', '**/ MRStudy; /**').as('getTESTStudy'); - cy.wait('@getTESTStudy.all'); - cy.get('@getTESTStudy').should($route => { - expect($route.status).to.be.eq(200); - }); - }); + // cy.wait('@getTESTStudy.all'); + // cy.get('@getTESTStudy').should($route => { + // expect($route.status).to.be.eq(200); + // }); + // }); }); diff --git a/platform/viewer/cypress/integration/common/OHIFStudyList.spec.js b/platform/viewer/cypress/integration/common/OHIFStudyList.spec.js index c711bf874..685d27b28 100644 --- a/platform/viewer/cypress/integration/common/OHIFStudyList.spec.js +++ b/platform/viewer/cypress/integration/common/OHIFStudyList.spec.js @@ -38,12 +38,12 @@ describe('OHIF Study List', function() { }); it('searches Accession with exact string', function() { - cy.get('@AccessionNumber').type('fpcben98890'); + cy.get('@AccessionNumber').type('0000481914'); //Wait result list to be displayed cy.waitStudyList(); cy.get('@searchResult').should($list => { expect($list.length).to.be.eq(1); - expect($list).to.contain('fpcben98890'); + expect($list).to.contain('0000481914'); }); }); @@ -53,7 +53,7 @@ describe('OHIF Study List', function() { cy.waitStudyList(); cy.get('@searchResult').should($list => { // TODO: Why are we facing some inconsistency with this result? ¯\_(ツ)_/¯ - expect($list.length).to.be.eq(15); + expect($list.length).to.be.eq(9); expect($list).to.contain('MR'); }); }); @@ -149,18 +149,18 @@ describe('OHIF Study List', function() { cy.waitStudyList(); cy.get('@searchResult').should($list => { // TODO: Why are we facing some inconsistency with this result? ¯\_(ツ)_/¯ - expect($list.length).to.be.eq(15); + expect($list.length).to.be.eq(9); expect($list).to.contain('MR'); }); }); it('searches Accession with exact string', function() { - cy.get('@accessionModalityDescription').type('fpcben98890'); + cy.get('@accessionModalityDescription').type('0000481914'); //Wait result list to be displayed cy.waitStudyList(); cy.get('@searchResult').should($list => { expect($list.length).to.be.eq(1); - expect($list).to.contain('fpcben98890'); + expect($list).to.contain('0000481914'); }); }); diff --git a/platform/viewer/cypress/integration/visual-regression/PercyCheckOHIFRoutes.spec.js b/platform/viewer/cypress/integration/visual-regression/PercyCheckOHIFRoutes.spec.js index 088b23218..04a4f727d 100644 --- a/platform/viewer/cypress/integration/visual-regression/PercyCheckOHIFRoutes.spec.js +++ b/platform/viewer/cypress/integration/visual-regression/PercyCheckOHIFRoutes.spec.js @@ -5,13 +5,15 @@ describe('Visual Regression - OHIF Routes', function() { cy.openStudyList(); }); + /* + // TODO -> Bring back when testJSON is hosted again. it('checks TEST json url study route', function() { cy.visit( '/viewer?url=https://ohif-viewer.s3.eu-central-1.amazonaws.com/JSON/testJSON.json' ); cy.server(); - cy.route('GET', '**/TESTStudy/**').as('getTESTStudy'); + cy.route('GET', '**/ TESTStudy; /**').as('getTESTStudy'); cy.wait('@getTESTStudy.all'); cy.get('@getTESTStudy').should($route => { @@ -20,4 +22,5 @@ describe('Visual Regression - OHIF Routes', function() { cy.percyCanvasSnapshot('TEST json study route'); }); + */ }); diff --git a/platform/viewer/public/config/idc.js b/platform/viewer/public/config/idc.js index 204fa4d0e..a92516d12 100644 --- a/platform/viewer/public/config/idc.js +++ b/platform/viewer/public/config/idc.js @@ -9,11 +9,10 @@ window.config = function(props) { httpErrorHandler: error => { // This is 429 when rejected from the public idc sandbox too often. console.warn(error.status); - // Could use services manager here to bring up a dialog/modal if needed. console.warn('test, navigate to https://ohif.org/'); window.location = 'https://ohif.org/'; }, - healthcareApiEndpoint: 'https://idc-sandbox-002.appspot.com/v1beta1', + healthcareApiEndpoint: 'https://proxy-dot-idc-dev.appspot.com/v1beta1', }; };