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

fix(editor): retrieve parameter default value from dereferenced spec #213

Conversation

ostridm
Copy link
Contributor

@ostridm ostridm commented Sep 20, 2023

closes #212

@ostridm ostridm added the Type: bug Something isn't working. label Sep 20, 2023
@ostridm ostridm requested a review from pmstss September 20, 2023 07:19
@ostridm ostridm self-assigned this Sep 20, 2023
@codeclimate
Copy link

codeclimate bot commented Sep 20, 2023

Code Climate has analyzed commit 92deeb5 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.7% (0.0% change).

View more on Code Climate.

@ostridm ostridm force-pushed the fix-#212/retrieve-parameter-default-value-from-dereferenced-spec-instance branch 2 times, most recently from a7aa43e to 8acb8b2 Compare September 20, 2023 07:35
Copy link
Contributor

@pmstss pmstss left a comment

Choose a reason for hiding this comment

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

This PR fixes two pretty different issues:

  1. parseBodyParameter() behavior in case of default value stored in referenced definition, as reflected in title. This part looks good.
  2. unhandled root-level consumes briefly mentioned in issue description. This part is not covered in fixtures, as imo at least three cases must be there:
  • root level consumes and no operation-level consumes
  • root level consumes overriding with non-empty operation-level consumes
  • root level consumes overriding with empty operation-level consumes; according to spec - "An empty value MAY be used to clear the global definition".

Could you please rework this PR to cover only first intention and prepare new issue and PR to cover the second intention?

@ostridm ostridm force-pushed the fix-#212/retrieve-parameter-default-value-from-dereferenced-spec-instance branch from 8acb8b2 to 92deeb5 Compare September 20, 2023 10:54
Copy link
Contributor

@pmstss pmstss left a comment

Choose a reason for hiding this comment

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

thanks for adjustments; lgtm

@ostridm ostridm merged commit 18888f0 into master Sep 20, 2023
4 checks passed
@ostridm ostridm deleted the fix-#212/retrieve-parameter-default-value-from-dereferenced-spec-instance branch September 20, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: bug Something isn't working.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OAS: Body parameter default value could not be retrieved
2 participants