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

Path-level parameters should be listed before operation-level parameters #7482

Open
hkosova opened this issue Sep 6, 2021 · 3 comments · May be fixed by swagger-api/swagger-js#2256
Open

Comments

@hkosova
Copy link
Contributor

hkosova commented Sep 6, 2021

Q&A (please complete the following information)

  • OS: Windows 10
  • Browser: Chrome, Firefox latest
  • Method of installation: https://editor.swagger.io
  • Swagger-UI version: 3.52.0
  • Swagger/OpenAPI version: any

Content & configuration

Example Swagger/OpenAPI definition:

openapi: 3.0.0
info:
  title: test
  version: 0.0.0
paths:
  /something/{id}:
    parameters:
      - in: path
        name: id
        required: true
        schema:
          type: string
    get:
      parameters:
        - in: query
          name: foo
          schema:
            type: string
      responses:
        '200':
          description: ok

Describe the bug you're encountering

In the API definition provided above, the /something/{id} path has path-level parameter id, plus the GET operation defines an additional parameter foo.

Swagger UI renders the path-level parameter id AFTER the operation-level parameters foo. This is often undesirable because path-level parameter list often contains in: path parameters (which are required), so it makes more sense to render path-level parameters first.

To reproduce...

Steps to reproduce the behavior:

  1. Paste the sample API definition into https://editor.swagger.io.
  2. Expand the operation.

Expected behavior

Parameter order is: id first, then foo.

Actual behavior

Parameter order is: foo, id.

Screenshots

image

@mathis-m
Copy link
Contributor

@hkosova @char0n this is not possible to do in swagger-ui. The fn resolveSubtree returns all parameters for an "method operation" by using swagger-js subtree-resolver. There the spec is normalized, in this process the inherited parameters are appended to the "method operation parameters".
With #6745 the grouping / sorting of parameters was implemented in swagger-ui, so prepending the inherited parameters in swagger-js instead will just solve the issue partially. With that said implementing the prepend of inherited parameters will result in having the first in: type at first place followed by the "method operation parameters" of in: type and so on.
Example:
input inherited:

  • in: path
    name: path1
  • in: query
    name: query1
  • in: path
    name: path2

input operation level:

  • in: path
    name: path3
  • in: query
    name: query2

result:

  • in: path
    name: path1

  • in: path
    name: path2

  • in: path
    name: path3

  • in: query
    name: query1

  • in: query
    name: query2

@char0n
Copy link
Member

char0n commented Sep 30, 2021

Hi all,

The new behavior suggested by @hkosova makes total sense (at least from my subjective perspective and others, that this has been discussed with).

@mathis-m I went through the #6745 and swagger-api/swagger-js#2256. I think we first need to define our expectations here. IMHO there is couple of things here that we need to decide and clarify.

Grouping

Grouping was introduced in #6745. We currently group Parameter Objects by in field and retain the order that they were defined in (at least within the groups). That is the current behavior according to my understanding (correct me if I'm wrong).

Sorting

We currently don't sort. It looks like we do because Grouping can appear to have that effect (again, correct me if I'm wrong).

What do we really want?

IMHO it would be beneficial to define the expected behavior here to the last intricate detail. I'm putting here the first proposal that we can elaborate on.

Proposal 1 - Rendering according the the user intention

We trust the OAS definition author, that she knows what she's doing and that she's defined the order of the parameters in the order that she deemed appropriate. The user then expects the parameters to be rendered in the order as she defined them. And here comes the @hkosova proposal - parameters defined in Path Item Object get rendered before Operation level parameters. This means we have to get rid of current grouping algorithm. Please let me know what you think about this proposal.

Feel free to provide your own proposal of expected behaviors.

@mathis-m
Copy link
Contributor

@char0n I understand proposal 1, but as a user I see 2 usability points that need consideration.

  1. Generated OAS
    When the spec is generated one might not have control over the order.
    1.1. Hope that the generator produces an logical order
    1.2. Let swagger-ui be the tool that handels the order

  2. Multiple different in: types Path Level and Method level params
    This will lead to the effect that one can not sort parameters by type via writing the spec, because the inherited params will occur before the method params. So this will actually take the possibility of sorting by type.

UX considerations that could fix this, is a table that can be sorted. Regarding #6661 and the points mentioned above I think there is the need of providing such functionality, either in automatic or manual way.
Regarding this issue there might also be the case where we support the raw order.

So this might need some UX input to find a solution:

  • make parameters sortable via ui
  • maybe also provide sort by and then by, Eg. By type then by required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants