shipengine-openapi icon indicating copy to clipboard operation
shipengine-openapi copied to clipboard

Name and Phone Fields On Address Model Should Not Be Required

Open duncan-G opened this issue 4 years ago • 1 comments

In the openapi.yaml, the name and phone fields on the partial address model as well as the address model are not nullable. This creates a problem in the validate address operation. Below you can see that the operation does not require name or phone in the request body.

    address_to_validate:
      title: address_to_validate
      type: object
      description: |
        Any residential or business mailing address, anywhere in the world.
      required:
        - address_line1
        - city_locality
        - state_province
        - country_code
      additionalProperties: false
      allOf:
        - $ref: '#/components/schemas/partial_address'

However, the response returns the original address, but the type for the original address is the address type which requires name and phone. See below.

    address:
      title: address
      type: object
      description: >
        Any residential or business mailing address, anywhere in the world.
        > **Note:** Either `name` or `company_name` must be set. Both may be
        specified, if relevant.
      required:
        - name  # <----- SHOULD NOT BE REQUIRED
        - phone # <----- SHOULD NOT BE REQUIRED
        - address_line1
        - city_locality
        - state_province
        - postal_code
        - country_code
        - address_residential_indicator
      additionalProperties: false
      allOf:
        - $ref: '#/components/schemas/partial_address'

On the partial_address, the name and phone are not set to be nullable. Maybe they should.

    partial_address:
      title: partial_address
      type: object
      description: A complete or partial mailing address.
      additionalProperties: false
      properties:
        name:
          type: string
          minLength: 1
          example: John Doe
          description: >
            The name of a contact person at this address.  This field may be set
            instead of - or in addition to - the `company_name` field.
         nullable: true # <---- MAYBE NEEDS TO BE NULLABLE
        phone:
          type: string
          minLength: 1
          nullable: true # <----  MAYBE NEEDS TO BE NULLABLE
          example: +1 204-253-9411 ext. 123
          description: >
            The phone number of a contact person at this address.  The format of
            this phone number varies depending on the country.

This error causes an issue when generating code for a strongly typed language or generating code with validation. The solution here is to make these fields not required or have separate address models.

duncan-G avatar Mar 26 '21 18:03 duncan-G

Hi @duncan-G. Thanks for reporting this issue. This discrepancy is because we currently re-use the same address model in multiple places in the API, including both requests and responses. But in some fields are required in some places but optional in others. Same goes for nullability. So we really need to just create separate address models for each request/response rather than trying to re-use the same model everywhere. I'll file a Jira ticket for this and we'll get an engineer on it as soon as we can.

JamesMessinger avatar Mar 27 '21 19:03 JamesMessinger