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.

Sign up for GitHub

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

Jump to bottom

Pacs progress websockets #1258

Merged
merged 49 commits into from
Oct 18, 2024
Merged

Pacs progress websockets #1258

merged 49 commits into from
Oct 18, 2024

Conversation

jennydaman
Copy link
Collaborator

@jennydaman jennydaman commented Sep 25, 2024 •
edited
Loading

This PR replaces the old PACS Q/R UI with a new one, redesigned from the ground up using antd and tanstack/query.
It implements the client-side of LONK-WS.

Additions

  • Mobile-friendly UI
  • Removed "PatientCard" component. If query results in a single study, it is expanded by default. There are fewer clicks needed to see the data and request a PACS pull
  • Search query is stored in URI search params (and is not base64 encoded)
  • Replaced PFDCM client with an OpenAPI generated one
  • installed @testing-library/react and configured it to work with vitest
  • WebSocket communication with CUBE over the LONK-WS endpoint
  • more...

Missing Features

The redesign is feature-incomplete.

  • No button to preview series, go to library, create feed, ...

@jennydaman
Add LonkClient
5dc1248
@jennydaman
Add new fp-ts PfdcmClient
288aea3
@jennydaman
Begin rewriting Pacs
86e791c
@jennydaman
Fix font styling conflict by antd App component
5991eda
@jennydaman
Change LonkClient to accept handlers via init
0419bb7
@jennydaman
Merge staging (switch from npm to pnpm)
f71c38c
@jennydaman
Migrate .../Pacs/components/input.tsx to antd
c0556a5 
And also...

1. PfdcmClient.getPacsServices returns ReadonlyNonEmptyArray
2. Use useSearchParams instead of React.useState
@jennydaman
Merge remote-tracking branch 'upstream/staging' into pacs-progress-we…
d2074e2 
…bsockets
@jennydaman
LonkClient.unsubscribeAll
707d10e
@jennydaman
Merge staging #1249
0c5b27d
@jennydaman
Reorganize Pacs to be MVC-ish
2a72864
@jennydaman
Add helpers for testing components which use redux
2ad8b55
@jennydaman
Merge staging #1250
75ed5fa
@jennydaman
Implement PACS query
5e3fd36
@jennydaman
Implement PACS study details
6bc5f3e
@jennydaman
Replace Radio.Group with Segmented
8628407
@jennydaman
Change Pacs studies to use Collapse instead of List
7c530d5
@jennydaman
Add AccessionNumber to study title
e2cdc0e
@jennydaman
SeriesList
66a0f0b
@jennydaman
Merge remote-tracking branch 'upstream/staging' into pacs-progress-we…
a20b1d0 
…bsockets
@jennydaman
Remove redux from Pacs code
5b3a2c4
@jennydaman
WIP
c4a2b39
@jennydaman
ReceiveState
5bb10d8
@jennydaman
Fix pfdcm tests since removal of fp-ts monads
f3057a3
@jennydaman
useLonk
02d7e39
@jennydaman
Rename things + more documentation
1a66b2a
@jennydaman
Merge staging
e39a50a
@jennydaman
PACS pull is working, progress messages aren't though.
0811d4e
@jennydaman
Pull study works, except for the last part.
76c5a75
@jennydaman jennydaman mentioned this pull request Sep 25, 2024
Closed
jennydaman added a commit to FNNDSC/miniChRIS-docker that referenced this pull request Sep 25, 2024
@jennydaman
Change ChRIS_ui image to the build of FNNDSC/ChRIS_ui#1258
f0c224b
@jennydaman
Everything working! I think...
ad7fba4
@jennydaman
Customize progress bar theme
a74ae25
@jennydaman
Implement series pull
2df2851
@jennydaman
Fix crash on invalid StudyDate
e8ddeef
@jennydaman
Add test stub
27db1fc
@jennydaman
Merge remote-tracking branch 'upstream/staging' into pacs-progress-we…
9e4a713 
…bsockets
@jennydaman
Hacky workaround for double firing of pull request
37f0839
@jennydaman
Revert "Hacky workaround for double firing of pull request"
08e5280 
This reverts commit 37f0839.
@jennydaman
Small changes
ef2cc3a
@jennydaman
pnpm run fmt
e174158
@jennydaman
Change type of pullRequests from ReadonlyArray to PullRequestStates
8e169bc
@jennydaman
terribleStrictModeWorkaround
f7cf25a
@jennydaman
Fix all lints
5e8505e 
Note: lint/complexity/noUselessFragments is disabled because of biomejs/biome#4247

Also follow biomejs/biome#4248
@jennydaman
Fix some pypx types
43d9fd0
@jennydaman
Automatically expand if only one study found
d0d2bd0
@jennydaman
Fix infinite useEffects calling lonk.subscribe
8a8aedb
@jennydaman
Add tests
c0001cd
@jennydaman jennydaman marked this pull request as ready for review October 11, 2024 03:33
@PintoGideon
Copy link
Collaborator

PintoGideon commented Oct 18, 2024 •
edited
Loading

Test Cases Covered:

  • Pulling a study with 15 series
  • Pulling multiple studies
  • Pulling individual series

Current Feedback:

  • No feedback on the code at this time.

UI Observation:

  • A small UI nit found while testing in light mode: The modality label is not visible in light mode.

Screenshot from 2024-10-18 11-12-45
Screenshot from 2024-10-18 11-12-53

PintoGideon
PintoGideon approved these changes Oct 18, 2024
View reviewed changes
Copy link
Collaborator

@PintoGideon PintoGideon left a comment

Choose a reason for hiding this comment

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

This is inspiring work. Thank you for your hard work and effort

@PintoGideon PintoGideon merged commit b911366 into staging Oct 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PintoGideon PintoGideon PintoGideon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jennydaman @PintoGideon