🐛 Bug Report: Wrong request body on create update and delete m2m nested entity

[abrl91]

What happened?

The data-service-generation package generates the wrong endpoints for m2m nested relation (for example Order and Product).
This bug includes few things that we need to discuss on:

1. one of the method is called createProduct, while the action in the UseRole is update' UPDATE /api/:id/product`
2. The DTO on the request body seems to be wrong
3. The resource and the action name seem to be wrong too

This happen also for updateProduct and deleteProduct on the order.controller.base.ts

What you expected to happen

It doesn't make sense that the request param and the request body are the same entity, the request body should be a dto to create a product on an order / connect an existing product to an order

After we decided on how to implement this (create/connect), according to our decision we would have to change the action and the resource on the UseRoles decorator.

How to reproduce

generate a sample app with amplication and make order and product to be m2m relation

• fix the function req body
• fix the function of m2m nested API endpoint in a way that it will fit the function name
• fix the resource on the UseRoles decorator in a way that it will fit the function name
• fix the action on the UseRoles decorator in a way that it will fit the function name
• check swagger annotation

Amplication version

0.12.4

Environment

node v16.3.0
npm 7.15.1
macOS

Are you willing to submit PR?

Yes I am willing to submit a PR!

[EugeneTseitlin]

After the discussion it was decided that the behavior is correct. Resource in header that used for permissions should stay a parent entity, action is supposed to be 'update' for 'connect', 'disconnect' and 'update' methods. Names of methods were adjusted accordingly to expected behavior.

[abrl91]

@EugeneTseitlin @mshidlov @yuval-hazaz

something doesn't make sense.
This is a screenshot from swagger showing the expected request body for a simple patch request (by simple I mean not nested or in other words: "update order by id")
it already has a product property in which the user can:
connect, disconnect and set a product to a specific order.
Therefore, it already covers the endpoints mentioned on this issue and their purpose is to
create, update and delete a product while connect, set or disconnect it from an order

[yuval-hazaz]

their purpose is to
create, update and delete a product while

This is not true. These properties only accept an ID so it will not create, update and delete a product

[abrl91]

their purpose is to
create, update and delete a product while

This is not true. These properties only accept an ID so it will not create, update and delete a product

@yuval-hazaz
But this is exactly what I mean. I used create, update and delete because this is what we call it.
The endpoint on the screenshot already do want the endpoints mentioned on this issue should be regarding the last comment of @EugeneTseitlin that mentioned that those endpoints will not create update and delete, they will do connect, disconnect and set

[yuval-hazaz]

what is your claim? what do you expect?

[abrl91]

why do we need 3 different endpoints that do: set, connect, disconnect, and one endpoint that do set+connect+disconnect (the one in the screenshot)

[yuval-hazaz]

what is your claim? what do you expect?

[abrl91]

that those 3 endpoints, mentioned on the issue would actually also create, update and delete and not just set connect and disconnect or - remove them

[yuval-hazaz]

Got it, thanks!
We looked into it yesterday and also considered these 2 options.

The endpoint /api/order/{id}/product endpoint exists since day 1 - so :

1. Initially, there was no support for the connect, disconnect, and set properties on the parent object. It was just added recently.
2. There is no trigger that should make us change the current implementation. No one reported an issue, no one asked or raised a question about the current implem. The impact of any change will be zero in the best case and negative in all other cases.
3. To create, update, and delete the child object - the user may use the /api/product (in your example) endpoints - so we didn't see a reason to include it again.
4. we concluded that the current implementation did not match the name of the function - so we decided to change the name of the function.