Skip to content

Conversation

@Vovan-VE
Copy link
Contributor

@Vovan-VE Vovan-VE commented Feb 20, 2020

Description

Display nullable marker for object model itself.

To display a nullable marker I used the same semantic as already used for primitive types, t.i. I display nullable: <boolean> when it is provided in schema independent of its value true/false.

Motivation and Context

Fixes #5660. When a property is nullable object, the model view did not show nullable status. This is a lack of information provided by an underlying schema.

As an example, a property may be required (that is always presented in a response or must be explicitly passed in a request), but can also be null and not required always to be an object.

How Has This Been Tested?

  1. I got modified example schema from Nullable objects #5660 and saved it to dev-helpers/test.yml:
openapi: 3.0.2
info:
  title: Foo
  description: ''
  version: '1.0'
paths: {}
components:
  schemas:
    SomeObject:
      type: object
      properties:
        meta:
          type: object
          properties:
            tag:
              type:  string
          nullable: true
      nullable: true
  1. npm run dev and loaded /test.yml
  2. I see nullable: true both for meta property definition and whole SomeObject model itself. I played with this nullables by commenting them out and reloading /test.yml on page.

Also I did check and confirm than I did not see nullable markers in step 3 before my changes.

Screenshots (if appropriate):

nullable-object

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@Vovan-VE
Copy link
Contributor Author

Are cypress tests required for such changes?

Copy link
Contributor

@tim-lai tim-lai left a comment

Choose a reason for hiding this comment

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

@Vovan-VE Please add Cypress or Mocha tests to this PR. Otherwise, this change looks good. Thanks!

@tim-lai tim-lai self-assigned this Jun 13, 2020
@Vovan-VE
Copy link
Contributor Author

Vovan-VE commented Jun 13, 2020

Should it render nullable: false when it forced in schema? As far as I remember, I did use the same logic as primitive type uses.

@Vovan-VE Vovan-VE requested a review from tim-lai June 13, 2020 06:12
@tim-lai
Copy link
Contributor

tim-lai commented Jun 15, 2020

@Vovan-VE

Should it render nullable: false when it forced in schema? As far as I remember, I did use the same logic as primitive type uses.

Yes. Ok to display whatever the actual boolean value is. Thanks.

@tim-lai tim-lai merged commit 41e595b into swagger-api:master Jun 23, 2020
@Vovan-VE Vovan-VE deleted the bug/5660-nullable-object branch June 23, 2020 01:13
@tim-lai
Copy link
Contributor

tim-lai commented Jun 23, 2020

@Vovan-VE Merged! Thanks for the contribution1

mattyb678 pushed a commit to mattyb678/swagger-ui that referenced this pull request Jun 24, 2020
@frantuma frantuma added this to the M4 milestone Jul 9, 2020
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.

Nullable objects

3 participants