Skip to content

Fix isBodyAllow to return true in case of DELETE method#10751

Merged
wing328 merged 5 commits intoOpenAPITools:masterfrom
topce:delete-body-angular
Nov 3, 2021
Merged

Fix isBodyAllow to return true in case of DELETE method#10751
wing328 merged 5 commits intoOpenAPITools:masterfrom
topce:delete-body-angular

Conversation

@topce
Copy link
Copy Markdown
Contributor

@topce topce commented Nov 1, 2021

fix
#7557
#10448
fix isBodyAllow to return true in case of DELETE method
add new method isMethodPutOrPatchOrPost

fix angular typescript client to allow delete with body
if angular version allow it use isBodyAllow in template
otherwise use isMethodPutOrPatchOrPost
angular fix client in version12.1
change default angular version to be latest
in this moment 12.2.12

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

technical committee:
@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)

Hi @wing328 if you have time can check this (with other maintainers) because
I change templates in other libs
however tried not to break anything

topce added 5 commits November 1, 2021 11:38
@topce
change isBodyAllowed to return true
8a8491b 
for delete method
Add method
isMethodPutOrPatchOrPost
@topce
change templates to use isMethodPutOrPatchOrPost
eebf2fb 
to be compatible with previous versions,
lubrary mainterners should decide
if want to use isBodyAllowed
@topce
since angular 12.1 delete with body is supported
bc766c3 
Change angular typescript to support delete with body if
angular version is >= 12.1
update default  ngVersion to be latest
12.2.12
@topce
generate samples and docs
dd77400
@topce
ApiClient was OK
df10442 
revert my changes
@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 3, 2021

cc @OpenAPITools/generator-core-team as the change covers CodegenOperation.java

@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 3, 2021

LGTM

@wing328 wing328 merged commit edb88d9 into OpenAPITools:master Nov 3, 2021
@wing328 wing328 changed the title Delete body angular Fix isBodyAllow to return true in case of DELETE method Nov 3, 2021
cghislai added a commit to cghislai/openapi-generator that referenced this pull request Nov 27, 2021
@cghislai
Revert "Delete body angular (OpenAPITools#10751)"
e73bc68 
This reverts commit edb88d9.
@wing328 wing328 added this to the 5.3.1 milestone Dec 18, 2021
@dartzki dartzki mentioned this pull request Dec 30, 2021
Closed
6 tasks
macjohnny pushed a commit that referenced this pull request Jan 31, 2022
@cghislai
Revert supports for delete body in typescript-angular client (#10976)
95c996f 
* Update generated angular 12 client version

* Revert "Delete body angular (#10751)"

This reverts commit edb88d9.

* Update generated files
@angelaki
Copy link
Copy Markdown
Contributor

angelaki commented Aug 26, 2022

Hey, I'm using openapi-generator-cli to generate my Angular code. Right now I'm strugging with DELETE not containing my body. Seams like this was pretty muched discussed - but implemented? What am I doing wrong? Isn't this fix included in the cli, yet?

docker run openapitools/openapi-generator-cli generate -i swagger.json -g typescript-angular -p=useSingleRequestParameter=true -o ./out --skip-validate-spec

@macjohnny
Copy link
Copy Markdown
Member

macjohnny commented Aug 26, 2022

@angelaki it was reverted in #10976

@angelaki
Copy link
Copy Markdown
Contributor

angelaki commented Aug 28, 2022

@angelaki it was reverted in #10976

Oh, too bad. But thank you. Is there right now any way to generate a DELETE-method containing a body? Was there a released version supporting it? And: why wasn't just the request method used?

@angelaki
Copy link
Copy Markdown
Contributor

angelaki commented Sep 5, 2022

@angelaki it was reverted in #10976

Sorry for bumping, but I'd really need this feature. Since I haven't found any pending issues, does this work as expected right now? Can I maybe help here fixing this issue?

@macjohnny
Copy link
Copy Markdown
Member

macjohnny commented Sep 5, 2022

@angelaki sure, you can file a new PR. Please keep in mind the discussion in #10976, ie a flag to enable this behavior would probably be good

@angelaki
Copy link
Copy Markdown
Contributor

angelaki commented Sep 5, 2022

@macjohnny do you maybe remember the reason request isn't used instead of get / post / patch ...? Not just for delete but in general? So the generated code would be less dependent on Angular changes?

@macjohnny
Copy link
Copy Markdown
Member

macjohnny commented Sep 5, 2022

@angelaki unfortunately I dont remember, but maybe you can find some previous PRs where this was discussed.

@amakhrov amakhrov mentioned this pull request Sep 17, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Client: TypeScript Feature: Generator Issue: Bug

Projects

None yet

Milestone

5.3.1

Development

Successfully merging this pull request may close these issues.

4 participants

@topce @wing328 @angelaki @macjohnny