goat-community / goat

This is the home of Geo Open Accessibility Tool (GOAT)
GNU General Public License v3.0
89 stars 47 forks source link

Update endpoints to allow for delete-many #1438

Closed EPajares closed 1 year ago

EPajares commented 1 year ago

Goal of this issue

Allow deleting many features at the same time using our existing endpoints. A list of ids should be passed and accordingly the features be deleted. Most of the delete endpoints would be affected. Though there are some exceptions. Delete many should be implemented for the following endpoints:

image image image image image image

Resources

Existing code from the mentioned endpoints.

Deliverables

Delete many functionalities for the mentioned endpoints. Adjust tests.

Branch to derive

Dev

metemaddar commented 1 year ago

I think it would be good to add this functionality to CRUD_BASE.delete() so that it can take also list as input id. Also we would change the input type to Union[List[Int],Int] at delete endpoints.

EPajares commented 1 year ago

I guess this should not break anything, right?

metemaddar commented 1 year ago

Yes, I think it won't break anything. Because we can check the input type at Base.delete() before perform delete. I should check more at endpoint procedure. A question is what should we do if there is some already deleted id in input list. As we raise 404 when the input id is not present at the table. Maybe we shouldn't raise 404. Because when the object is not present, it means maybe it is deleted before. There is a discussion at StackOverFlow that suggest not to raise 404 (at comments).

Image

metemaddar commented 1 year ago

The RFC also says we should raise 204 (No content) when the object is not present at table (ref):

If a DELETE method is successfully applied, the origin server SHOULD send a 202 (Accepted) status code if the action will likely succeed but has not yet been enacted, a 204 (No Content) status code if the action has been enacted and no further information is to be supplied, or a 200 (OK) status code if the action has been enacted and the response message includes a representation describing the status.

Of course it doesn't say about deleting multiple objects, if some of them are not present. If some of them are present, the request should be accepted (202) and if some of them not present (204?) However it says the response can be 200 + a message that describes the status.

metemaddar commented 1 year ago

I suggest at the moment to

metemaddar commented 1 year ago

I guess this should not break anything, right?

I think it can. Because the base.remove() method returns the deleted object, and the returned object is used in some places. I think we would create remove_multi() without any returned object. Or maybe just return deleted ids.

metemaddar commented 1 year ago

The endpoint: scenarios/{scenario_id} is used for Goat Client. Should be refactor it as well? @EPajares