Add support for array input for all constraints in HangingProtocolService validator #3322

Sofien-Sellami · 2023-04-12T16:34:43Z

Context

This PR updates the validator.js file in the Hanging Protocol Service library to allow constraints to accept an array of values. Previously, only the contains constraint could accept an array.

Changes & Results

-Updated functions to accept an array of values.
-Made changes to ensure all constraints accept an array of values for consistency.

Constraint Rules

Rule	Single Value	Array Value	Example
equals	===	All members are === in same order	value = ['abc', 'def', 'GHI'] testValue = 'abc' (Fail) = ['abc'] (Fail) = ['abc', 'def', 'GHI'] (Valid) = ['abc', 'GHI', 'def'] (Fail) = ['abc', 'def'] (Fail) value = 'Attenuation Corrected' testValue = 'Attenuation Corrected' (Valid) = 'Attenuation' (Fail) value = ['Attenuation Corrected'] testValue = ['Attenuation Corrected'] (Valid) = 'Attenuation Corrected' (Valid) = 'Attenuation' (Fail)
doesNotEqual	!==	Any member is !== for the array, either in value, order, or length	value = ['abc', 'def', 'GHI'] testValue = 'abc' (Valid) = ['abc'] (Valid) = ['abc', 'def', 'GHI'] (Fail) = ['abc', 'GHI', 'def'] (Valid) = ['abc', 'def'] (Valid) value = 'Attenuation Corrected' testValue = 'Attenuation Corrected' (Fail) Valid = 'Attenuation' (Valid) value = ['Attenuation Corrected'] testValue = ['Attenuation Corrected'] (Fail) = 'Attenuation Corrected' (Fail) = 'Attenuation' (Fail)
includes	Not allowed	Value is equal to one of the values of the array	value = ['abc', 'def', 'GHI'] testValue = ['abc'] (Valid) = ‘abc’ (Fail) = [‘abc’] (Fail) = ‘dog’ (Fail) = = [‘att’, ‘abc’] (Valid) = ['abc', 'def', 'dog'] (Valid) = ['cat', 'dog'] (Fail) value = 'Attenuation Corrected' testValue = ['Attenuation Corrected', 'Corrected'] (Valid) = ['Attenuation', 'Corrected'] (Fail) value = ['Attenuation Corrected'] testValue = 'Attenuation Corrected' (Fail) = ['Attenuation Corrected', 'Corrected'] (Valid) = ['Attenuation', 'Corrected'] (Fail)
doesNotInclude	Not allowed	Value is not in one of the values of the array	value = ['abc', 'def', 'GHI'] testValue = ‘Corr’ (Valid) = ‘abc’ (Fail) = [‘att’, ‘cor’] (Valid) = ['abc', 'def', 'dog'] (Fail) value = 'Attenuation Corrected' testValue = ['Attenuation Corrected', 'Corrected'] (Fail) = ['Attenuation', 'Corrected'] (Valid) value = ['Attenuation Corrected'] testValue = 'Attenuation' (Fail) = ['Attenuation Corrected', 'Corrected'] (Fail) = ['Attenuation', 'Corrected'] (Valid)
containsI	String containment (case insensitive)	String containment (case insensitive) is OK for one of the rule values	value = 'Attenuation Corrected' testValue = ‘Corr’ (Valid) = ‘corr’ (Valid) = [‘att’, ‘cor’] (Valid) = [‘Att’, ‘Wall’] (Valid) = [‘cat’, ‘dog’] (Fail) value = ['abc', 'def', 'GHI'] testValue = 'def' (Valid) = 'dog' (Fail) = ['gh', 'de'] (Valid) = ['cat', 'dog'] (Fail)
contains	String containment (case sensitive)	String containment (case sensitive) is OK for one of the rule values	value = 'Attenuation Corrected' testValue = ‘Corr’ (Valid) = ‘corr’ (Fail) = [‘att’, ‘cor’] (Fail) = [‘Att’, ‘Wall’] (Valid) = [‘cat’, ‘dog’] (Fail) value = ['abc', 'def', 'GHI'] testValue = 'def' (Valid) = 'dog' (Fail) = ['cat', 'de'] (Valid) = ['cat', 'dog'] (Fail)
doesNotContain	String containment is false	String containment is false for all values of the array	value = 'Attenuation Corrected' testValue = ‘Corr’ (Fail) = ‘corr’ (Valid) = [‘att’, ‘cor’] (Valid) = [‘Att’, ‘Wall’] (Fail) = [‘cat’, ‘dog’] (Valid) value = ['abc', 'def', 'GHI'] testValue = 'def' (Fail) = 'dog' (Valid) = ['cat', 'de'] (Fail) = ['cat', 'dog'] (Valid)
doesNotContainI	String containment is false (case insensitive)	String containment (case insensitive) is false for all values of the array	value = 'Attenuation Corrected' testValue = ‘Corr’ (Fail) = ‘corr’ (Fail) = [‘att’, ‘cor’] (Fail) = [‘Att’, ‘Wall’] (Fail) = [‘cat’, ‘dog’] (Valid) value = ['abc', 'def', 'GHI'] testValue = 'DEF' (Fail) = 'dog' (Valid) = ['cat', 'gh'] (Fail) = ['cat', 'dog'] (Valid)
startsWith	Value begins with characters	Starts with one of the values of the array	value = 'Attenuation Corrected' testValue = ‘Corr’ (Fail) = ‘Att’ (Fail) = ['cat', 'dog', 'Att'] (Valid) = [‘cat’, ‘dog’] (Fail)
endsWith	Value ends with characters	ends with one of the value of the array	value = 'Attenuation Corrected' testValue = ‘TED’ (Fail) = ‘ted’ (Valid) = ['cat', 'dog', 'ted'] (Valid) = [‘cat’, ‘dog’] (Fail)
greaterThan	value is => to rule	Not applicable	value = 30 testValue = 20 (Valid) = 40 (Fail)
lessThan	value is <= to rule	Not applicable	value = 30 testValue = 40 (Valid) = 20 (Fail)
range	Not applicable	2 value requested (min and max)	value = 50 testValue = [10,60] (Valid) = [60, 10] (Valid) = [0, 10] (Fail) = [70, 80] (Fail) = 45 (Fail) = [45] (Fail)
notNull	Not Applicable	Not Applicable	No value

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 · 2023-04-12T16:34:52Z

