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

Add Vet endpoint does not work as described in the OpenAPI spec #103

Closed
stevenvanophem opened this issue Nov 26, 2022 · 6 comments
Closed

Comments

@stevenvanophem
Copy link

The OpenAPI spec describes the POST vet endpoint payload with the example

{ "firstName": "James", "lastName": "Carter", "specialties": [ { "name": "radiology" } ] }

This does not work. The endpoint only works when also passing the id of an existing specialty.

I think the correct way to model the payload of this endpoint would be to pass a list of specialty ids, but that would break all the existing clients.

@arey arey added the bug label Dec 23, 2022
@arey
Copy link
Member

arey commented Dec 23, 2022

By removing the readOnly: true from the id property of the Specialty, the Swagger UI is working.
This adds side effect on the speciality API.
I don't know how to enhance the API design.

@alexandre-touret
Copy link
Contributor

Hi,
It depends on if you want to avoid or not duplicate Specialty entries in your database.
You can remove the id in the DTO and always create new specialties...

I don't know about potential functional side effects w/ the duplication of these data

@arey
Copy link
Member

arey commented Jul 7, 2023

I prefer to share the specialities between vets and don't duplicate this static data used in drop down list.

@alexandre-touret
Copy link
Contributor

alexandre-touret commented Jul 8, 2023

OK
I think, you should remove the IDs from the DTO and search for existing specialties before creating them during Vet Creation

@stevenvanophem
Copy link
Author

Vet and Specialty are two different aggregates for the reason that you can register a specialty independent of a Vet. They should be linked by their ids in my opinion.

@arey
Copy link
Member

arey commented Nov 26, 2023

See #125

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

No branches or pull requests

3 participants