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

[REQ][typescript-angular]: support for object as query parameters #4407

Merged

Conversation

aanno2
Copy link
Contributor

@aanno2 aanno2 commented Nov 7, 2019

fixes #4404

@macjohnny: Thank you for review

I've updated the PR and hope it will be more in-line with what is expected.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @nicokoenig @topce @akehir

issue by @aanno2

@auto-labeler
Copy link

auto-labeler bot commented Nov 7, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@macjohnny
Copy link
Member

Thanks for your PR
Please follow the steps described in https://github.com/OpenAPITools/openapi-generator/blob/master/.github/PULL_REQUEST_TEMPLATE.md

@aanno2
Copy link
Contributor Author

aanno2 commented Nov 7, 2019

BTW, travis dies with 'A tuple type element list cannot be empty' but my local build with maven has not reported a problem. I'm not sure how to reproduce the error locally.

@aanno2 aanno2 force-pushed the feature/aanno2-tsng-fix-query-params-master branch 2 times, most recently from 9df82cf to da8c203 Compare November 13, 2019 15:17
@aanno2
Copy link
Contributor Author

aanno2 commented Nov 13, 2019

I have updated and merge-squashed my PR with an appropriate author email. No 'real' changes.

@aanno2 aanno2 force-pushed the feature/aanno2-tsng-fix-query-params-master branch from da8c203 to 4a96e2d Compare November 13, 2019 15:46
@macjohnny macjohnny added this to the 4.2.1 milestone Nov 14, 2019
@aanno2
Copy link
Contributor Author

aanno2 commented Nov 14, 2019

@macjohnny: Thank you for accepting my contribution.

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

LGTM

@macjohnny
Copy link
Member

macjohnny commented Nov 14, 2019

please merge the current master into your branch and update the samples again

@wing328 wing328 modified the milestones: 4.2.1, 4.2.2 Nov 15, 2019
@aanno2
Copy link
Contributor Author

aanno2 commented Nov 15, 2019

Introduced a typo before I run out of time yesterday. All fixed now.

"version": "1.0.0",
"description": "OpenAPI client for @openapitools/typescript-angular-petstore",
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be reverted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Not done on purpose. Sorry!

@wing328 wing328 modified the milestones: 4.2.2, 4.2.3 Dec 2, 2019
@macjohnny
Copy link
Member

@aanno2 can you please merge the current master and update the samples?

@dougal83
Copy link
Contributor

@aanno2 Thanks for this pull request. Just run into the issue myself. All set for next release?

@greghopkins
Copy link

Would love for this to get merged soon... 🙏 😉

@wing328
Copy link
Member

wing328 commented Jan 14, 2020

@aanno2 please resolve the merge conflicts and merge the latest master into this branch. Let me know if you need help on this.

@aanno2 aanno2 force-pushed the feature/aanno2-tsng-fix-query-params-master branch from 7102035 to e461319 Compare January 22, 2020 14:35
@aanno2
Copy link
Contributor Author

aanno2 commented Jan 22, 2020

@wing328: As requested, I have rebased the PR onto the current master branch.

@macjohnny
Copy link
Member

@aanno2 can you please fix the build error:

ERROR in builds/default/api/pet.service.ts(78,27): error TS1122: A tuple type element list cannot be empty.
builds/default/api/store.service.ts(64,27): error TS1122: A tuple type element list cannot be empty.
builds/default/api/user.service.ts(64,27): error TS1122: A tuple type element list cannot be empty.

see https://travis-ci.org/OpenAPITools/openapi-generator/builds/640473532?utm_source=github_status&utm_medium=notification

private addToHttpParamsRecursive(httpParams: HttpParams, value: any, key?: string): HttpParams {
if (typeof value === "object") {
if (Array.isArray(value)) {
(value as []).forEach( elem => httpParams = this.addToHttpParamsRecursive(httpParams, elem, key));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(value as []).forEach( elem => httpParams = this.addToHttpParamsRecursive(httpParams, elem, key));
(value as any[]).forEach( elem => httpParams = this.addToHttpParamsRecursive(httpParams, elem, key));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for helping out. I just applied your suggestion.

@macjohnny
Copy link
Member

please re-generate the samples

@kenisteward
Copy link
Contributor

@macjohnny. Has this been released in a version yet? I was legit looking into fixing this but found this or and am super happy haha

@dougal83
Copy link
Contributor

dougal83 commented Mar 7, 2020

@macjohnny. Has this been released in a version yet? I was legit looking into fixing this but found this or and am super happy haha

Yes, it is in the latest version.

@spencerfontein
Copy link

looks like this was merged to 4.3.0, but the latest available on npm is 4.2.3 https://www.npmjs.com/package/@openapitools/openapi-generator-cli. Is there a way I can use the fix ?

@dougal83
Copy link
Contributor

looks like this was merged to 4.3.0, but the latest available on npm is 4.2.3 https://www.npmjs.com/package/@openapitools/openapi-generator-cli. Is there a way I can use the fix ?

Are you sure? I thought that the milestone on the right was an at-a-glance indicator and it says 4.2.3. Also, if I recall correctly then I've used this feature from the current npm version. Where are you seeing 4.3.0?

@spencerfontein
Copy link

I clicked on this link #5174 that daniel-frank posted a few comments back and it mentioned 4.3.

I am currently using 4.2.3

I have the Pagination object in my Java code as a query param and this is the TS I get

import { Sort } from'./sort';
exportinterfacePageable {
offset?: number;
sort?: Sort;

pageSize?: number;
pageNumber?: number;
paged?: boolean;
unpaged?: boolean;
}
export interface Sort {
    sorted?: boolean;
    unsorted?: boolean;
    empty?: boolean;
}

Maybe I'm missing something

This is the command I'm running

openapi-generator generate -g typescript-angular -i ../../backend/escalation-service/target/openapi.json -o swagger/escalation-api --additional-properties npmName=@mycustom/escalation-api,ngVersion=8.1.2,npmVersion=1.0.0

Thanks for the help

@dougal83
Copy link
Contributor

I think it would be best to open a new issue to query this and include your 'openapi.json'. Hopefully, someone can point out the issue.

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] [typescript-angular]: support for object as query parameters
8 participants