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 case where query parameters are not added #61

Merged
merged 2 commits into from
May 17, 2024

Conversation

raguiar9080
Copy link
Collaborator

@raguiar9080 raguiar9080 commented May 17, 2024

We've introduced query parameters in the generator by adding them as normal parameters in the request body for our users. This was done by simply pushing these parameters to the data field the users use to send the necessary data to the endpoint.

However, there was an edge case. Since we first process the query params and then the requestBody, for cases where the requestBody was a anyOf/allOf we were fully substituting the parameters to be the requestBody, overwriting the query parameters that were previously added. To fix this, simply inverse the order of operations, first add the requestBody to the parameters and then process query parameters. This made some existing tests change the order of the parameters inside the object but resulting in the same data in the end.

We've introduced query parameters in the generator by adding them as
normal parameters in the request body for our users. This was done by
simply pushing these parameters to the `data` field the users use to
send the necessary data to the endpoint.

However, there was an edge case. Since we first process the query params
and then the requestBody, for cases where the requestBody was a
`anyOf`/`allOf` we were fully substituting the parameters to be the
requestBody, overwriting the query parameters that were previously
added. To fix this, simply inverse the order of operations, first add
the requestBody to the parameters and then process query parameters.

Signed-off-by: Ruben Aguiar <[email protected]>
@raguiar9080 raguiar9080 requested a review from a team May 17, 2024 14:15
@raguiar9080 raguiar9080 self-assigned this May 17, 2024
Copy link
Member

@rbruggem rbruggem left a comment

Choose a reason for hiding this comment

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

Looks ok

@raguiar9080
Copy link
Collaborator Author

Might have to rework things a bit since the change in order seems to be affecting other cases 😩

Signed-off-by: Ruben Aguiar <[email protected]>
@raguiar9080
Copy link
Collaborator Author

Fixed now. The query parameters are added at the end to prevent any issues. This made some existing tests change the order of the parameters inside the object but resulting in the same data in the end.

@raguiar9080 raguiar9080 merged commit b132649 into master May 17, 2024
1 check passed
@raguiar9080 raguiar9080 deleted the query-params-with-any-off-request branch May 17, 2024 14:57
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.

2 participants