Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[REQ] [Typescript] Support DELETE Payload #13380

Closed
angelaki opened this issue Sep 8, 2022 · 17 comments · Fixed by #13457
Closed

[REQ] [Typescript] Support DELETE Payload #13380

angelaki opened this issue Sep 8, 2022 · 17 comments · Fixed by #13457

Comments

@angelaki
Copy link
Contributor

angelaki commented Sep 8, 2022

Like discussed in #10976 the possibility to add a payload to the DELETE method was removed. I'd suggest to completely switch to Angular HttpClient's request method (instead of get / post / put e.g.). All the other methods are just overloads to the request method.

This way the generator would be less dependent on Angulars API. I hope this is easy to implement for someone here knowing the mustache templates? I've checked it but it pretty much felt like I'd need some more knowledge of the generator's code.

@cmalard
Copy link

cmalard commented Sep 12, 2022

Oh gosh, this reminds me something 🤦 😅 😂
swagger-api/swagger-codegen-generators#568

@angelaki
Copy link
Contributor Author

Was anything wrong with your PR? Why is Codegen back to post / put / ... ?

@cmalard
Copy link

cmalard commented Sep 12, 2022

I guess something was lost with all the forks & merge when OpenApiTools & SwaggerApi were not yet aligned 😅

@angelaki
Copy link
Contributor Author

angelaki commented Sep 12, 2022

So you obviously totally agree on changing the method used? Could you maybe patch it again? I would be endlessly thankfull, pretty much need a DELETE body.

@angelaki
Copy link
Contributor Author

Hey @cmalard, any chance you could fix this, soon? 🙄 It seams like many issues were caused by just not using the request method (fixed DELETE method forth and back and so on ...) plus I'd really need it these days. And I'm actually quite scred I'd need to do this myself ... 😂🙈

@cmalard
Copy link

cmalard commented Sep 14, 2022

Hey @angelaki 🙋 I will also need it in the next weeks (again for another project 😂), if you want it asap, don't wait for me & feel free to do it 😉 🍻
-> https://github.com/swagger-api/swagger-codegen-generators/pull/568/files
-> https://github.com/swagger-api/swagger-codegen-generators/pull/576/files (bugs introduced above fixed ^^")

@amakhrov
Copy link
Contributor

I'll take a stab at implementing it (via the "HttpClient#request" method under the hood) over the weekend

@angelaki
Copy link
Contributor Author

Thank you so much! Reading your PR I just noticed that @cmalard initial fix was actually for SwaggerCodGen, not for OpenAPIGenerator 😂. Never the less it's fixed now, great! :)

When while this fix probably make it to the Docker CLI?

@amakhrov
Copy link
Contributor

@angelaki you can wait for the 6.1.1 release (planned for Oct 11). Or you can use the latest tag of the docker image (looks like it's already updated): https://hub.docker.com/layers/openapitools/openapi-generator-cli/latest/images/sha256-c73d1c3d9612b29d691ca95ee5cf80a7e6ae9cb04033e7b283bb0ed49bfdadc1?context=explore

@angelaki
Copy link
Contributor Author

angelaki commented Sep 19, 2022

@amakhrov the latest version

$ docker run openapitools/openapi-generator-cli version
6.0.0-SNAPSHOT

still seams to generate using the old methods.

@macjohnny
Copy link
Member

@angelaki I think it should be in the 6.1.1 snapshot: https://github.com/OpenAPITools/openapi-generator#11---compatibility

@amakhrov
Copy link
Contributor

amakhrov commented Sep 19, 2022

@angelaki

docker run openapitools/openapi-generator-cli

this runs whatever version already exists in your cache (or downloads one if it's the first time use).
you can enforce specific version or hash of an image:

# by tag -  might still not be up to date if you already have an older version in your local cache
docker run openapitools/openapi-generator-cli:latest version

# by hash - always the exact version
docker run openapitools/openapi-generator-cli@sha256:c73d1c3d9612b29d691ca95ee5cf80a7e6ae9cb04033e7b283bb0ed49bfdadc1 version

@angelaki
Copy link
Contributor Author

angelaki commented Sep 20, 2022

@amakhrov I was actually running latest, just simplified it. Maybe I shouldn't have. Never the less, latest is version 6.0.0-SNAPSHOT ... for me?

Edit: I actually needed to manually pull it. I thought Docker always checks for the latest version with the tag provided? Interesting ...

@amakhrov
Copy link
Contributor

@angelaki did everything work for you then?

@angelaki
Copy link
Contributor Author

@amakhrov yes, looks pretty good, thank you so much!

@angelaki
Copy link
Contributor Author

Completly off-topic, but maybe you can answer this question: how to retrieve blob data via generated client? All the overloads only accept httpHeaderAccept?: 'text/plain' | 'application/json' | 'text/json' while the method looks like this:

let responseType_: 'text' | 'json' | 'blob' = 'json';
if (localVarHttpHeaderAcceptSelected) {
    if (localVarHttpHeaderAcceptSelected.startsWith('text')) {
        responseType_ = 'text';
    } else if (this.configuration.isJsonMime(localVarHttpHeaderAcceptSelected)) {
        responseType_ = 'json';
    } else {
        responseType_ = 'blob';
    }
}

having this swagger json:

 "200": {
    "description": "Success",
    "content": {
        "application/octet-stream": {
        "schema": {
            "$ref": "#/components/schemas/DownloadResult"
        }
        }
    }
},

Cannot call it with 'blob' since no overload accepts this. Create a new issue? Guess someone would have noticed yet, if this wasn't possible? 😅

@amakhrov
Copy link
Contributor

Not sure what exactly you try to do.
Perhaps the Slack workspace would be a better place to ask: https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

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

Successfully merging a pull request may close this issue.

4 participants