Skip to content

Add vesselINONumber filter to voyage-references - #640

Merged
HenrikHL merged 1 commit into
masterfrom
voyage-reference_IMO-filter
Jul 31, 2026
Merged

Add vesselINONumber filter to voyage-references#640
HenrikHL merged 1 commit into
masterfrom
voyage-reference_IMO-filter

Conversation

@HenrikHL

Copy link
Copy Markdown
Contributor

No description provided.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add vesselIMONumber query filter to voyage-references API spec

✨ Enhancement 📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Document a new vesselIMONumber query parameter for GET /voyage-references.
• Constrain the parameter with a 7–8 digit numeric pattern and length limits.
• Clarify behavior: only return partner voyages matching the provided IMO number.
Diagram

graph TD
  C["API consumer"] --> A["OVS Hub Ref API"] --> E["GET /voyage-references"] --> F["Filter: vesselIMONumber"] --> R["Filtered voyage list"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Promote vesselIMONumber to a reusable OpenAPI parameter component
  • ➕ Avoids duplication if other endpoints add the same filter later
  • ➕ Ensures consistent validation/description across the spec
  • ➖ Slightly more indirection in the YAML for a single use-case
  • ➖ Requires refactoring if the spec currently keeps parameters inline
2. Model IMO as an integer query parameter
  • ➕ More explicit “numeric” intent for consumers
  • ➕ May simplify some client-side validation
  • ➖ Loses leading-zero safety (if ever applicable) and can be awkward in some clients
  • ➖ OpenAPI tooling often treats query integers differently than string patterns; current pattern validation is clearer

Recommendation: Current inline string+pattern approach is appropriate for a single endpoint and provides precise validation. If the API roadmap suggests reusing IMO filtering elsewhere, extracting vesselIMONumber into components/parameters would improve consistency and maintainability.

Files changed (1) +10 / -0

Documentation (1) +10 / -0
OVS_HUB_REF_v1.0.0.yamlAdd vesselIMONumber query parameter to /voyage-references +10/-0

Add vesselIMONumber query parameter to /voyage-references

• Introduces a new 'vesselIMONumber' query parameter for 'GET /voyage-references'. The parameter is documented with a numeric regex pattern and 7–8 character length constraints, and the description clarifies that results are restricted to voyages matching the provided IMO number.

ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unclear filter requirements 🐞 Bug ⚙ Maintainability
Description
vesselIMONumber was added as a query parameter, but the endpoint’s documented mandatory filter
conditions were not updated, leaving it ambiguous whether vesselIMONumber can be used alone or
only as an additional narrowing filter. This ambiguity can cause client implementations to
misconstruct requests or avoid using the new filter due to unclear contract semantics.
Code

ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[R302-306]

+        - name: vesselIMONumber
+          in: query
+          description: |
+            The identifier of a vessel via the `vesselIMONumber`. The result will only return partner voyages including the `vesselIMONumber`.
+          schema:
Evidence
The operation description explicitly states mandatory filters and lists only four voyage filters,
but the newly added vesselIMONumber parameter is not mentioned in those requirements, creating
ambiguity about whether it is mandatory/alternative/additional.

ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[194-213]
ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[241-298]
ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[302-311]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GET /v1/voyage-references` documents mandatory filters (service + one of the existing voyage filters), but the newly added `vesselIMONumber` parameter is not reflected in those conditions. The contract should explicitly state whether `vesselIMONumber` (a) can satisfy the “voyage filter” requirement, or (b) is only an optional additional filter that still requires the existing mandatory filters.

## Issue Context
The endpoint-level description and per-parameter **Condition** notes currently enumerate only `carrierImportVoyageNumber`, `carrierExportVoyageNumber`, `universalImportVoyageReference`, and `universalExportVoyageReference` as mandatory voyage filters, while the new `vesselIMONumber` parameter is added without integrating it into that narrative.

## Fix Focus Areas
- ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[201-213]
- ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[241-311]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. IMO length inconsistent 🐞 Bug ≡ Correctness
Description
The new vesselIMONumber query parameter allows 7 or 8 digits (^\d{7,8}$), which conflicts with
other specifications in this repo that define IMO number as exactly 7 digits. This inconsistency can
lead to interoperability issues and inconsistent client/server-side validation across APIs.
Code

ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[R307-310]

+            type: string
+            pattern: ^\d{7,8}$
+            minLength: 7
+            maxLength: 8
Evidence
The added parameter explicitly permits 8 digits, while other repo specs define vesselIMONumber as
exactly 7 digits, demonstrating a concrete contract inconsistency introduced/expanded by this
change.

ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[302-311]
ovs/v3/OVS_v3.0.2.yaml[85-93]
domain/dcsa/dcsa_domain_v3.1.1.yaml[1997-2003]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `vesselIMONumber` query parameter schema accepts 8-digit values, but other authoritative-looking specs in this repo define `vesselIMONumber` as exactly 7 digits. Update the query parameter schema (and, if appropriate, the shared Vessel schema) to match the intended standard.

## Issue Context
`OVS_v3.0.2` and `dcsa_domain_v3.1.1` define `vesselIMONumber` as `^\d{7}$` with `maxLength: 7`. The hub reference spec’s new query parameter currently uses `^\d{7,8}$` with `maxLength: 8`.

## Fix Focus Areas
- ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[302-311]
- ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml[812-819]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml
Comment thread ovs_hub_ref/v1/OVS_HUB_REF_v1.0.0.yaml
@HenrikHL
HenrikHL merged commit 8a55f5c into master Jul 31, 2026
1 check passed
@HenrikHL
HenrikHL deleted the voyage-reference_IMO-filter branch July 31, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant