Skip to content

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

Open
igoroctaviano wants to merge 15 commits into
masterfrom
feat/48-abstract-routing
Open

feat: abstract routing configuration and URI parameter access (#48)#392
igoroctaviano wants to merge 15 commits into
masterfrom
feat/48-abstract-routing

Conversation

@igoroctaviano

@igoroctaviano igoroctaviano commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves #48. The URI template /studies/:studyInstanceUID/series/:seriesInstanceUID (and its GCP DICOM store variant) was assumed/hardcoded in several places and re-parsed by hand, making routing changes error-prone.

This introduces src/utils/routes.ts as a single source of truth for:

  • Route templates (RoutePaths) consumed by react-router <Route path> definitions, with parameter names centralized in RouteParams.
  • Path builders: buildStudyPath, buildSeriesPath, buildLogoutPath.
  • Parsers / predicates: parseSeriesInstanceUID, hasSeriesInPath, isProjectsPath, getProjectStorePath, withSeriesInProjectPath, isViewerPath, isGcpDicomStorePath.

Refactored to use the abstraction

  • App.tsx — <Route> paths (ROOT, STUDY, GCP_STUDY, LOGOUT) and GCP client base-URL derivation.
  • components/CaseViewer.tsx — series-selection URL building, series parsing, and nested SERIES route (removed ad-hoc interpolation + regex).
  • components/Worklist.tsx — study navigation.
  • components/Header.tsx — DICOM Tag Browser visibility, series parsing, and GCP server-URL validation (removed the local DICOM_TAG_BROWSER_PATHS constant).
  • auth/OidcManager.tsx — post_logout_redirect_uri.

No behavior change — paths produced/parsed are identical to before; the logic is just centralized.

Test plan

  • pnpm run typecheck passes
  • pnpm run lint (biome) passes
  • pnpm test — all 53 tests pass, including new src/utils/__tests__/routes.test.ts (22 assertions covering builders, parsers, and predicates)
  • Manual: open a study from the worklist, switch series, open DICOM Tag Browser, and verify GCP projects/.../dicomStores/... deep links still route correctly

@igoroctaviano
feat: abstract routing configuration and URI parameter access (#48)
2b65892 
Introduce src/utils/routes.ts as a single source of truth for route
templates, path builders, and URI parameter parsing. Previously the
/studies/:studyInstanceUID/series/:seriesInstanceUID template (and its
GCP DICOM store variant) was hardcoded and re-parsed by hand across
App, CaseViewer, Worklist, Header, and OidcManager, making routing
changes error-prone.

Components now consume RoutePaths for <Route> definitions and the
buildStudyPath/buildSeriesPath/buildLogoutPath builders plus
parse/predicate helpers instead of ad-hoc string interpolation,
split(), includes(), and regex replacement.

Add unit tests for the new routing helpers.

Closes #48
@sonarqubecloud

sonarqubecloud Bot commented Jun 22, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@deepsource-io

deepsource-io Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in f726405...5c06b10 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Aug 14, 2026 4:43p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@github-actions

github-actions Bot commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 5c06b10):

https://idc-external-006--pr392-feat-48-abstract-rou-0r7miscb.web.app

(expires Fri, 21 Aug 2026 16:45:02 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307

@igoroctaviano
igoroctaviano requested a review from fedorov August 7, 2026 17:53
igoroctaviano and others added 8 commits August 11, 2026 17:15
@igoroctaviano
Merge remote-tracking branch 'origin/master' into feat/48-abstract-ro…
1ab3bd0 
…uting

# Conflicts:
#	src/App.tsx
#	src/components/CaseViewer.tsx
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
5a078c3
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
c00bd4b
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
359d79e
@igoroctaviano @claude
refactor(routes): address code review feedback
c6e96a2 
- Add comments explaining SERIES_PREFIX vs SERIES_PATH_PREFIX distinction
- Document getProjectStorePath behavior when /study/ is not present
- Remove unnecessary export from DICOM_TAG_BROWSER_PATHS (only used internally)
- Add edge case test for getProjectStorePath with non-GCP paths

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
76be8be
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
4d31c0a
@igoroctaviano
Merge branch 'master' into feat/48-abstract-routing
9642041
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:
Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:
Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

@igoroctaviano @claude
fix: resolve DeepSource code quality issues
c20b09f 
- Add skipcq for valid patterns (SlideViewer import, void operator)
- Rename lifecycle parameters to prevProps/prevState (Worklist)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:
Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

@sonarqubecloud

sonarqubecloud Bot commented Aug 14, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:
Version ^0.48.23

To link a DMV branch for testing (requires an open PR in DMV):

  • Add dmv-branch: <branch-name> to the PR description, OR
  • Use the same branch name in both repos (automatic matching)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fedorov fedorov Awaiting requested review from fedorov

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Abstract routing configuration and access to URI parameters

1 participant

@igoroctaviano