Skip to content

Conversation

@HenrikHL
Copy link
Contributor

@HenrikHL HenrikHL commented Aug 14, 2025

User description

SD-2389: Allow only a single B/L (the sum of B/Ls with/without charges cannot exceed 1)


PR Type

Documentation


Description

  • Add validation constraint for Bill of Lading originals

  • Ensure sum of originals with/without charges ≤ 1

  • Update API documentation across multiple schemas


Diagram Walkthrough

flowchart LR
  A["numberOfOriginalsWithCharges"] --> C["Sum Validation"]
  B["numberOfOriginalsWithoutCharges"] --> C
  C --> D["Must be ≤ 1"]
Loading

File Walkthrough

Relevant files
Documentation
EBL_v3.0.2.yaml
Add B/L originals sum validation constraint                           

ebl/v3/EBL_v3.0.2.yaml

  • Add constraint requiring sum of numberOfOriginalsWithCharges +
    numberOfOriginalsWithoutCharges ≤ 1
  • Update field descriptions in multiple schema components
  • Apply validation rule consistently across all Bill of Lading
    definitions
+8/-8     
EBL_ISS_v3.0.2.yaml
Add B/L originals validation in issuance schema                   

ebl/v3/issuance/EBL_ISS_v3.0.2.yaml

  • Add same validation constraint for Bill of Lading originals
  • Update descriptions for numberOfOriginalsWithCharges and
    numberOfOriginalsWithoutCharges fields
+2/-2     
EBL_PINT_v3.0.0.yaml
Add B/L originals validation in PINT schema                           

pint/v3/EBL_PINT_v3.0.0.yaml

  • Apply identical validation constraint for Bill of Lading originals
  • Update field descriptions to include sum validation rule
+2/-2     

@qodo-code-review
Copy link

qodo-code-review bot commented Aug 14, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

SD-2389 - Partially compliant

Compliant requirements:

  • Apply this condition only when transportDocumentType = BOL.
  • If isElectronic = 'True', accepted value for each attribute is 0 or 1.
  • Update API documentation/specification accordingly across relevant schemas.

Non-compliant requirements:

  • Update the condition for the two attributes so only one of them can be 1 (their sum must be ≤ 1).

Requires further human verification:

  • Confirm that the mutual exclusivity (sum ≤ 1) is enforced by actual schema validation (e.g., via oneOf/anyOf/allOf with property dependencies) and not only documented text.
  • Verify no other schema locations referencing these fields were missed.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Validation Missing

The change updates descriptions only; no JSON Schema/OpenAPI validation keywords (e.g., oneOf with sum constraint, custom extension) enforce that the sum of the two fields is ≤ 1. This may leave the API permissive unless validators are implemented elsewhere.

  description: |
    Number of originals of the Bill of Lading that has been requested by the customer with charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1
numberOfOriginalsWithoutCharges:
  type: integer
  format: int32
  minimum: 0
  description: |
    Number of originals of the Bill of Lading that has been requested by the customer without charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1
Consistency Check

Similar description-only change; ensure actual schema constraints or server-side validation are added and consistent between base and issuance specs.

  description: |
    Number of originals of the Bill of Lading that has been requested by the customer with charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1
numberOfOriginalsWithoutCharges:
  type: integer
  format: int32
  minimum: 0
  description: |
    Number of originals of the Bill of Lading that has been requested by the customer without charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1
Enforcement Gap

PINT spec also only updates descriptive text; consider adding a formal constraint or cross-field validation mechanism recognized by consumers.

  description: |
    Number of originals of the Bill of Lading that has been requested by the customer with charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1
numberOfOriginalsWithoutCharges:
  type: integer
  format: int32
  minimum: 0
  description: |
    Number of originals of the Bill of Lading that has been requested by the customer without charges.

    **Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
  example: 1

@qodo-code-review
Copy link

qodo-code-review bot commented Aug 14, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Enforce sum constraint via schema