✅ Deploy Preview for ohif-platform-viewer canceled.

Name	Link
🔨 Latest commit	`6b0e4dc`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-viewer/deploys/6478a78e28280800071f9d03

netlify · 2023-04-12T16:34:55Z

✅ Deploy Preview for ohif-platform-docs ready!

Name	Link
🔨 Latest commit	`81d788d`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/649d97935342fb00081d0f5e
😎 Deploy Preview	https://deploy-preview-3322--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.

wayfarer3130 · 2023-05-26T12:51:18Z

platform/core/src/services/HangingProtocolService/lib/validator.js

+    }
+    // We need to compare each element in the array
+    else {
+      if (JSON.stringify(testValue) !== JSON.stringify(value)) {


Please iterate over each item and return the information about which element is different. Also, stringify is slow and this can be called many times in order to determine which stage and which hanging display set to apply.

wayfarer3130 · 2023-05-26T12:51:37Z

platform/core/src/services/HangingProtocolService/lib/validator.js

+      if (testValue[0] === value) {
+        return `${key} must not be ${testValue}`;
+      }
+    } else if (JSON.stringify(testValue) === JSON.stringify(value)) {


Ditto about stringify.

platform/core/src/services/HangingProtocolService/lib/validator.js

codecov · 2023-06-01T12:30:02Z

Codecov Report

Merging #3322 (81d788d) into master (b5b7d32) will increase coverage by 4.53%.
The diff coverage is 92.00%.

@@            Coverage Diff             @@
##           master    #3322      +/-   ##
==========================================
+ Coverage   38.27%   42.81%   +4.53%     
==========================================
  Files          82       82              
  Lines        1348     1448     +100     
  Branches      303      337      +34     
==========================================
+ Hits          516      620     +104     
  Misses        666      666              
+ Partials      166      162       -4

Impacted Files	Coverage Δ
...c/services/HangingProtocolService/lib/validator.js	`93.54% <92.00%> (+19.00%)`	⬆️

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 fa37593...81d788d. Read the comment docs.

platform/core/src/services/HangingProtocolService/lib/validator.js

wayfarer3130 · 2023-06-01T19:23:23Z

platform/core/src/services/HangingProtocolService/lib/validator.js

+ *           = ['abc', 'def', 'dog'] (Valid)
+ *           = ['cat', 'dog'] (Fail)
+ *
+ * value = 'Attenuation Corrected' (Fail)


This case should be equivalent to ['Attenuation Corrected'], which should pass if the testValue contains the same identical string.

wayfarer3130

Could you add the two edge cases I described - the issue is that for the DICOM library we are using, it converts to/from single item arrays, so on the equals and the other one I commented on,
['abc'] and 'abc'
should be treated equivalently, both for the test value and the original value.

…COM is always an array and fix includes and doesNotIncludes validators

…lidator # Conflicts: # platform/core/src/services/HangingProtocolService/lib/validator.js # platform/core/src/services/HangingProtocolService/lib/validator.test.js

netlify · 2023-06-05T16:38:43Z

✅ Deploy Preview for ohif-dev ready!

Name	Link
🔨 Latest commit	`81d788d`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/649d9793a0517d00095b151d
😎 Deploy Preview	https://deploy-preview-3322--ohif-dev.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.

wayfarer3130 · 2023-06-06T12:02:12Z

platform/docs/docs/platform/extensions/modules/hpModule.md

+
+  - | Rule 	| Single Value 	| Array Value 	| Example 	|
+    |---	|---	|---	|---	|
+    | equals 	| === 	| All members are === in same order 	| value = ['abc', 'def', 'GHI']<br>testValue = 'abc' (Fail)<br><br>          = ['abc'] (Fail)<br><br>          = ['abc', 'def', 'GHI'] (Valid)<br><br>          = ['abc', 'GHI', 'def'] (Fail)<br><br>          = ['abc', 'def'] (Fail)<br><br>value = 'Attenuation Corrected'<br>testValue = 'Attenuation Corrected' (Valid)<br>          = 'Attenuation' (Fail)<br> 	|


I believe the tests are failing on how the <br> tag is used - I suspect you need to use
as a closed tag value (XHTML syntax). Try building hte docs locally to test.

wayfarer3130

Will approve as soon as hte build is fixed.

salimkanoun · 2023-06-13T12:05:11Z

Dear @wayfarer3130 should be good ?

wayfarer3130

Now passing the tests so good to go.

Refact equal and doesNotEqual Validation

f623164

sedghi requested a review from wayfarer3130 April 13, 2023 14:07

Adding doesNotEquals, includes and doesNotInclude Rules

d20f791

wayfarer3130 reviewed May 26, 2023

View reviewed changes

platform/core/src/services/HangingProtocolService/lib/validator.js Show resolved Hide resolved

refact

6429e82

add documentation

6b0e4dc

Sofien-Sellami changed the base branch from v3-stable to master June 1, 2023 14:35

sedghi requested a review from wayfarer3130 June 1, 2023 14:49

wayfarer3130 reviewed Jun 1, 2023

View reviewed changes

platform/core/src/services/HangingProtocolService/lib/validator.js Show resolved Hide resolved

wayfarer3130 reviewed Jun 1, 2023

View reviewed changes

wayfarer3130 requested changes Jun 1, 2023

View reviewed changes

Sofien-Sellami added 2 commits June 5, 2023 18:12

create dicomTagToArray function to ensure the value that come from DI…

095807e

…COM is always an array and fix includes and doesNotIncludes validators

Merge remote-tracking branch 'origin/refact_validator' into refact_va…

fee9445

…lidator # Conflicts: # platform/core/src/services/HangingProtocolService/lib/validator.js # platform/core/src/services/HangingProtocolService/lib/validator.test.js

Sofien-Sellami requested a review from wayfarer3130 June 6, 2023 08:59

wayfarer3130 reviewed Jun 6, 2023

View reviewed changes

wayfarer3130 requested changes Jun 6, 2023

View reviewed changes

fix documentation

07253fe

Sofien-Sellami requested a review from wayfarer3130 June 6, 2023 14:43

wayfarer3130 added 2 commits June 29, 2023 10:32

Merge branch 'master' into refact_validator

6565506

Merge branch 'master' into refact_validator

81d788d

wayfarer3130 approved these changes Jun 29, 2023

View reviewed changes

wayfarer3130 merged commit e653961 into OHIF:master Jun 29, 2023
9 of 10 checks passed

sedghi mentioned this pull request Sep 26, 2023

release candidate 3p7 RC1 #3680

Closed

This was referenced Oct 10, 2023

candidate 3p7 rc 2 #3712

Closed

feat(v3p7): segmentation mode and cloud server configuration #3717

Closed

feat(v3p7): segmentation mode and cloud server configuration #3718

Closed

feat(v3p7): segmentation mode and cloud server configuration #3720

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for array input for all constraints in HangingProtocolService validator #3322

Add support for array input for all constraints in HangingProtocolService validator #3322

Sofien-Sellami commented Apr 12, 2023 •

edited

netlify bot commented Apr 12, 2023 •

edited

netlify bot commented Apr 12, 2023 •

edited

wayfarer3130 May 26, 2023

wayfarer3130 May 26, 2023

codecov bot commented Jun 1, 2023 •

edited

wayfarer3130 Jun 1, 2023

wayfarer3130 left a comment

netlify bot commented Jun 5, 2023 •

edited

wayfarer3130 Jun 6, 2023

wayfarer3130 left a comment

salimkanoun commented Jun 13, 2023

wayfarer3130 left a comment

Add support for array input for all constraints in HangingProtocolService validator #3322

Add support for array input for all constraints in HangingProtocolService validator #3322

Conversation

Sofien-Sellami commented Apr 12, 2023 • edited

Context

Changes & Results

Constraint Rules

Testing

Checklist

PR

Code

Public Documentation Updates

Tested Environment

netlify bot commented Apr 12, 2023 • edited

✅ Deploy Preview for ohif-platform-viewer canceled.

netlify bot commented Apr 12, 2023 • edited

✅ Deploy Preview for ohif-platform-docs ready!

wayfarer3130 May 26, 2023

Choose a reason for hiding this comment

wayfarer3130 May 26, 2023

Choose a reason for hiding this comment

codecov bot commented Jun 1, 2023 • edited

Codecov Report

wayfarer3130 Jun 1, 2023

Choose a reason for hiding this comment

wayfarer3130 left a comment

Choose a reason for hiding this comment

netlify bot commented Jun 5, 2023 • edited

✅ Deploy Preview for ohif-dev ready!

wayfarer3130 Jun 6, 2023

Choose a reason for hiding this comment

wayfarer3130 left a comment

Choose a reason for hiding this comment

salimkanoun commented Jun 13, 2023

wayfarer3130 left a comment

Choose a reason for hiding this comment

Sofien-Sellami commented Apr 12, 2023 •

edited

netlify bot commented Apr 12, 2023 •

edited

netlify bot commented Apr 12, 2023 •

edited

codecov bot commented Jun 1, 2023 •

edited

netlify bot commented Jun 5, 2023 •

edited