Skip to content

Conversation

@HenrikHL
Copy link
Contributor

@HenrikHL HenrikHL commented Aug 14, 2025

SD-2384: Remove mandatory shipmentCutOffTime requirement

@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-2384 - Partially compliant

Compliant requirements:

  • Ensure shipmentCutOffTimes remains available/optional in the schema.
  • Apply the change specifically for BKG v2.0.3 Booking confirmation documentation/spec.

Non-compliant requirements:

  • Update BKG API conformance so that when bookingStatus is CONFIRMED/PENDING_AMENDMENT/COMPLETED/DECLINED, only confirmedEquipments and transportPlan are required; shipmentCutOffTimes must no longer be mandatory.

Requires further human verification:

  • Verify across the spec that shipmentCutOffTimes is not referenced as mandatory under PENDING_AMENDMENT, COMPLETED, or DECLINED states (not visible in this diff).
  • Confirm server-side validation and API implementation align with the updated conformance (not just documentation).
  • Check changelog/release notes and versioning consistency for v2.0.3.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incomplete Scope

The ticket requires relaxing the mandate for shipmentCutOffTimes for multiple statuses (CONFIRMED, PENDING_AMENDMENT, COMPLETED, DECLINED). The diff only removes the condition under a CONFIRMED-specific note; verify other status-dependent sections and any global validation rules are updated accordingly.

    **Condition:** Mandatory and non-empty for a `CONFIRMED` Booking
  items:
    $ref: '#/components/schemas/Transport'
shipmentCutOffTimes:
  type: array
  description: |
    A list of cut-off times provided by the carrier in the booking confirmation. A cut-off time indicates the latest deadline within which a task must be completed. The confirmed schedule cannot be guaranteed if a cut-off time is missed. Customs brokers may set additional cut-off times to receive the export customs documentation, which is not included in the shipment cut-off times of a carrier booking.
  items:
    $ref: '#/components/schemas/ShipmentCutOffTime'
advanceManifestFilings:
Consistency

Ensure no other parts of the schema (e.g., property descriptions, allOf/anyOf constraints, examples) still imply shipmentCutOffTimes is required, and that examples remain valid after making the field optional.

shipmentCutOffTimes:
  type: array
  description: |
    A list of cut-off times provided by the carrier in the booking confirmation. A cut-off time indicates the latest deadline within which a task must be completed. The confirmed schedule cannot be guaranteed if a cut-off time is missed. Customs brokers may set additional cut-off times to receive the export customs documentation, which is not included in the shipment cut-off times of a carrier booking.
  items:
    $ref: '#/components/schemas/ShipmentCutOffTime'
advanceManifestFilings:

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Set explicit zero minItems

Since the array may be empty, define minItems: 0 explicitly to align the schema
with the new optional behavior and avoid validators inheriting a nonzero
default. This prevents accidental validation failures when no cut-off times are
provided.

bkg/v2/BKG_v2.0.3.yaml [4105-4106]

+minItems: 0
 items:
   $ref: '#/components/schemas/ShipmentCutOffTime'

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: This is a good suggestion as it makes the schema definition more explicit by adding minItems: 0, which aligns with the removal of the "non-empty" condition and ensures validators correctly handle empty arrays.

Low
General
Make optionality explicit in docs

Clarify the non-mandatory nature in the description to avoid consumer
assumptions of guaranteed presence. Explicitly state that the array may be
omitted or empty for CONFIRMED bookings to prevent validation and client parsing
errors.

bkg/v2/BKG_v2.0.3.yaml [4103-4106]

 description: |
   A list of cut-off times provided by the carrier in the booking confirmation. A cut-off time indicates the latest deadline within which a task must be completed. The confirmed schedule cannot be guaranteed if a cut-off time is missed. Customs brokers may set additional cut-off times to receive the export customs documentation, which is not included in the shipment cut-off times of a carrier booking.
+  
+  Note: This field is optional and may be omitted or an empty array, including for `CONFIRMED` bookings.
 items:
   $ref: '#/components/schemas/ShipmentCutOffTime'

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that while the PR removes the mandatory condition, explicitly documenting the new optional nature in the description improves clarity for API consumers.

Low
  • More

Copy link
Contributor

@palatsangeetha palatsangeetha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Do we have an issue to track these changes in coformance?

@HenrikHL
Copy link
Contributor Author

Looks good to me. Do we have an issue to track these changes in coformance?

Yes - it is part of the Word document

@HenrikHL HenrikHL merged commit 4833d2b into master Aug 14, 2025
1 check passed
@HenrikHL HenrikHL deleted the SD-2384_Remove-mandatory-cutOffTimes branch August 14, 2025 09:59
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