feat: abstract routing configuration and URI parameter access (#48)

src/App.tsx

@@ -28,6 +28,7 @@ import NotificationMiddleware, {
   NotificationMiddlewareContext,
 } from './services/NotificationMiddleware'
 import { CustomError, errorTypes } from './utils/CustomError'
+import { getProjectStorePath, isProjectsPath, RoutePaths } from './utils/routes'
 import { joinUrl, normalizeServerUrl } from './utils/url'
 
 function ParametrizedCaseViewer({
@@ -105,8 +106,8 @@ function _createClientMapping({
         }
       })
     } else {
-      if (window.location.pathname.includes('/projects/')) {
-        const pathname = window.location.pathname.split('/study/')[0]
+      if (isProjectsPath(window.location.pathname)) {
+        const pathname = getProjectStorePath(window.location.pathname)
         const pathUrl = `${gcpBaseUrl}${pathname}/dicomWeb`
         serverSettings.url = pathUrl
       }
@@ -564,7 +565,7 @@ class App extends React.Component<AppProps, AppState> {
         <BrowserRouter basename={this.props.config.path}>
           <Routes>
             <Route
-              path="/"
+              path={RoutePaths.ROOT}
               element={
                 <AppShell enableMemoryMonitoring={enableMemoryMonitoring}>
                   <Layout style={layoutStyle}>
@@ -586,7 +587,7 @@ class App extends React.Component<AppProps, AppState> {
               }
             />
             <Route
-              path="/studies/:studyInstanceUID/*"
+              path={RoutePaths.STUDY}
               element={
                 <SettingsProvider>
                   <AppShell enableMemoryMonitoring={enableMemoryMonitoring}>
@@ -615,7 +616,7 @@ class App extends React.Component<AppProps, AppState> {
               }
             />
             <Route
-              path="/projects/:project/locations/:location/datasets/:dataset/dicomStores/:dicomStore/study/:studyInstanceUID/*"
+              path={RoutePaths.GCP_STUDY}
               element={
                 <SettingsProvider>
                   <AppShell enableMemoryMonitoring={enableMemoryMonitoring}>
@@ -644,7 +645,7 @@ class App extends React.Component<AppProps, AppState> {
               }
             />
             <Route
-              path="/logout"
+              path={RoutePaths.LOGOUT}
               element={
                 <AppShell enableMemoryMonitoring={enableMemoryMonitoring}>
                   <Layout style={layoutStyle}>

src/auth/OidcManager.tsx

@@ -5,6 +5,7 @@ import NotificationMiddleware, {
   NotificationMiddlewareContext,
 } from '../services/NotificationMiddleware'
 import { CustomError, errorTypes } from '../utils/CustomError'
+import { buildLogoutPath } from '../utils/routes'
 import { isAuthorizationCodeInUrl } from '../utils/url'
 import type { AuthManager, SignInCallback, User } from '.'
 
@@ -63,7 +64,7 @@ export default class OidcManager implements AuthManager {
       loadUserInfo: true,
       automaticSilentRenew: true,
       revokeAccessTokenOnSignout: true,
-      post_logout_redirect_uri: `${baseUri}/logout`,
+      post_logout_redirect_uri: buildLogoutPath(baseUri),
     })
     if (
       settings.endSessionEndpoint !== null &&
@@ -95,7 +96,7 @@ export default class OidcManager implements AuthManager {
               loadUserInfo: true,
               automaticSilentRenew: true,
               revokeAccessTokenOnSignout: true,
-              post_logout_redirect_uri: `${baseUri}/logout`,
+              post_logout_redirect_uri: buildLogoutPath(baseUri),
               metadata,
             })
           }

src/components/CaseViewer.tsx

@@ -22,9 +22,18 @@ import {
   seriesUidFromSlide,
 } from '../utils/recoverSeriesInstanceUID'
 import { type RouteComponentProps, withRouter } from '../utils/router'
+import {
+  buildSeriesPath,
+  hasSeriesInPath,
+  isProjectsPath,
+  parseSeriesInstanceUID,
+  RoutePaths,
+  withSeriesInProjectPath,
+} from '../utils/routes'
 import ClinicalTrial from './ClinicalTrial'
 import Patient from './Patient'
 import SlideList from './SlideList'
+// skipcq: JS-W1028 - SlideViewer has a default export
 import SlideViewer from './SlideViewer'
 import Study from './Study'
 
@@ -192,6 +201,7 @@ function ParametrizedSlideViewer({
         }
       }
 
+      // skipcq: JS-0098 - void operator intentionally discards the Promise
       void findReferencedSlide()
     }
   }, [
@@ -262,22 +272,14 @@ function Viewer(props: ViewerProps): JSX.Element | null {
     seriesInstanceUID: string
   }): void => {
     console.info(`switch to series "${seriesInstanceUID}"`)
-    let urlPath = `/studies/${studyInstanceUID}/series/${seriesInstanceUID}`
+    let urlPath = buildSeriesPath(studyInstanceUID, seriesInstanceUID)
 
-    if (location.pathname.includes('/projects/')) {
-      urlPath = location.pathname
-      if (!location.pathname.includes('/series/')) {
-        urlPath += `/series/${seriesInstanceUID}`
-      } else {
-        urlPath = urlPath.replace(
-          /\/series\/[^/]+/,
-          `/series/${seriesInstanceUID}`,
-        )
-      }
+    if (isProjectsPath(location.pathname)) {
+      urlPath = withSeriesInProjectPath(location.pathname, seriesInstanceUID)
     }
 
     if (
-      location.pathname.includes('/series/') &&
+      hasSeriesInPath(location.pathname) &&
       location.search !== null &&
       location.search !== undefined
     ) {
@@ -307,11 +309,8 @@ function Viewer(props: ViewerProps): JSX.Element | null {
    * the first slide contained in the study.
    */
   let selectedSeriesInstanceUID: string
-  if (location.pathname.includes('series/')) {
-    const seriesFragment = location.pathname.split('series/')[1]
-    const seriesFromPath = seriesFragment.includes('/')
-      ? seriesFragment.split('/')[0]
-      : seriesFragment
+  const seriesFromPath = parseSeriesInstanceUID(location.pathname)
+  if (seriesFromPath !== '') {
     const slideForPath = findSeriesSlide(slides, seriesFromPath)
     selectedSeriesInstanceUID =
       slideForPath !== undefined
@@ -399,7 +398,7 @@ function Viewer(props: ViewerProps): JSX.Element | null {
 
       <Routes>
         <Route
-          path="/series/:seriesInstanceUID"
+          path={RoutePaths.SERIES}
           element={
             <ParametrizedSlideViewer
               clients={props.clients}

src/components/Header.tsx

@@ -35,6 +35,11 @@ import NotificationMiddleware, {
 } from '../services/NotificationMiddleware'
 import type { CustomError } from '../utils/CustomError'
 import { type RouteComponentProps, withRouter } from '../utils/router'
+import {
+  isGcpDicomStorePath,
+  isViewerPath,
+  parseSeriesInstanceUID,
+} from '../utils/routes'
 import { normalizeServerUrl } from '../utils/url'
 import Button from './Button'
 import DicomTagBrowser from './DicomTagBrowser/DicomTagBrowser'
@@ -44,8 +49,6 @@ const aboutModalCopyTooltips: [React.ReactNode, React.ReactNode] = [
   'Copied!',
 ]
 
-const DICOM_TAG_BROWSER_PATHS = ['/studies/', '/study/', '/projects/'] as const
-
 const aboutModalStyles: Record<string, React.CSSProperties> = {
   container: {
     textAlign: 'center',
@@ -335,12 +338,7 @@ class Header extends React.Component<HeaderProps, HeaderState> {
       }
     }
     const pathNorm = trimmedUrl.startsWith('/') ? trimmedUrl : `/${trimmedUrl}`
-    return (
-      pathNorm.includes('/projects/') &&
-      pathNorm.includes('/locations/') &&
-      pathNorm.includes('/datasets/') &&
-      pathNorm.includes('/dicomStores/')
-    )
+    return isGcpDicomStorePath(pathNorm)
   }
 
   static handleUserMenuButtonClick(e: React.SyntheticEvent): void {
@@ -489,13 +487,9 @@ class Header extends React.Component<HeaderProps, HeaderState> {
   handleDicomTagBrowserButtonClick = (): void => {
     const width = window.innerWidth - 200
 
-    let seriesInstanceUID = ''
-    if (this.props.location.pathname.includes('series/')) {
-      const seriesFragment = this.props.location.pathname.split('series/')[1]
-      seriesInstanceUID = seriesFragment.includes('/')
-        ? seriesFragment.split('/')[0]
-        : seriesFragment
-    }
+    const seriesInstanceUID = parseSeriesInstanceUID(
+      this.props.location.pathname,
+    )
 
     Modal.info({
       title: 'DICOM Tag Browser',
@@ -753,9 +747,7 @@ class Header extends React.Component<HeaderProps, HeaderState> {
       </HeaderCountBadge>
     )
 
-    const showDicomTagBrowser = DICOM_TAG_BROWSER_PATHS.some((path) =>
-      this.props.location.pathname.includes(path),
-    )
+    const showDicomTagBrowser = isViewerPath(this.props.location.pathname)
 
     const dicomTagBrowserButton = showDicomTagBrowser ? (
       <Button

src/components/Worklist.tsx

@@ -17,6 +17,7 @@ import NotificationMiddleware, {
 import { CustomError, errorTypes } from '../utils/CustomError'
 import { logger } from '../utils/logger'
 import { type RouteComponentProps, withRouter } from '../utils/router'
+import { buildStudyPath } from '../utils/routes'
 import { parseDate, parseName, parseSex, parseTime } from '../utils/values'
 import { SlimSpinner } from './AppLoading'
 
@@ -152,18 +153,15 @@ class Worklist extends React.Component<WorklistProps, WorklistState> {
     window.addEventListener('resize', this.updateTableScrollY)
   }
 
-  componentDidUpdate(
-    previousProps: WorklistProps,
-    previousState: WorklistState,
-  ): void {
-    if (this.props.clients !== previousProps.clients) {
+  componentDidUpdate(prevProps: WorklistProps, prevState: WorklistState): void {
+    if (this.props.clients !== prevProps.clients) {
       this.searchForStudies()
     }
     // Pagination bar can appear/hide (hideOnSinglePage); remeasure the table pane.
     if (
-      previousState.numStudies !== this.state.numStudies ||
-      previousState.pageSize !== this.state.pageSize ||
-      previousState.isLoading !== this.state.isLoading
+      prevState.numStudies !== this.state.numStudies ||
+      prevState.pageSize !== this.state.pageSize ||
+      prevState.isLoading !== this.state.isLoading
     ) {
       this.updateTableScrollY()
     }
@@ -221,7 +219,7 @@ class Worklist extends React.Component<WorklistProps, WorklistState> {
     _event: React.SyntheticEvent,
     study: dmv.metadata.Study,
   ): void => {
-    this.props.navigate(`/studies/${study.StudyInstanceUID}`)
+    this.props.navigate(buildStudyPath(study.StudyInstanceUID))
   }
 
   private async collectModalitiesFromSeries(

src/utils/__tests__/routes.test.ts

@@ -0,0 +1,119 @@
+import {
+  buildLogoutPath,
+  buildSeriesPath,
+  buildStudyPath,
+  getProjectStorePath,
+  hasSeriesInPath,
+  isGcpDicomStorePath,
+  isProjectsPath,
+  isViewerPath,
+  parseSeriesInstanceUID,
+  RouteParams,
+  RoutePaths,
+  withSeriesInProjectPath,
+} from '../routes'
+
+const studyUID = '1.2.3'
+const seriesUID = '4.5.6'
+const otherSeriesUID = '7.8.9'
+
+const gcpStorePath =
+  '/projects/idc-sandbox-000/locations/us-central1/datasets/dev/dicomStores/store'
+
+describe('route templates', () => {
+  it('embed the configured parameter names', () => {
+    expect(RoutePaths.STUDY).toBe(`/studies/:${RouteParams.STUDY_INSTANCE_UID}/*`)
+    expect(RoutePaths.SERIES).toBe(
+      `/series/:${RouteParams.SERIES_INSTANCE_UID}`,
+    )
+    expect(RoutePaths.GCP_STUDY).toBe(
+      '/projects/:project/locations/:location/datasets/:dataset/dicomStores/:dicomStore/study/:studyInstanceUID/*',
+    )
+    expect(RoutePaths.ROOT).toBe('/')
+    expect(RoutePaths.LOGOUT).toBe('/logout')
+  })
+})
+
+describe('path builders', () => {
+  it('builds study paths', () => {
+    expect(buildStudyPath(studyUID)).toBe('/studies/1.2.3')
+  })
+
+  it('builds series paths', () => {
+    expect(buildSeriesPath(studyUID, seriesUID)).toBe(
+      '/studies/1.2.3/series/4.5.6',
+    )
+  })
+
+  it('builds logout paths', () => {
+    expect(buildLogoutPath('https://example.org')).toBe(
+      'https://example.org/logout',
+    )
+  })
+})
+
+describe('parseSeriesInstanceUID', () => {
+  it('returns the series UID at the end of a path', () => {
+    expect(parseSeriesInstanceUID('/studies/1.2.3/series/4.5.6')).toBe(seriesUID)
+  })
+
+  it('returns the series UID followed by further segments', () => {
+    expect(parseSeriesInstanceUID('/studies/1.2.3/series/4.5.6/extra')).toBe(
+      seriesUID,
+    )
+  })
+
+  it('returns an empty string when no series is present', () => {
+    expect(parseSeriesInstanceUID('/studies/1.2.3')).toBe('')
+  })
+})
+
+describe('path predicates', () => {
+  it('detects series in a path', () => {
+    expect(hasSeriesInPath('/studies/1.2.3/series/4.5.6')).toBe(true)
+    expect(hasSeriesInPath('/studies/1.2.3')).toBe(false)
+  })
+
+  it('detects GCP project paths', () => {
+    expect(isProjectsPath(`${gcpStorePath}/study/1.2.3`)).toBe(true)
+    expect(isProjectsPath('/studies/1.2.3')).toBe(false)
+  })
+
+  it('detects viewer paths', () => {
+    expect(isViewerPath('/studies/1.2.3')).toBe(true)
+    expect(isViewerPath(`${gcpStorePath}/study/1.2.3`)).toBe(true)
+    expect(isViewerPath('/')).toBe(false)
+  })
+
+  it('detects GCP DICOM store paths', () => {
+    expect(isGcpDicomStorePath(gcpStorePath)).toBe(true)
+    expect(isGcpDicomStorePath('/projects/foo')).toBe(false)
+  })
+})
+
+describe('getProjectStorePath', () => {
+  it('returns the store path up to the study segment', () => {
+    expect(getProjectStorePath(`${gcpStorePath}/study/1.2.3`)).toBe(gcpStorePath)
+  })
+
+  it('returns the full pathname when /study/ is not present', () => {
+    expect(getProjectStorePath('/studies/1.2.3')).toBe('/studies/1.2.3')
+  })
+})
+
+describe('withSeriesInProjectPath', () => {
+  it('appends a series segment when none is present', () => {
+    expect(
+      withSeriesInProjectPath(`${gcpStorePath}/study/1.2.3`, seriesUID),
+    ).toBe(`${gcpStorePath}/study/1.2.3/series/4.5.6`)
+  })
+
+  it('replaces an existing series segment', () => {
+    expect(
+      withSeriesInProjectPath(
+        `${gcpStorePath}/study/1.2.3/series/${otherSeriesUID}`,
+        seriesUID,
+      ),
+    ).toBe(`${gcpStorePath}/study/1.2.3/series/4.5.6`)
+  })
+})