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

required parameters w/in request body not being recognized as required #337

Closed
dawn-stripe opened this issue Mar 2, 2021 · 5 comments
Closed
Assignees
Labels
bug Something isn't working request-body

Comments

@dawn-stripe
Copy link

I'm having trouble generating a collection that only enables required parameters as by default (I also don't see the extra description) it looks like required parameters get). If I don't set the disableOptionalParameters config option than all parameters are enabled by default, but if I add it, then all my parameters including required parameters are enabled by default. My schema has a content type and I've tried specifying the required both as a property on the parameter itself as well as including it in the schema.required hash. When I look at the logic in SchemaUtils.js it seems like the required property on the parameter itself should have been picked up? I couldn't see any logic that is looking at the schema.required property but I'm still familiarizing myself with the code. Advice?

Here's an snippet from my schema (which I have confirmed validates against the OpenAPI spec) :

"post": {
        "description": 
        "operationId": "PostCoupons",
        "requestBody": {
          "content": {
            "application/x-www-form-urlencoded": {
              "encoding": ...
              "schema": {
                "additionalProperties": false,
                "properties": {
                  "duration": {
                    "description": "Specifies how long the discount will be in effect. Can be `forever`, `once`, or `repeating`.",
                    "enum": [
                      "forever",
                      "once",
                      "repeating"
                    ],
                    "required":"true",
                    "type": "string"
                  },
                 ...
                },
                "required": [
                  "duration"
                ],
                "type": "object"
              }
            }
          },
          "required": true
        },
etc.
@dawn-stripe
Copy link
Author

dawn-stripe commented Mar 3, 2021

Investigating this a bit further with a forked version of the converter, I think this is bug in how required params are handled when included in request bodies. I only know the schema I'm working with so I'm hesitant to submit a PR, but the fix for us were the following changes convertToPmBody:

before looping through the keys within the body pull the required hash from the schema, i.e. schema_requirements = _.get(contentObj[URLENCODED], ['schema', 'required'], {});

limit checking for the required property within the properties path on the key. I had to do for our preferences - we want to restrict setting disabled:false to only properties that are required on their own and not set it for properties that are required conditionally (i.e. n the presence of other properties that aren't required). To support this I needed to limit looking for 'required' w/in a key's path to just the key's properties path. So

_.get(contentObj[URLENCODED], ['schema', 'properties', key, 'required'], false);

here

becomes

_.get(contentObj[URLENCODED], ['schema', 'properties', key, 'properties', 'required'], false);

Now when calculating the required property for a given key, look both for the required property within the key's path as as well for the key within the schema_requirements hash, i.e.:

required_on_param = ( _.get(contentObj[URLENCODED], ['schema', 'properties', key, 'properties', 'required'], false) || _.includes(schema_requirements, key));

Lastly I had to add the required value for the key within the encoding hash.

Also needed to remove the logic for prepending "(Required") to descriptions, as it's duplicated later on, causing the descriptions to start with "(Required) (Required)".

@VShingala VShingala self-assigned this Mar 4, 2021
@VShingala VShingala added bug Something isn't working request-body labels Mar 4, 2021
@VShingala
Copy link
Member

@dawn-stripe Thanks for reporting the issue and investigation.! I have verified that the issue exists and we will fix this soon.

The specification mentioned by you seems to be wrong, as defined by OAS required keyword can only be an array and should not be defined for a particular property. source (1), (2). I believe keeping required at schema level is the correct way.

As for the fix, we will include a correct description and similar functionality of disableOptionalParameters option happening to parameters.

@dawn-stripe
Copy link
Author

wonderful thank you @VShingala . And I can believe I misread the OAS as I'm still getting up to speed in it.

@dev1702ed
Copy link

Heyy, @VShingala can I contribute?

@VShingala
Copy link
Member

@dev1702ed This is fixed already so no need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working request-body
Projects
None yet
Development

No branches or pull requests

3 participants