Skip to content

Conversation

@furkanayhan
Copy link
Contributor

@furkanayhan furkanayhan commented Apr 13, 2017

Hi there,

I am not sure that this is the right way to achieve this behaviour, but before this, it was not rendering my nested readOnly attributes.

Here is my resources:

  ResourceIdentifier:
    type: object
    required:
      - id
    properties:
      id:
        type: string
        maxLength: 255
      type:
        type: string
        readOnly: true
  ResourceMeta:
    type: object
    properties:
      created-at:
        type: string
        format: date-time
        readOnly: true
      updated-at:
        type: string
        format: date-time
        readOnly: true

  Product:
    type: object
    allOf:
      - $ref: '#/definitions/ResourceIdentifier'
      - type: object
        properties:
          meta:
            $ref: '#/definitions/ResourceMeta'
      - type: object
        properties:
          attributes:
            type: object
            properties:
              stock_count:
                type: number
                readOnly: true
              name:
                type: string
                readOnly: false
              unit:
                type: string
                readOnly: false

Before:

screen shot 2017-04-13 at 16 04 39

After:
screen shot 2017-04-13 at 16 04 55

@webron webron requested a review from shockey April 13, 2017 13:21
@webron
Copy link
Contributor

webron commented Apr 13, 2017

Thanks for the PR. I'll wait for @shockey to review the code itself, but a couple of issues:

  1. readOnly, if rendered in the sample, should only be rendered in the responses. It seems like your code change would affect body parameter rendering too.
  2. Build is failing 😕

@furkanayhan
Copy link
Contributor Author

Thanks for the quick response @webron .

  1. We don't send { includeReadOnly: true } to getSampleSchema in anywhere but in response.jsx. So I don't think it will affect. (Still not sure about it)
  2. I don't know If I should this, but I added react-test-renderer as a dev dependency, and it worked. Any idea?

@shockey shockey self-assigned this Apr 13, 2017
@shockey
Copy link
Contributor

shockey commented Apr 15, 2017

LGTM

@shockey
Copy link
Contributor

shockey commented Apr 15, 2017

thanks @furkanayhan!

@shockey shockey merged commit 2e91ab0 into swagger-api:master Apr 15, 2017
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.

3 participants