-
Notifications
You must be signed in to change notification settings - Fork 878
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
Cannot POST to /petclinic/api/pets #139
Comments
Having 2 endpoints to add a pet to an existing owner may be discussed. I'm not sure there's any point to it. Technically, the Maybe we have to remove the |
Hi @arey |
Hi, If in a functional point of view, creating Pet without any ownerId doesn't mean a thing, I think the solution proposed by @breedx-splk would be OK. In my view, regarding the constraints of the schema, having two ways to create a Pet is weird. |
Thanks Alexandre for your feedback. Let's simplify the API by removing the POST to the |
@arey Sorry I don't have time to pick this up in the near future. Thanks for the consideration. |
It used to be that
POST
ing to/petclinic/api/pets
was a reasonable and functioning way to create a pet. This is simply no longer the case. It doesn't work.The
Pet
model object has anowner
field of typeOwner
but the DTOs don't seem to wire this up correctly from the input JSON, and when we reachthis.clinicService.savePet(pet);
in thePetController
, the call fails because the db requires a non-null foreign key forowner.id
. This change has broken existing API clients, such as my k6 test harness that I use to drive traffic.The example(s) in the swagger UI also don't work and result in a 400.
The current workaround is to POST to
/petclinic/api/owners/{id}/pets
instead...which is fine, but there shouldn't be two apis that apparently do the same thing (create a pet associated with an owner).I don't know enough about this mapstruct magic to propose a solution that fixes the current dto shortcoming, but an alternative approach would be to just remove
POST
from/petclinic/api/pets
from the public API surface.The text was updated successfully, but these errors were encountered: