search

osmlab / to-fix-backend

The to-fix server
BSD 3-Clause "New" or "Revised" License
15 stars 13 forks source link

Archive things #246

Closed batpad closed 6 years ago

batpad commented 6 years ago

Adds DELETE endpoints to projects and items which sets an is_archived property on them and removes them from list results.

cc @kepta

kepta commented 6 years ago

@batpad the PR looks great and we should totally merge it!

As a side note, I wanted discuss the long term implications of using “DELETE” to not actually delete , but instead add a property archive to a resource.

What really confuses me is that why is archive given a special power ?

What if we go ahead with adding one more property (in this case archive) which can be set by calling the update endpoint. To me this approach would seem more consistent and easier for someone in the future reading the code.
Somewhat related is a todo list, where there is a distinction between archiving and deleting, you wouldn't wanna press delete just to see it get archived Thoughts ?

batpad commented 6 years ago

@kepta really thoughtful notes, thanks.

My initial thinking was to try and keep the fact that this was a soft delete in the database completely transparent / immaterial to the client, who could treat it like a regular DELETE op.

I am seeing potential problems with this approach, especially on places where a client may try to POST an item with an id of something that has been "deleted" and would now get duplicate errors.

I think we should think about that a bit, and perhaps move to making this a regular update operation.

I would really like to see though if we can make this as transparent for clients as possible - I'm going to go ahead with merging this for now - and I think we can change the implementation to be an update and deprecate the DELETE endpoint down the line, if we feel this is getting problematic / confusing for clients and item creators.