RESTfull: patch, delete, soft delete

I generally agree with the two pieces of feedback you got, but I also agree with some of the points you make. I don't think they are mutually exclusive.

I would say all of this is true:

• A DELETE request would be inappropriate here.
• Using /delete as an endpoint is confusing.
• A "universal" PATCH endpoint is bad, for the reasons you note.
• Sending redundant information is bad, again as you note.
• A consensus on implicit rules in how our API is constructed is important so it doesn't feel like a Frankenstein's monster when folks go to use it.

It seems to me the way to satisfy all of the above would be something like:

PATCH /articles/5/status HTTP/1.1  
Host: example.com  
Content-Type: application/json  
{ "visible" : "0" }

The point isn't in the particular word choices I've made there, but rather that we're using one endpoint for both toggle directions, giving us a logical payload choice (visible=0 or visible=1), and making a full response similarly logical to confirm it's now the intended value. It's not a "universal" patch, but nor is it entirely bespoke - it's possible we could add other states in the future or make a more complex status calculation.

Coming to a consensus on "rules" like "Every PATCH request should take input and give a response" is important precisely because limitations like that constrain our design enough to create a coherent experience across our project. In this case, the rule manifests by dictating that a toggle use one endpoint rather than two, since two would implicitly violate the rule (by making further parameters redundant). That kind of emergent design is what holds complex systems together as they grow.

It's the convention. The API is an interface that is exposed to users. It's important that it be a consistent experience. That means not only consistent with Vanilla's APIs, but APIs, in general. Actions working differently between endpoints is not consistent. The general practice is you send a patch request to an endpoint and the body of the request contains the changes you want to make. Forgoing the body in a patch request is probably going to be counterintuitive for a lot of people and will likely feel wrong.

When you patch a row, you typically return the altered value. I can send a request, get a 200 and assume it's okay, or I can get the altered row/field in the response and be sure it's been updated. The idea that we'd throw an error if something went wrong is valid in theory, but the confirmation of receiving the altered row makes me more confident the action successfully completed. Again, this is commonplace in existing APIs in and out of Vanilla. I don't think it's a good idea to change the way things work in some areas, just because part of the development team feels differently today than it did six months ago. If there's a valid reason not to do it, then it should be redone everywhere.

It's also worth noting that we use this design in Vanilla for things like discussion updates. For example, you can send a patch request on a discussion record and get the updated discussion in the result. This simplifies the overall number of API requests required and can be utilized effectively by UIs to avoid a subsequent API calls (i.e. a patch request to update the row, a followup get request to grab the updated resource).

I don't think the single responsibility principle is broken by managing the deleted status of an article in a single API endpoint ( also: API interfaces are not OOP). I'm certainly not dictating that's what we should do , hence the original question in the PR. But to me, it seems simpler for the client to manage the deleted status in one area and pass a value to toggle the flag. There are fewer items to document, which makes it easier on the user, and fewer items to maintain, which makes it easier on us. Having separate methods on the API controller just seems unnecessarily complicated to me. I'm not saying everything has to go under the umbrella patch operation, but these two extremely similar actions (delete/undelete) could share a lot of code.

All of our patch requests are "sparse" updates. You don't need to specify every field that is accepted.

Personal preferences shouldn't be PR blockers, no. But when it comes to the API, which is our product's public interface, it's extremely important to have a consistent experience. That means developers shouldn't implement an interface that functions differently from everything else, just because they think it should be different. Me saying your patch should have a request body, as well as including a relevant body in the response, isn't because I, personally, think it should. It's because it's the way anyone using our API likely expects it to, based on how everything else works.

It's also important to keep in mind that not every comment on a PR is a "blocker". Sometimes they're just questions. Sometimes they're suggestions. Sometimes they're just idle observations.

Not in this case, no. An "edit" isn't being performed by that get request. It's retrieving the row with specific values to aid in a patch request (e.g. returning the raw body content).

The post you originally linked to with the SOAP example (RESTful URLs: Actions Need Not Apply) discusses it.

There are many other articles regarding best practices for restful APIs, which often include warnings against using verbs in URLs.

