search

OpenPEPPOL / logistics-bis

3 stars 0 forks source link

Changes needed to DeliveryTerms #16

Closed DidrikN closed 1 year ago

DidrikN commented 1 year ago

There is a request for ID in DeliveryTerms to be mandatory (currently 0..1). We have discussed this before, and we landed on ID in DeliveryTerms to be optional, but to establish a business rule for either SpecialTerms or ID to be defined (see attached image).

Therefore, if we agree that this is still the best solution, please implement the missing business rule unit tests.

image

DidrikN commented 1 year ago

We agree on this. @PeterLBorresen: Please complete:

DidrikN commented 1 year ago

Solved with https://github.com/OpenPEPPOL/logistics-bis/commit/4d37353e2e557ff01970051e9d263a000d6eaab2 , containing both rule and unit test.

RikardLarsson-BEAst commented 1 year ago

Since the validation failed, the issue cannot be closed. @DidrikN you need to check the logs before closing issue. Two errors in the log. One is unit test and the other is a use case file that the example looks ok. I suspect that the rule does not work correctly. Link to the log https://github.com/OpenPEPPOL/logistics-bis/actions/runs/4163152580/jobs/7203189841

Snippet from the example file that fails, should be ok in my opinion.

EXW Stockholm
DidrikN commented 1 year ago

Rule seems to be misconfigured. It should have a reference to cac:DeliveryTerms/cbc:ID?

"not(cac:Delivery/cac:DeliveryTerms) or ((cac:Delivery/cac:DeliveryTerms/cbc:SpecialTerms) and (cac:Delivery/cac:DeliveryTerms/cbc:SpecialTerms))" flag="fatal">Either ID or special terms need to be specified in Delivery terms

@PeterLBorresen: Please correct this.

RikardLarsson-BEAst commented 1 year ago

I think that we can use a similar rule as for PEPPOL-T120-R003 Something like (cac:Delivery/cac:DeliveryTerms/cbc:ID or cac:Delivery/cac:DeliveryTerms/cbc:SpecialTerms)

RikardLarsson-BEAst commented 1 year ago

This worked not(cac:Delivery/cac:DeliveryTerms) or ((cac:Delivery/cac:DeliveryTerms/cbc:ID) or (cac:Delivery/cac:DeliveryTerms/cbc:SpecialTerms)) Sorry, it was not for me to fix but these rule can be quite fun to work with @DidrikN will you check?

DidrikN commented 1 year ago

@RikardLarsson-BEAst: Awesome!

@PeterLBorresen: Please disregard my message above. Rikard has fixed the issue.

We have verified that rule 22 is now working - and all unit test are now working.