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

docs(api-sdk): enums specific properties use-case #3013

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vscaiceanu-1a
Copy link
Member

@vscaiceanu-1a vscaiceanu-1a requested a review from a team as a code owner March 17, 2025 15:23
Copy link

nx-cloud bot commented Mar 17, 2025

View your CI Pipeline Execution ↗ for commit a453146.

Command Status Duration Result
nx run-many --target=test-int ✅ Succeeded 2s View ↗
nx run-many --target=test-e2e ✅ Succeeded 2m 23s View ↗
nx run-many --target=build --projects=eslint-pl... ✅ Succeeded <1s View ↗
nx run-many --target=publish --nx-bail --userco... ✅ Succeeded 31s View ↗
nx run-many --target=prepare-publish --exclude-... ✅ Succeeded 7s View ↗
nx run-many --target=documentation ✅ Succeeded 1m 21s View ↗
nx affected --target=test --collectCoverage --c... ✅ Succeeded <1s View ↗
nx run ama-sdk-schematics:build-swagger ✅ Succeeded 6s View ↗
Additional runs (3) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-03-19 08:22:10 UTC

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 17, 2025
@vscaiceanu-1a vscaiceanu-1a force-pushed the docs/swagger-enum-inheritance branch from e613d8e to e0b7c14 Compare March 17, 2025 15:41
@@ -147,6 +147,43 @@ export function revivePet<T extends Pet = Pet>(data: any): undefined | T | Cat |
> The discriminator needs to be of enum type as the string format would be too generic to map the accepted
> value to the supported models.

### Enum specific properties

If the a specific property is of type enum, make sure to explicitly declare the type object.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect to always have the type object for allOf elements that are not ref
We discovered the issue concerning enum but maybe there are others
https://swagger.io/docs/specification/v3_0/data-models/oneof-anyof-allof-not/#allof
looking at there documentation they always specify the type object

Copy link
Contributor

Choose a reason for hiding this comment

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

If you check on https://editor.swagger.io/ it seems that

Dog:
  allOf:
    - $ref: '#/components/schemas/Pet'
    properties:
      bark:
        type: boolean
      breed:
        type: string
        enum: [Dingo, Husky, Retriever, Shepherd]

is not even valid

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I missed the indent in the docs.

Dog:
  allOf:
  - $ref: '#/components/schemas/Pet'
  properties:
    bark:
      type: boolean
    breed:
      type: string
      enum: [Dingo, Husky, Retriever, Shepherd]

is valid.

@@ -147,6 +147,43 @@ export function revivePet<T extends Pet = Pet>(data: any): undefined | T | Cat |
> The discriminator needs to be of enum type as the string format would be too generic to map the accepted
> value to the supported models.

### Enum specific properties
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not reduce the case to enums.
I wonder if it would not make more sense to have a file dedicated to cases that we do not support but have a workaround.
Here it is mainly the properties to a allOf instruction.

@vscaiceanu-1a vscaiceanu-1a force-pushed the docs/swagger-enum-inheritance branch from e0b7c14 to d3d8fb9 Compare March 18, 2025 07:00
@vscaiceanu-1a vscaiceanu-1a force-pushed the docs/swagger-enum-inheritance branch 2 times, most recently from 5d62964 to b9000cb Compare March 18, 2025 15:48
@vscaiceanu-1a vscaiceanu-1a force-pushed the docs/swagger-enum-inheritance branch from b9000cb to a453146 Compare March 19, 2025 08:14
export type BreedEnum = 'Dingo' | 'Husky' | 'Retriever' | 'Shepherd';
```

The issue is tracked via https://github.com/AmadeusITGroup/otter/issues/3017.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The issue is tracked via https://github.com/AmadeusITGroup/otter/issues/3017.
> [!INFO]
> The issue is tracked via https://github.com/AmadeusITGroup/otter/issues/3017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Enum wrongly generated in SDK models when used in allOf
5 participants