Skip to content
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

include filename in response when posting a reply #4047

Closed
heartsucker opened this issue Jan 18, 2019 · 4 comments
Closed

include filename in response when posting a reply #4047

heartsucker opened this issue Jan 18, 2019 · 4 comments

Comments

@heartsucker
Copy link
Contributor

Description

This request is motivated by implementing the replies feature in the SD client. When we make a reply, we get an object back from the API like

{
    "message": "Message created or whaterver",
    "uuid": "some-uuud-abc123"
}

This is sufficient for us to track in-flight message, but it is not enough information for us to be able to correctly order replies. We need the filename that the server generates as well to be able to display the messages in the correct order. This is especially true if the Tor connections that are made cause messages to reach the server out of order.

We might as well return the entire object and drop the message field humans will not be seeing this and such messages are best conveyed the HTTP status code.

Without returning this field, if we want to have correct ordering of messages, for every reply we send send, we must make an additional call to fetch the reply object from the API to retrieve the filename for ordering.

User Stories

As a consumer of the API, I want to make as few calls as possible in order to display information in a UI.

@kushaldas
Copy link
Contributor

@heartsucker makes sense to me, please write down the full list of items you want to get (returned from the server) in the API call.

@heartsucker
Copy link
Contributor Author

POST /api/v1/sources/<source_uuid>/replies should return the same object as GET /api/v1/sources/<source_uuid>/replies/<reply_uuid> for consistency (even though some of that information is redundant).

This is actually the only major change. The rest of the POSTs and DELETEs shouldn't return

{
    "message": "The thing happened"
}

But just be a 204 No Content and have an empty body IMO. This is a breaking change, but since we're the only consumers of this API, I think that's acceptable.

@redshiftzero
Copy link
Contributor

It makes sense to add the filename to the reply API response so we know the interaction count on the client side without doing a sync.

We might as well return the entire object and drop the message field humans will not be seeing this and such messages are best conveyed the HTTP status code.

Why remove an informative message (and thus make a breaking change, which the other suggestion in this ticket isn't)? Developers are humans.

@heartsucker
Copy link
Contributor Author

In all the cases where we have a confirmation message, it is always the same for a given HTTP code. I like to use the Github API as an example of a fairly good API, and in the case of object creation, they return the full object. In the case of all POSTs or DELETEs, they don't return a status message outside of the HTTP code.

But w/e that's not really that important. I have a PR ready for the filename change which is all I really need to keep working.

@heartsucker heartsucker changed the title API calls that create an object should return the full object that is created. include filename in response when posting a reply Jan 21, 2019
@emkll emkll mentioned this issue Feb 19, 2019
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants