[WIP] feat: Save DICOM SR to the same series

[wayfarer3130]

Context

It becomes very confusing to have a lot of series all with the same series description, but differing date/times and content.
This PR changes the save so items get saved to the same series.

Changes & Results

Testing

Checklist

PR

• [] My Pull Request title is descriptive, accurate and follows the
  semantic-release format and guidelines.

Code

• [] My code has been well-documented (function documentation, inline comments,
  etc.)

Public Documentation Updates

• [] The documentation page has been updated as necessary for any public API
  additions or removals.

Tested Environment

• [] "OS:
• [] "Node version:
• [] "Browser:

[netlify]

✅ Deploy Preview for ohif-platform-docs ready!

Name	Link
🔨 Latest commit	afcc6d5
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/66705cf913884200086c502d
😎 Deploy Preview	https://deploy-preview-3273--ohif-platform-docs.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 configuration.

[netlify]

❌ Deploy Preview for ohif-platform-viewer failed.

Name	Link
🔨 Latest commit	a6b93e5
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-viewer/deploys/64959c332747ab0008bad87d

[codecov]

Codecov Report

Merging #3273 (b995a78) into master (72207e6) will decrease coverage by 5.59%.
Report is 2 commits behind head on master.
The diff coverage is 0.00%.

❗ Current head b995a78 differs from pull request most recent head 44dc3ba. Consider uploading reports for the commit 44dc3ba to get more accurate results

@@            Coverage Diff             @@
##           master    #3273      +/-   ##
==========================================
- Coverage   42.93%   37.34%   -5.59%     
==========================================
  Files          80       85       +5     
  Lines        1444     1403      -41     
  Branches      338      310      -28     
==========================================
- Hits          620      524      -96     
- Misses        661      707      +46     
- Partials      163      172       +9     

Files Changed	Coverage Δ
platform/core/src/classes/MetadataProvider.js	4.16% <0.00%> (+0.08%)	⬆️

... and 31 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

[cypress]

Passing run #4085 ↗︎

0

44

2

0

0

Details:

Add missing variable
Project: Viewers	Commit: afcc6d5df8
Status: Passed	Duration: 05:30 💡
Started: Jun 17, 2024 4:10 PM	Ended: Jun 17, 2024 4:16 PM

Review all test suite changes for PR #3273 ↗︎

[fedorov]

@wayfarer3130 note that it is not the right DICOM practice to overwrite existing content. What if the prior version of the SR has already been used somewhere, is referenced from somewhere, or is indexed somewhere?

I understand sometime some deviation and compromises are unavoidable, but at the very least it should be possible to disable this feature, and it should be communicated to the user that an existing instance is being overwritten.

In general, I believe when using SR the idea is to use CompletionFlag attribute, in conjunction with references to prior evidence to track the chain of edits. I understand that the large number of reports can be overwhelming, but perhaps you can handle this in the UI by tracking the chain of edits and allowing to disable visualization of prior "versions".

If you have not yet read the section "Copies and Versions" of @dclunie "DICOM Structured Reporting" book, you are very strongly encouraged to do so!

[pieper]

@fedorov from a quick look I think @wayfarer3130 is not overwriting the SR instance, but adding the new instance to the same series as existing SRs. I don't know if this is allowed in the standard (not sure why it wouldn't be) but if there's a chain of edits of the "same" SR then to me it sounds good to put them in the same series.

[wayfarer3130]

@fedorov from a quick look I think @wayfarer3130 is not overwriting the SR instance, but adding the new instance to the same series as existing SRs. I don't know if this is allowed in the standard (not sure why it wouldn't be) but if there's a chain of edits of the "same" SR then to me it sounds good to put them in the same series.

@fedorov - yes, I'm adding to the existing series. Ideally adding a deprecation reference to the earlier instances, but that isn't something I've implemented yet (I have to look up in the standard what the tags are for that - I've NEVER seen them used, but have seen them in the standard).

[dclunie]

"adding the new instance to the same series as existing SRs. I don't know if this is allowed in the standard (not sure why it wouldn't be)" - criteria for same Series are defined in the standard here - same equipment may be a barrier here, since arguably two deployed instances of the same viewer used on different computers at different times are not really "the same equipment".

[fedorov]

@wayfarer3130 would you be interested to get together to discuss this feature together with the members of the Imaging Data Commons team (David Clunie, Steve Pieper and myself)? We would be interested in such a discussion.

[netlify]

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	afcc6d5
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/66705cf97446690008ffc313

[sedghi]

@wayfarer3130 Could you please revisit this after the new replies ? Thanks