-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some .NET endpoints may not follow best practices #2008
Comments
|
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.
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. |
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. 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. |
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. |
Closing this issue. .NET API is no longer in service. |
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.
The text was updated successfully, but these errors were encountered: