Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SD-642 Adjust the isExportDeclarationRequired condition #244

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

preetamnpr
Copy link
Collaborator

@preetamnpr preetamnpr commented Dec 13, 2024

User description

…ant unit test cases.


PR Type

Enhancement


Description

  • Enhanced export declaration reference validation by implementing a new IS_EXPORT_DECLARATION_REFERENCE_PRESENCE check that:
    • Validates presence of exportDeclarationReference when isExportDeclarationRequired is true
    • Ensures absence of exportDeclarationReference when isExportDeclarationRequired is false
  • Added comprehensive unit tests covering all possible scenarios for export declaration reference validation
  • Removed deprecated import license reference validation
  • Cleaned up booking message templates by removing redundant declaration flags

Changes walkthrough 📝

Relevant files
Enhancement
BookingChecks.java
Refactor export declaration reference validation logic     

booking/src/main/java/org/dcsa/conformance/standards/booking/checks/BookingChecks.java

  • Replaced separate export/import declaration checks with a single
    IS_EXPORT_DECLARATION_REFERENCE_PRESENCE check
  • Updated check logic to handle both presence and absence of export
    declaration reference based on isExportDeclarationRequired flag
  • Removed import license reference check
  • +5/-11   
    Tests
    BookingChecksTest.java
    Add test coverage for export declaration reference validation

    booking/src/test/java/org/dcsa/conformance/standards/booking/checks/BookingChecksTest.java

  • Added comprehensive test cases for
    IS_EXPORT_DECLARATION_REFERENCE_PRESENCE check
  • Tests cover all scenarios: required/not required, present/absent,
    missing flag cases
  • +45/-0   
    Configuration changes
    *.json
    Clean up declaration flags from booking message templates

    booking/src/main/resources/standards/booking/messages/*.json

  • Removed isExportDeclarationRequired and isImportLicenseRequired flags
    from multiple booking message templates
  • Removed exportDeclarationReference and importLicenseReference from
    regular.json template

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

    Copy link

    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 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logic Validation
    Verify that the new export declaration reference validation logic correctly handles all edge cases and maintains backward compatibility

    Data Removal
    Confirm that removing the import license fields and modifying export declaration fields doesn't break existing integrations

    Copy link

    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
    General
    Add descriptive error message for validation failure cases to improve user experience

    The ifThenElse condition is missing a description for the 'else' case. Add a
    descriptive message to help users understand why the exportDeclarationReference must
    be absent when isExportDeclarationRequired is false.

    booking/src/main/java/org/dcsa/conformance/standards/booking/checks/BookingChecks.java [194-199]

     static final JsonContentCheck IS_EXPORT_DECLARATION_REFERENCE_PRESENCE = JsonAttribute.ifThenElse(
       "Check Export declaration reference presence",
       JsonAttribute.isTrue(JsonPointer.compile("/isExportDeclarationRequired")),
       JsonAttribute.mustBePresent(JsonPointer.compile("/exportDeclarationReference")),
    -  JsonAttribute.mustBeAbsent(JsonPointer.compile("/exportDeclarationReference"))
    +  JsonAttribute.mustBeAbsent(JsonPointer.compile("/exportDeclarationReference"), "Export declaration reference must be absent when not required")
     );
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion improves error messaging by adding a descriptive message for the 'else' case, which would help users better understand validation failures. While helpful for debugging and user experience, it's not critical for functionality.

    5

    @jkosternl jkosternl changed the title SD-642 adjust the isExportDeclarationRequired condition and add relev… SD-642 Adjust the isExportDeclarationRequired condition Dec 13, 2024
    Copy link
    Collaborator

    @jkosternl jkosternl left a comment

    Choose a reason for hiding this comment

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

    Nice Preetam. Please wait for S. as well.

    @preetamnpr preetamnpr merged commit 16577e0 into dev Dec 16, 2024
    1 check passed
    @preetamnpr preetamnpr deleted the SD-642-export-import-licenses branch December 16, 2024 08:49
    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