shotvibe / shotvibe-web

ShotVibe REST API webservice
1 stars 0 forks source link

Extract album "add_members" to its own endpoint #52

Closed benny-shotvibe closed 10 years ago

benny-shotvibe commented 10 years ago

adding members to an album should have its own separate endpoint.

This will allow it to return a response that will give a success/failure result for each phone number.

This way, if a user adds multiple contacts, but one of them is an invalid phone number, then the app can display an error message telling the user exactly what the problem is

benny-shotvibe commented 10 years ago

This should be at

POST /albums/{aid}/add_members/
benny-shotvibe commented 10 years ago

The JSON input should be an array in the same format as we currently have with the "add_members" array.

We should decide what the best format is for the response

prudnikov commented 10 years ago

To be more RESTful in my opinion we shoud create it as /albums/{aid}/members/ which is album members endpoint. Sending POST request to the collection endpoint creates new object in this collection. In our example we add a new member to the album members collection.

Response in this case should be 201 Created with Location header. Location should be URI of the member in album... but I'd rather return URI of the album. This is what I return in my api at snippets.

benny-shotvibe commented 10 years ago

That does seem more RESTful. Only problem I see is that it will require multiple requests for adding multiple users. I'm not sure that it will offer much of an advantage to have a unique URL for each album member

prudnikov commented 10 years ago

Agree, it makes no sense to have an endpoint /album/{album_id}/members/{member_id}/, that's why I said that it should return Location header with the URI of the album.

The questions are: Can we create more than one membership with one POST request? Or should we? User selects new members from the list one-by-one and sending one request for each new user instead of collecting all members and sending all of them seems not that bad. On practice, I think 60-80% of times user will add one new member at a time.

prudnikov commented 10 years ago

Should I remove the old way of adding members or you need both?

benny-shotvibe commented 10 years ago

We should leave the old one in for now, until the apps are updated

prudnikov commented 10 years ago

This is almost done, but "We should decide what the best format is for the response" — so, what should I send in response?

prudnikov commented 10 years ago

I'd rather make a separate request for each member, in a RESTful way it should be PUT /album/{album_id}/members/{member_id}/, response 201 Created and Location: /album/{album_id}/members/{member_id}/ header. Later on DELETE /album/{album_id}/members/{member_id}/ to remove member from the album. It should be much simpler on both server and client side.

benny-shotvibe commented 10 years ago

I agree that trying to be RESTful is best, but there is no good way to be truly RESTful when dealing with multiple "resources" (such as multiple phone numbers).

It is important for the app to be able to add several contacts using a single API call. Our app allows the user to select multiple contacts and add them all at once. It will be common for a user to add 10 users at one time, if we need to do 10 API calls for this then it will take 10 times as long for the app and will be too slow.

benny-shotvibe commented 10 years ago

Here is how stackoverflow deals with the issue of handling multiple resources in a single request: https://api.stackexchange.com/docs/vectors Not RESTful at all, but unfortunately workarounds like this are necessary for performance reasons. I think that trying to be RESTful is important, but if something doesn't fit with the REST philosophy, then it's better not to force it, and better rather to keep a clean simple design

prudnikov commented 10 years ago

Repeat: This is almost done, but "We should decide what the best format is for the response" — so, what should I send in response?

benny-shotvibe commented 10 years ago

Input JSON can be a list of contacts (the same as we currently have):

[
        {
            "user_id": 4
        },
        {
            "user_id": 6
        },
        {
            "phone_number": "212-718-2000",
            "default_country": "US",
            "contact_nickname": "John Smith"
        },
        {
            "phone_number": "212-718-3000",
            "default_country": "US",
            "contact_nickname": "Jane Doe"
        }
]

Output JSON can be an array of the same length with a success/failure result for each contact:

[
    {
        "success": true
    },
    {
        "success": false,
        "error": "invalid_user_id"
    },
    {
        "success": true
    },
    {
        "success": false,
        "error": "invalid_phone_number"
    }
]

Right now, the only possible "error" value can be "invalid_user_id", but in the near future we should add a check for the result of the Twilio SMS send, which will tell us if sending the SMS failed due to being an invalid number