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

#776: Fixed bug for request body not resolving schema when using $ref #830

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

aakankshabhende
Copy link

Fixes #776

Signed-off-by: Aakanksha <aakanksha0407@gmail.com>
Signed-off-by: Aakanksha <aakanksha0407@gmail.com>
@aakankshabhende
Copy link
Author

@VShingala please review my PR

Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

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

@aakankshabhende We'll also need to make changes in libV2/schemaUtils.js file. Current file is only responsible for older v1 APIs.

Also, we should add unit tests for the related fix under test/unit/convertV2.test.js file. You can look at the other unit tests added for other fixes for reference.

@aakankshabhende
Copy link
Author

@VShingala Please correct me if I'm wrong, libV2/shemaUtils.js file already has a function resolveRefFromSchema which seems to resolve the ref.

@VShingala
Copy link
Member

@aakankshabhende It does but it seems actual handling is not considering it.

if (concreteUtils.isBinaryContentType(bodyType, requestContent)) {

Here, the schema passed to the corresponding function should be resolved but it seems it's need resolved first hand. Due to this, we see issues with even v2 APIs.

@aakankshabhende
Copy link
Author

@VShingala I'm new to contributing to this project and am eager to get involved, but I’m having some trouble understanding how to reflect changes in my local dev environment as there is no contribution guide available. Could you please provide some guidance on this?

@VShingala
Copy link
Member

VShingala commented Oct 18, 2024

@aakankshabhende Sure, If you mean setting up local development environment, below is what I usually follow.

  1. Checkout debug/setup branch.
  2. There are 3 files that I've created temporarily named test.js, spec, .vscode/launch.json on this branch. You can avoid committing them in your PR changes.
  3. You can keep pull debug/setup branch into your branch as well.
  4. test.js can consume APIs exposed by this module and generate collection in new file named collection.json. Simply run node test.js from CLI to see it working.
  5. I've also included vscode debug action named Conversion which does the same thing, you can use that to debug properly if you're using VSCode.

how to reflect changes in my local dev environment

I didn't exactly get above part so feel free to add more details if above guide doesn't help you with your question.

@aakankshabhende
Copy link
Author

Thanks a lot for the info, I have made changes in libV2/shemaUtils.js to fix this bug but I'm not able to test it because I'm not able to log anything in the console for this particular libV2/shemaUtils.js file.

@VShingala
Copy link
Member

@aakankshabhende Not sure why you're not getting logs. Are you able to generate the collection from the steps I mentioned?

@aakankshabhende
Copy link
Author

Yes, I'm able to generate logs. Thank you @VShingala

Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

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

@aakankshabhende As discussed, we'll also need changes in schemaUtilsV2 to address the issue in Postman App since it's using v2 APIs, feel free to request a review once you've made the changes.

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.

Request body for binary does not resolve properly when using $ref
2 participants