search

OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.38k stars 3.41k forks source link

[Bug] getHangingProtocolModule Name or Id ? #3755

Open salimkanoun opened 1 year ago

salimkanoun commented 1 year ago

Describe the Bug

It seems to be a discordance between documentation and code

in the documentation : https://docs.ohif.org/platform/services/data/hangingprotocolservice/#protocol-definition Hanging protocol should be registered by Id

But in the code they are registered by name property : https://github.com/OHIF/Viewers/blob/c73a403cdcbc687981308d03462d2d2d10f73df5/extensions/default/src/getHangingProtocolModule.js#L101

Also the url param to set a HP is hangingProtocolId so I guess there is a error in the code (should use id instead of name, and will led to breaking change)

Steps to Reproduce

None

The current behavior

registering of HP use name

The expected behavior

should use id property

OS

linux

Node version

20

Browser

Chrome 100+

sedghi commented 7 months ago

I think id makes more sense, you are saying URL is based on name?

salimkanoun commented 7 months ago

The weird thing is that getHangingProtocolModule is mapping the id of the hanging protocol to a "name" property

The HP object has itself a "name" and an "id" property, so I guess getHangingProtocolModule could simply return array of protocols. If needed to add a differentiating key then I guess it would be better to expose "id" instead of "name" in this getHangingProtocolModule