zerobase-io / smart-tracing-api

Backend/DB/API repository for the Zerobase platform
Apache License 2.0
6 stars 9 forks source link

V1 RESTful APIs #5

Closed toadzky closed 4 years ago

toadzky commented 4 years ago

V0 APIs do not follow REST best practices and make it harder to use and develop against. In order to make it easier for both frontend and backend developers to understand where new endpoints fit, we need to move to a RESTful API. Here are the proposed endpoints for v1:

Organization Management

Create Organization

Update Organization Multiple Sites Setting

Create Site

Get Sites

Create Scan-able

User/Device Management

Create Device without User

Create User

Create Device with User

Update Check-In Location

User Summary

Delete User

Models

Some things come from the backend and are global. All items in this section will have a base path segment of /models and will only respond to GET requests for the time being.

Site Types

Scannable Types

togakangaroo commented 4 years ago

Some quick notes about RESTfullness - obviously any and all of these may or may not be correct or worthwhile to follow

toadzky commented 4 years ago

From a REST point of view, JSON should not be specified - format is generally determined automatically by the framework via content negotiation by using the Accept header

technically, this is correct, but the backend must be configured to support specific media types both for serializing/deserializing as well as routing. the backend is only being configured to support JSON, so it's just a not that if you don't send JSON, the api will reject the request

Create organization - probably should be singular /organization since you're creating one...

in terms of single vs plural for collection endpoints, it's not a defined standard and it's mostly personal preference. the main argument for singular is related to it representing a sql relation, but since we aren't in sql, that's not relevant here. the /collections path represents a collection of organizations. POSTing to the collection is how you create a new organization inside that collection. collections are, IMO by definition, plural and my path segments reflect that. i've never heard anyone suggest that the POST to a collections endpoint should create multiple items in that collection.

GET sites return should be a 200 Yep, that's just a copy/paste error. My bad.

Is the workflow that you can create a device and then create a user associated with it on a later call? ... Yes, the creation of a user basically adding contact information to a device (keeping it separate for a number of reasons). A user is not required to add their contact information and can still use the service in full. As such, for now, you create a device when you first start interacting with the system, but then can add contact information after that. I tried to structure the api so that, in the future, users could be created before devices, but the focus of the workflow is quick and easy access to the check-in, rather than a sign-up process.

Similarly, is it possible to change or add the device for a user after the user is created? If so I'd expect a PATCH or PUT endpoint there (probably the former)

In the current model, the user has a 1-to-1 relationship with the device. Since the device is nothing but a UUID with a fingerprint used for check-ins, there isn't a way to update that device. The model currently doesn't support the idea of multiple devices for a user, but one day it could. I tend to avoid PATCH endpoints because their behavior is not intuitive. I prefer sub-resources to manipulate specific fields with PUTs and a PUT endpoint to update the entire object.

Check in - rather than having to specify the type, why not make that part of the url? Seems like those different types of checkins would really be different resources

The type of check-in only affects whether or not location data is copied from the check-in site. Device-to-device check-ins are a legacy feature that doesn't have much of a place in the current workflow, but we are trying to keep some of the legacy stuff. The reason not to make it part of the URL is that it's not a separate resource, it's a property of the check-in you are creating.

Update checkin location - does that really conceptually replace the pre-existing checkin location or just create a new checkin? If the latter it should probably be a POST

The workflow, when checking in from a verified testing site, is to verify the users location to make sure it matches what is in the database for that site. This is partially to avoid spoofed scanning, among other things. To avoid having the check-in lost if the user doesn't give location permissions or closes the browser or something like that, we are having it create a check-in immediately (including location data if available). If the user has not given permission for location data, then the UI can ask the user for permission and update the check-in with the location data if allowed. So it's updating the location data on the check-in, not creating a new one - hence the PUT to a sub-resource.

User summary - is that really different from getting info on the user This endpoint is basically for GDPR compliance. We need a way to dump all a user's information into a single output for them. A GET on the user endpoint (assuming we had one) would only return the user, not all the devices and check-ins they had made over time. This endpoint would return the entire graph of user-related data.

Delete User - I think you have a typo in the path Yes, i do. Good catch.

Should specify what other resources will become invalid when this is deleted - for example will the device url of any device associated with this user stop working?

For the moment, no. The way we are trying to design this is that deleting the user will delete the user node, which includes all their contact information, but leaves the check-in data since it's critical to the overall calculations of risk indexes and prediction algorithms. Once we remove all identifying information, we are hoping that will satisfy GDPR. If it doesn't, we will have to cascade the delete to the user's entire graph.