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

asterisk is not shown for inherited nested required properties #7618

Closed
lipnitsk opened this issue Nov 5, 2021 · 1 comment · Fixed by swagger-api/swagger-js#2324
Closed

Comments

@lipnitsk
Copy link
Contributor

lipnitsk commented Nov 5, 2021

Q&A

  • OS: Any
  • Browser: Any
  • Swagger-UI version: 4.0.1
  • Swagger/OpenAPI version: OpenAPI 3.0

Content & configuration

Example Swagger/OpenAPI definition:

openapi: 3.0.3
info:
  version: 1.0.0
  title: test
  description: test
  contact:
    name: test
    email: test@example.com
servers:
  - url: 'https://localhost/api'
tags:
  - name: Test
    description: Test
paths:
  /one:
    get:
      description: test
      operationId: one
      tags:
        - Test
      parameters:
        - name: test
          in: header
          required: true
          schema:
            type: string
      responses:
        'default':
          description: Response
          content:
            text/plain:
              schema:
                type: integer
                enum: [ 401, 404, 500 ]

components:
  schemas:
    parent:
      type: object
      required:
        - required1
      properties:
        required1:
          type: string
        nested1:
          type: object
          required:
            - nestedrequired1
          properties:
            nestedrequired1:
              type: string

    child:
      allOf:
        - $ref: '#/components/schemas/parent'
        - type: object
          required:
            - required2
          properties:
            required2:
              type: string
            nested1:
              type: object
              required:
                - nestedrequired2
              properties:
                nestedrequired2:
                  type: string

Swagger-UI configuration options: Default.

Describe the bug you're encountering

To reproduce...

Steps to reproduce the behavior:

  1. Open the above YAML in https://editor.swagger.io/
  2. Expand "Schemas"
  3. Notice missing * next to nestedrequired1 under child

Expected behavior

nestedrequired1 is marked with *, just like required1, under child

Screenshots

image

Additional context or thoughts

Is this a swagger-ui or a swagger-js bug?

@lipnitsk
Copy link
Contributor Author

lipnitsk commented Nov 6, 2021

This is the deep case of #3328

lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 6, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 6, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 8, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 8, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 8, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 29, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Nov 29, 2021
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
lipnitsk added a commit to lipnitsk/swagger-js that referenced this issue Jan 4, 2022
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Extend all-of merge test case based on
swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui swagger-api#3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
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 a pull request may close this issue.

1 participant