search

dcsaorg / DCSA-OpenAPI

DCSA - Digital Container Shipping Association
https://app.swaggerhub.com/apis/dcsaorg/DCSA_OAS
68 stars 13 forks source link

SD-1847: Add conditions for Buyer and Seller on ICS2 #454

Closed HenrikHL closed 2 days ago

HenrikHL commented 2 days ago

User description

SD-1847: Conditions have been added to the Buyer and Seller descriptons. In this PR - a new SellerHBL and BuyerHBL object have been created because of the difference in the conditions when Seller and Buyer are used on master level vs House B/L level

PR Type

enhancement

Description

Changes walkthrough 📝

Relevant files
Enhancement
EBL_v3.0.0.yaml
Add conditions and new objects for Buyer and Seller           
ebl/v3/EBL_v3.0.0.yaml
  • Added conditions for Buyer and Seller when certain criteria are met.
  • Introduced new SellerHBL and BuyerHBL objects for House B/L level.
  • Updated references from Seller to SellerHBL and Buyer to BuyerHBL.
    		•  +100/-2 
    Additional files (token-limit)
    styleguide.json
    ...                                                                                                           
    .stoplight/styleguide.json ...
    		+2/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    codiumai-pr-agent-pro[bot] commented 2 days ago

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Schema Duplication
    The Buyer/Seller and BuyerHBL/SellerHBL schemas are nearly identical except for their conditions. Consider using a shared base schema to reduce duplication and maintenance overhead. Documentation Clarity
    The conditions for when Buyer/Seller are mandatory could be formatted more clearly in the description, perhaps using bullet points or a table format for better readability.
    codiumai-pr-agent-pro[bot] commented 2 days ago

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Ensure consistent validation conditions across related schema definitions to prevent validation ambiguity ___ **The condition description for Buyer and Seller should be consistent across all
    instances. Currently, the House B/L level condition is missing the
    isHouseBillofLadingsIssued check, which could lead to validation inconsistencies.** [ebl/v3/EBL_v3.0.0.yaml [5103]](https://github.com/dcsaorg/DCSA-OpenAPI/pull/454/files#diff-d4c1278844f45120b3c40d19b390ea6bec7a06a3f63258e9af74eba381c36e35R5103-R5103) ```diff -**Condition:** Buyer and Seller are mandatory if `isCargoDeliveredInICS2Zone=true` (on House B/L level) and `manifesttypecode='ENS'` and `advancedManifestFilingPerformedBy='CARRIER'`. +**Condition:** Buyer and Seller are mandatory if `isCargoDeliveredInICS2Zone=true` (on House B/L level) and `manifesttypecode='ENS'` and `advancedManifestFilingPerformedBy='CARRIER'` and `isHouseBillofLadingsIssued=true`. ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 8 Why: The suggestion identifies an important inconsistency in validation conditions between regular and House B/L schemas that could lead to incorrect validation behavior. Adding the missing condition is crucial for maintaining data integrity.
    		8
    General
    Replace regex pattern with enum for fields that only accept predefined values ___ **The pattern for typeOfPerson should be replaced with an enum since it only accepts
    specific predefined values.** [ebl/v3/EBL_v3.0.0.yaml [5066-5068]](https://github.com/dcsaorg/DCSA-OpenAPI/pull/454/files#diff-d4c1278844f45120b3c40d19b390ea6bec7a06a3f63258e9af74eba381c36e35R5066-R5068) ```diff typeOfPerson: type: string - maxLength: 50 - pattern: ^\S(?:.*\S)?$ + enum: + - NATURAL_PERSON + - LEGAL_PERSON + - ASSOCIATION_OF_PERSONS ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: Using an enum instead of a regex pattern would provide better type safety and validation for the typeOfPerson field, as it has a fixed set of valid values explicitly defined in the description.
    		7

    💡 Need additional feedback ? start a PR chat