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

Support optional singleRequestParameter #18467

Merged
merged 1 commit into from
May 3, 2024

Conversation

angelaki
Copy link
Contributor

Fixes #18456. Add optional chaining to singleRequestParameter, if all parameters are optional.

@wing328
Copy link
Member

wing328 commented Apr 24, 2024

https://github.com/OpenAPITools/openapi-generator/actions/runs/8798721887/job/24147516590?pr=18467 already fixed in the master

cc TypeScript technical committee for review

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04)

@wing328
Copy link
Member

wing328 commented Apr 30, 2024

@macjohnny macjohnny merged commit 06b00e6 into OpenAPITools:master May 3, 2024
13 of 14 checks passed
@angelaki
Copy link
Contributor Author

angelaki commented May 6, 2024

Woohoo 🥳 Deployment estimations? Or: when will it hit https://hub.docker.com/r/openapitools/openapi-generator-cli? Doesn't seam to be part of 7.6.0-Snapshot, yet.

@macjohnny
Copy link
Member

Woohoo 🥳 Deployment estimations? Or: when will it hit https://hub.docker.com/r/openapitools/openapi-generator-cli? Doesn't seam to be part of 7.6.0-Snapshot, yet.

i think it should soon be part of the snapshot

@angelaki
Copy link
Contributor Author

angelaki commented May 6, 2024

Woohoo 🥳 Deployment estimations? Or: when will it hit https://hub.docker.com/r/openapitools/openapi-generator-cli? Doesn't seam to be part of 7.6.0-Snapshot, yet.

i think it should soon be part of the snapshot

What kind of soon are you talking about? Minutes? Hours? Days? Thank you! :)

@angelaki
Copy link
Contributor Author

angelaki commented May 6, 2024

So... Shouldn't it be part of the image, yet?

Edit: oh my ... was just my compose file not pulling the latest latest version ... everythings fine, ty!

@wing328 wing328 added this to the 7.6.0 milestone May 20, 2024
@xfh
Copy link

xfh commented Jul 17, 2024

Hi @angelaki, your change doesn't make any sense, because CodegenModel hasRequiredVars is never actually initialized. It's always false. Since your changes, all my singleRequestParameter are generated as optional parameter, even though they do actually have required parameters...

There is another property hasRequiredParams, which might be what you were looking for - at least it works for me. Could you test if that solves your original issue?

@angelaki
Copy link
Contributor Author

Haha... yes, that explains a lot. I lately noticed that all arguments are optional now (what imho is still better than the old behavior, where I had to but an empty object with thousant of call that don't have any mandatory parameters) and wanted to check it these days.

@wing328 did that change and recommended to use that value, maybe ask him?

@qirex
Copy link
Contributor

qirex commented Nov 4, 2024

Hi!

I just ran into the problem that all request params are optional, even if they are marked required in the spec when generating useSingleRequestParameter.
I think this is wrong behavior as by default we are now able to send requests without any parameters even if they are required. And these requests likely will result in an error because the expected parameters are missing.

Are there any plans on fixing this behavior?

@macjohnny
Copy link
Member

@qirex do you want to implement a fix?

@qirex
Copy link
Contributor

qirex commented Nov 6, 2024

I have no clue about the software stack used in this project. So I am not sure if I can be much of help.

@qirex
Copy link
Contributor

qirex commented Nov 10, 2024

@qirex do you want to implement a fix?

Done: #20077

Was not as complicated as I thought. :)

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

Successfully merging this pull request may close these issues.

[REQ] [angular-typescript] Support optional paramters for useSingleRequestParameter=true
5 participants