facilityregistry / fred-api

Facility Registry API Documentation Website
11 stars 4 forks source link

Clarify facility creation response format #44

Closed mwhite closed 11 years ago

mwhite commented 11 years ago

The spec currently defines the JSON format for a facility response as:

{
   "facility": {
       "name": "Kakamega HC",
       ...
   }
}

I believe the implementations I've been testing against simply return the bare facility object. I propose switching the spec to that unless someone can provide a plausible future use-case for additional keys in the facility creation response that wouldn't be better accomplished with HTTP headers and also justify why that format couldn't just be adopted when it became necessary instead of now -- it seems like any additional properties might warrant a version bump anyway?

edjez commented 11 years ago

Agree the container is strange - we should remove it

rowenaluk commented 11 years ago

voted to remove that container on march 7 tech call. no objections.

ctford commented 11 years ago

I thought we had agreed to have the response to a creation be a redirect to the new facility?

mberg commented 11 years ago

This is a bit frustrating :)

@ctford we did vote to have it redirect to the facility. This is reflected in the response header docs.

http://facilityregistry.org/#create-facility

On the last call when we discussed this group people said they wanted to return the created facility and complained that it wasn't in the docs. Please let me know what you want.

I added the ... to avoid having to copy the entire facility object response.

edjez commented 11 years ago

Two issues here - return format and whether to use a redirect or a response body

  1. Agree return object should not be returned in a container 'facility' { "facility":{ <------- remove this "name": "Kakamega HC", ... }
  2. Martin, Ed and Mwhite discussed on Mar 15 and we all prefer a Response body with the json document rather than a redirect link. Low end developers may prefer the response body as well.

I think this is ready to close if the group agrees and no one really is set on a redirect

edjez commented 11 years ago

Do you know of good APIs that return a redirect? Seems a bit frustrating as a developer to be 'chasing the white rabbit' on a facility I just created. Especially now that the total URL for the created facility is less important than the internal id/uuid field the registry may have assigned. Are there any reasons FOR a redirect?

ctford commented 11 years ago

I suggested a redirect because it made sense to me, but I'm not wedded to it. In fact, it's quite ok to do both Location header and body, but just body is fine to me. If an implementation wants to supply a Location header as well they could and it would cause no problems.

If the facility is going to be echo-ed back, I would suggest we use exactly the same format as we do for the individual facility URL.

edjez commented 11 years ago

Agree on the format. Was there any particular reason that at the time the redirect seemed better at the time? Matt, did you prefer the redirect; or were you just frustrated about the apparent flipflop?

" I'm not wedded to it" --> glad to hear! But I like so far the fact that we've been able to discuss design alternatives on their own merits, and not as a function of their 'spouses', and I hope we can keep it that way.

On Mar 19, 2013, at 3:08 AM, Chris Ford notifications@github.com wrote:

I suggested a redirect because it made sense to me, but I'm not wedded to it. In fact, it's quite ok to do both Location header and body, but just body is fine to me. If an implementation wants to supply a Location header as well they could and it would cause no problems.

If the facility is going to be echo-ed back, I would suggest we use exactly the same format as we do for the individual facility URL.

— Reply to this email directly or view it on GitHub.

ctford commented 11 years ago

Some reasons why you might want a redirect:

Ultimately though, the double-request problem to retrieve the uuid (and any other generated fields) seems a good enough reason to me to include at least the core fields directly in the response, if that's what people feel. I think it's a line call.

mberg commented 11 years ago

Cleaning up the response json type. Don't feel strongly either way. Does having the redirect in the header have any affect on the client? Don't feel strongly either way.

Related thread on stackoverflow

http://stackoverflow.com/questions/2427518/which-http-redirect-status-code-is-best-for-this-rest-api-scenario

mberg commented 11 years ago

Add uuid in the response then ok to close

mortenoh commented 11 years ago

What about simply saying that the response should match the output from a GET to the location header. This is necessary for what we are doing in Uganda, where we are using ETags for checking whether a facility have been updated.

edjez commented 11 years ago

Good suggestion Morten to simplify the documentation- I assume you mean "the response should match the output from a GET". In our case the default is all the facility object, not just core properties. This may be redundant for some (it is just an echo back of the properties one just sent in, arguably only the uuid is new information to the caller) but ok for now. Updated the docs.