Skip to content

[kotlin][client] respect sortModelPropertiesByRequiredFlag#5211

Merged
wing328 merged 1 commit intoOpenAPITools:masterfrom
4brunu:feature/kotlin-model-parameters-order
Feb 13, 2020
Merged

[kotlin][client] respect sortModelPropertiesByRequiredFlag#5211
wing328 merged 1 commit intoOpenAPITools:masterfrom
4brunu:feature/kotlin-model-parameters-order

Conversation

@4brunu
Copy link
Copy Markdown
Contributor

@4brunu 4brunu commented Feb 5, 2020

#4707 introduced a flag sortModelPropertiesByRequiredFlag to control how the model orders the properties to fix #4705.

By default the model properties already are sorted by the required here

And the user can choose if he wants to sort the model properties already by the required or not here

cliOptions.add(new CliOption(CodegenConstants.SORT_MODEL_PROPERTIES_BY_REQUIRED_FLAG, CodegenConstants.SORT_MODEL_PROPERTIES_BY_REQUIRED_FLAG_DESC));

With #4453 this become non functional.

This PR restores this functionality.

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.

@jimschubert (2017/09) ❤️, @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11)

Copy link
Copy Markdown
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

The fix looks reasonable to me

@wing328 wing328 added this to the 4.3.0 milestone Feb 13, 2020
@wing328 wing328 merged commit 9475556 into OpenAPITools:master Feb 13, 2020
@mtraynham
Copy link
Copy Markdown
Contributor

mtraynham commented Feb 20, 2020

@4brunu @wing328 , this change has effectively removed all inherited properties from child classes.

In the template, vars does not contain parentVars it only contains the immediate child vars. E.g. if I have a model that inherits property a from a parent class using allOf, a is not included in the vars list, but is included in the requiredVars and optionalVars lists.

Maybe instead it should be allVars, not vars.

@4brunu
Copy link
Copy Markdown
Contributor Author

4brunu commented Feb 20, 2020

Maybe we can change vars to allVars and sort allVars.
What do you think @wing328?

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.

[BUG][Kotlin] models properties order change when they become optional's

3 participants