Documenting the sum constraint in descriptions does not enforce it. Add a
schema-level validation (e.g., allOf with a conditional and a maxItems-like sum
rule) to programmatically ensure the two fields' sum is ≤ 1 when
transportDocumentType is BOL. This prevents invalid payloads from passing
validation.

ebl/v3/EBL_v3.0.2.yaml [3123]

-**Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
+allOf:
+  - if:
+      properties:
+        transportDocumentType:
+          const: BOL
+    then:
+      properties:
+        numberOfOriginalsWithCharges:
+          type: integer
+          minimum: 0
+        numberOfOriginalsWithoutCharges:
+          type: integer
+          minimum: 0
+      required:
+        - numberOfOriginalsWithCharges
+        - numberOfOriginalsWithoutCharges
+      unevaluatedProperties: true
+      allOf:
+        - description: Sum of originals with/without charges must be ≤ 1
+          not:
+            anyOf:
+              - properties:
+                  numberOfOriginalsWithCharges:
+                    minimum: 1
+                  numberOfOriginalsWithoutCharges:
+                    minimum: 1
+                required:
+                  - numberOfOriginalsWithCharges
+                  - numberOfOriginalsWithoutCharges
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that adding a constraint to a description field does not enforce it, and proposes a valid schema-level validation to fix this, which is a critical improvement.

High
General
Scope constraint to electronic B/L

Clarify whether the sum constraint applies only when isElectronic = 'True' or
always for BOL. If it's only for electronic, gate the sum rule additionally on
isElectronic = true to avoid rejecting valid paper B/L cases.

ebl/v3/issuance/EBL_ISS_v3.0.2.yaml [662]

-**Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
+allOf:
+  - if:
+      properties:
+        transportDocumentType:
+          const: BOL
+        isElectronic:
+          const: true
+      required:
+        - transportDocumentType
+        - isElectronic
+    then:
+      allOf:
+        - description: For electronic B/L, originals sum must be ≤ 1
+          not:
+            anyOf:
+              - properties:
+                  numberOfOriginalsWithCharges:
+                    minimum: 1
+                  numberOfOriginalsWithoutCharges:
+                    minimum: 1
+                required:
+                  - numberOfOriginalsWithCharges
+                  - numberOfOriginalsWithoutCharges
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an ambiguity in the condition's wording and asks for clarification, which is valuable for ensuring the logic is implemented as intended.

Medium
Set defaults and enforce integers

Ensure numeric types are strictly integers and defaults are defined to avoid
nulls bypassing the sum check. Set default: 0 for both fields and include type:
integer with minimum: 0 within the conditional block so validators compute the
sum correctly.

pint/v3/EBL_PINT_v3.0.0.yaml [1247]

-**Condition:** Only applicable if `transportDocumentType` = `BOL` (Bill of Lading). If `isElectronic = 'True'`, accepted value is `1` (one) or `0` (zero). `numberOfOriginalsWithoutCharges` + `numberOfOriginalsWithCharges` must be ≤ 1.
+properties:
+  numberOfOriginalsWithCharges:
+    type: integer
+    format: int32
+    minimum: 0
+    default: 0
+  numberOfOriginalsWithoutCharges:
+    type: integer
+    format: int32
+    minimum: 0
+    default: 0
+allOf:
+  - if:
+      properties:
+        transportDocumentType:
+          const: BOL
+      required:
+        - transportDocumentType
+    then:
+      allOf:
+        - description: Sum of originals must be ≤ 1
+          not:
+            anyOf:
+              - properties:
+                  numberOfOriginalsWithCharges:
+                    minimum: 1
+                  numberOfOriginalsWithoutCharges:
+                    minimum: 1
+                required:
+                  - numberOfOriginalsWithCharges
+                  - numberOfOriginalsWithoutCharges
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion to add default values is a good practice for robustness, but the schema already defines the fields as integers with a minimum of 0, making part of the suggestion redundant.

Low
  • Update

@HenrikHL HenrikHL merged commit 9dc2c87 into master Aug 14, 2025
1 check passed
@HenrikHL HenrikHL deleted the SD-2389_Only_1_Original branch August 14, 2025 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants