Some .NET endpoints may not follow best practices

[GrahamS-Quartech]

Starting this thread to track discussion about potential improvements we could make to the API route structure while making the transition from .NET to Express. I'll start by listing some things I noticed while working on the notifications routes. Feel free to comment on whether or not these are issues worth addressing in the new API, and please add issues present in other routes as well.

• PUT /projects/disposal/:id/notifications/cancel - This doesn't seem to make much sense as a PUT endpoint, since you don't really need to send the entire resource. Delete would be more appropriate, and in general I think that having a verb as an endpoint is considered non-RESTful.
• /notifications/queue/:id - It's not obvious at a glance, but these seem to be using the projectId's, not the notificationId's
• /notifications/queue/:id/resend + cancel - Similar thoughts as the other cancel endpoint. It's not clear to me why resend exists instead of just hitting the relevant POST endpoint again instead.
• /notifications/templates/:id - The implementation for these never does anything with the templateId provided, it just trusts that the body is correct.

[dbarkowsky]

1. There are a number of endpoints using POST requests for their filter searches. Looking at what info is passed in the filters, there doesn't seem to be any reason to not just use a GET with query parameters. Even better would be if we can cut all the /filter endpoints and just have the filter active if query parameters are included, otherwise GET all.
2. Several endpoints use PUT methods to update records, even when only updating a single field. I recommend we reserve PUT methods for replacements, not just small edits. Instead, we could use PATCH, including only the data we want to update. This would also cut down on volume of data transferred.

[GrahamS-Quartech]

1. There are a number of endpoints using POST requests for their filter searches. Looking at what info is passed in the filters, there doesn't seem to be any reason to not just use a GET with query parameters. Even better would be if we can cut all the /filter endpoints and just have the filter active if query parameters are included, otherwise GET all.

I am going to adopt this going forward, they really should just be query strings filtering a "get all" style endpoint. No need to have a separate filter route.

Several endpoints use PUT methods to update records, even when only updating a single field. I recommend we reserve PUT methods for replacements, not just small edits. Instead, we could use PATCH, including only the data we want to update. This would also cut down on volume of data transferred.

I think there is some merit in using PUT in an application that's heavily form based. If doing a PATCH the frontend would need to check which values of the resource were actually changed compared to the previous form, but if you just send the whole form every time you don't need to bother. That can simplify things a bit. Either way is fine though as long as we use the HTTP method correctly.

[GrahamS-Quartech]

Some notes on the reports endpoints:

I'm pretty sure the surplus properties endpoint is not necessary but perhaps someone can weigh in. Why do we need an endpoint specifically for surplus properties reports when surplus is simply a classification of properties? I feel like you should really only need to apply a filter on the general properties endpoint. The endpoints do actually function differently but they appear to produce a similar end result so not sure what the distinction is for.

As per other routers, filter is omitted.
There is another route I omitted: /reports/properties/all/fields/
This is also something that I think should just be rolled into the query string of the properties endpoint. If there are only two export modes (ie default and allFields) then we can just make it a boolean query string. Alternatively, we could add support for enumerating what fields should be excluded/included
ie. /reports/properties?include=agency,classification,type

The swagger docs for the old API list the responses for some of these endpoints incorrectly or not at all. These endpoints are different from the endpoints that get report by id under the projects ticket, which return JSON objects. These endpoints return a text representation of the requested spreadsheet format.

[GrahamS-Quartech]

A note on tools:

For the imports, the POST bulk uploads seem to allow updates behind the scenes, so it might make more sense to make these PUTs if we are going to allow both add and update in the same endpoint.