That's where architecting a solution comes in. We had some recommended reading for the "soft delete" action, which illustrated some common approaches. We know the basic acceptance criteria for the functionality. It's more on us to define the technical criteria, but we aren't developing an purely internal component. This API will very likely be used by customers. It is crucial we consider common API practices, both in Vanilla's existing endpoints and in contemporary API design. It has to feel like using the rest of the API v2, just as much as it has to feel like a restful API (versus a SOAP API). Consistently following restful API best practices makes the API feel more intuitive. These endpoints cannot be designed according to however each individual developer feels about implementing them.

By "sparse" I mean: you can send one, all or only a few of the fields in most of our patch requests.

From a documentation point of view, the "in" schema exists to let you know what the endpoint accepts. It is also responsible for validating the input request.

The Swagger documentation is useful for anyone who wants to see what capabilities are available to them via the API. It automatic generation certainly has its bugs, but we are working to improve it. If you find anything specifically wrong, I highly recommend filing an issue for it (if one doesn't already exist).

My 2 cents:

PATCH /articles/{id}/softDeleted

softDeleted:b

Addresses:

• The verb problem
• The clarity with DELETE
• If you want to "restore" just put softDeleted = 0
• Anyone reading the following will understand
•     if ($record['softDeleted']) {
•         continue;
•     }
•     (I can't codeblock in a list!)

That is true. We don't have to make a restful API. We want to. We don't have to make it follow best practices. We want to. We want to do this, because other professional APIs follow the same patterns and practices. They are patterns and practices that have been tried and tested for years, by companies much larger than our own. They are existing, valid solutions to common problems. If we say we have a restful API, there are expectations that it functions in a certain way. We don't have to meet those expectations. We want to.

There isn't a stone tablet of "Best Practices for RESTful APIs". If you want to read an RFC, you can probably find a list of "musts" and "must nots", but beyond that, you'll find the recommendations are largely a collaborative effort by the global developer community. We rely on the insight and experiences of others, so we don't have to make the same mistakes.

One of the biggest players in the knowledge base space is Zendesk. Their restful API docs are public and available for review. You can checkout their Articles endpoint page for an example. You'll notice none of their URLs include "doing words". This is the same across all their endpoints, near as I can tell. They rely on HTTP verbs, not "actions" in the URL. This is what we'll be competing against and this is the type of API interface professional developers (especially from companies we poach) will expect. Like Zendesk, we can follow an established, widely-accepted pattern for designing APIs, or we can try to make things up as we go. I do not think we want to go with the latter.

It accepts two. You can give it one, if you want. Either one. The endpoint doesn't care how many fields you send it. It'll update the target record with the values you sent. If you send it fields that it doesn't accept (i.e. anything other than knowledgeCategoryID and sort), the input schema will remove them before the save operation is performed.

How about "remove" and "restore"? or is that stepping on other endpoints?

Pretty much all of the get verbs are specific to getting data to perform an action. I said this before, but they are not "doing words". I'm not creating a quote when I access /discussion/{id}/quote, I'm getting a discussion row in a format that's more compatible for quoting. I'm not editing a comment when I access "/comments/{id}/edit", I'm getting the fields necessary for an edit (e.g. the raw body contents, instead of the rendered body contents). The act of creating a discussion with a quote is still a post to "/discussions". The act of editing a comment is still a patch to "/comments/{id}".

The put request to "/users/{id}/ban" is writing to a specific field. The user's row has a "ban" flag. It's part of the resource. Similarly, "/categories/{id}/follow" is updating the user's "follow" status of the target category. It's part of the user's category data. You can unban a user or unfollow a category with these same endpoints by changing the request body. You're directly setting fields on the resources. You aren't banning a user by requesting "/users/{id}/ban". You're updating the user's ban flag. You aren't following a category by requesting "/categories/{id}/follow" . You are updating the "follow" flag in your personal category data.

Posting to "/users/{id}/confirm-email" is not ideal. Given the chance to do this over again, I would maybe rename it to "/users/{id}/email-confirmed". The actual field is "Confirmed" in the database, but that name is probably not intuitive to most users.

Posting to "/media/scrape" is not a typical scenario. In this case, we're essentially managing a virtual resource. It scrapes external page data. We had a lot of internal discussion of how to go about creating the interface for this operation and this is ultimately what we decided on. This is definitely an exception to the rule, since it isn't truly managing a real resource. It is not an example to be followed. We almost certainly could make improvements on its implementation now to be more "restful".

We've certainly made design decisions that we'd do differently the second time around, but they aren't justification for perpetuating similar decisions in newer designs.