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 imports and property name when using anyof/oneof in services #3639

Merged

Conversation

smasala
Copy link
Contributor

@smasala smasala commented Aug 14, 2019

PR checklist

Description of the PR

Fixes request params when using anyOf / oneOf for api services where the param was gerenated as "UNKNOWN_BASE_TYPE" instead of (in TS) Model1 | Model2.

Currently:

import { Model1 | Model2} from '../model/model1Model2';
import { UNKNOWN_BASE_TYPE } from '../model/uNKNOWNBASETYPE';

...

public updateRequest(id: string, UNKNOWN_BASE_TYPE: UNKNOWN_BASE_TYPE, observe?: 'body', reportProgress?: boolean): Observable<EnvelopeOfModels>;

Fix:

import { Model1 } from '../model/model1';
import { Model2 } from '../model/model2';

...

public updateRequest(id: string, model1Model2: Model1 | Model2, observe?: 'body', reportProgress?: boolean): Observable<EnvelopeOfModels>;

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10)

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


if (codegenProperty != null && codegenProperty.getComplexType() != null && codegenProperty.getComplexType().contains(" | ")) {
Copy link
Member

Choose a reason for hiding this comment

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

will it always be surrounded by spaces, i.e. |?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But I must admit, the whole (any/one) of issue needs to be reconsidered as it's more of a hack rather than a sustainable fix. :|

@macjohnny
Copy link
Member

@smasala thanks for your PR!
can you perform a search to find which of the open issues would be closed by this PR?
https://github.com/OpenAPITools/openapi-generator/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+UNKNOWN_BASE_TYPE

@smasala
Copy link
Contributor Author

smasala commented Aug 14, 2019

@macjohnny don't think it fixes any of them

@smasala
Copy link
Contributor Author

smasala commented Aug 15, 2019

@macjohnny don't think the travis fail is due to this commit

@macjohnny macjohnny closed this Aug 15, 2019
@macjohnny macjohnny reopened this Aug 15, 2019
@macjohnny
Copy link
Member

@smasala I restarted CI

@macjohnny macjohnny closed this Aug 15, 2019
@macjohnny macjohnny reopened this Aug 15, 2019
@macjohnny
Copy link
Member

@smasala somehow the travis build keeps failing. could you please merge the most recent master?

@macjohnny macjohnny added this to the 4.1.1 milestone Aug 15, 2019
@macjohnny macjohnny merged commit 6f7d279 into OpenAPITools:master Aug 15, 2019
@smasala smasala deleted the smasala/anyof-oneof-fix-services branch August 15, 2019 10:49
@wing328 wing328 changed the title fix imports and property name when using anyof/oneof in services Fix imports and property name when using anyof/oneof in services Aug 26, 2019
@wing328
Copy link
Member

wing328 commented Aug 26, 2019

FYI @OpenAPITools/generator-core-team

@wing328
Copy link
Member

wing328 commented Aug 26, 2019

@smasala thanks for the PR, which has been included in the v4.1.1 release: https://twitter.com/oas_generator/status/1165944867391860737

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.

None yet

3 participants