Moving AllergyAPI into OpenMRS 2.0 Platform - TRUNK-4747 has raised the following issues:
should the “/allergies” resource be a subresource of patient since allergies are always within the context of a patient (In the current allergyapi code this is not the case)
should “/allergy” resource also be a subresource of patient (In current webservices.rest this is not the case and /allergy/id is not implemented. In any case to implement this feature, a new method will have to be added to PatientService, getAllergyByUuid())
Any suggestions on how to proceed?
@darius, as part of TRUNK-4747 - Move Allergy API into OpenMRS 2.0 Platform - , the allergyapi features of PatientService has already been merged into core. The only issue remaining for that ticket is how to handle the REST API features of allergyapi.
allergyapi did not have the getAllergyByUuid() method though.
Let’s see what views @burke has for moving this forward.
From a clinical standpoint, the resources that would be intuitive to me would be the sub resource approach:
/patient/{uuid}/allergies → [] (for NKA), [ {allergy1}, … ], or HTTP 404 if unknown
/patient/{uuid}/allergies/{uuid} → { allergy }
I suppose I could be convinced of allergies as a separate resource, since FHIR has a separate AllergyIntolerance resource; however, it seems unintuitive to me to reference allergies outside the context of a patient. In my mind, the existence of allergies (or no known allergies) is a “property” of a patient.
@burke, someone else suggested they should be a subresource on the design
call. I pushed back by saying that visit, encounter, and lots of other
things also belong to patient, but nothing else is a subresource. Why
should allergy in particular be a subresource when nothing else is?
It’s a fair point. While I wouldn’t say that everything linked to a patient (encounters, orders, etc.) needs to be a subresource, it would make sense to me if those things that are attributes of the patient (allergies, identifiers, and conditions/problems) were subresources.
You can ask about an encounter and discover who the patient was for it. It would be odd to ask about an allergy and then find out who had it. (given this logic, I suppose identifier could make sense as an independent resource… but allergies & conditions/problems are properties of the patient).
I’m not sure how easy it is to implement this as a sub resource given our
existing base classes in the framework and the special behavior associated
with the NKA state, but we can try.
I don’t really like representing NKA as a 404. Especially because it’s not
intuitive how you would set that status. I would lean towards having an
object with one property for the status +/- a property for the list.
Allergies as a sub-resource of patient seems a good design choice.
No Known Allergies, NKA, may best be represented as a empty list rather than a 404, though a bit easier to implement it still may be a bit difficult as @darius points out. However NKA as an empty list would require that the API around allergyStatus be removed.
Is this the consensus and should implementation proceed?
This is not the same as unknown allergy information. Someone has explicitly confirmed that the patient does not have any known allergies – i.e., allergy information is known, there just aren’t any allergies. In this case, we have a known list of allergies that is an empty list.
curl -i /patient/d8f598df-6f24-4434-845b-0849f2360dc3/allergies
HTTP/1.1 202 OK
[]
Patient with allergies
Surprise, surprise. We return a list of allergy resources.
curl -i /patient/d8f598df-6f24-4434-845b-0849f2360dc3/allergies
HTTP/1.1 200 OK
[
{
# allergy resource #1 here
},
{
# allergy resource #2 here
}
]
Allergies unknown
This is not “no known allergies.” We do not have any information about the patient’s allergies. They might be allergic to something; they may not have any allergies. Returning an empty list would be misleading. Our clearest response is to say the allergies resource does not yet exist for this patient.
curl -i /patient/d8f598df-6f24-4434-845b-0849f2360dc3/allergies
HTTP/1.1 404 Not Found
Sorry for the confusion, I have been completely incorrect in my usage of NKA in my previous two posts.
What I mean to ask was: how do you set a patient’s allergy status to unknown?
The pattern we have followed for personattribute and patientidentifier are as follows:
GET person/uuid/attribute => { results: […] } // results are paged if there are many
POST person/uuid/attribute => creates one new personattribute
DELETE person/uuid/attribute => invalid operation
GET person/uuid/attribute/attruuid => { representation of one personattribute }
POST person/uuid/attribute/attruuid => updates a specific personattribute
DELETE person/uuid/attribute/attruuid => voids a specific personattribute
To follow this same pattern (which should be our default approach) allergy would look like:
GET patient/uuid/allergy => { results: […] } // results are paged if there are many
see below for discussion
POST patient/uuid/allergy => creates one new allergy
DELETE patient/uuid/allergy => sets allergyStatus to unknown and voids all allergies
this is new (e.g. there’s no equivalent mechanism to remove all personattributes)
GET patient/uuid/allergy/allergyuuid => { representation of one allergy }
POST patient/uuid/allergy/allergyuuid => updates a specific allergy
DELETE patient/uuid/allergy/allergyuuid => voids a specific allergy
About GET patient/uuid/allergy …
@burke says that should return 404 for the unknown state. I think this will be unintuitive for a client, and I propose that we do:
(if no known allergies)
{
status: "No known allergies"
results: []
}
(if some)
{
status: "See list"
results: [
{ /* ... */ }
],
links: [ ... ] // if >1 page of results, which should never happen in real life unless the client requests a small page size
}
(if unknown)
{
status: "Unknown"
}
(these text strings are from the Java API (see here)
Are you suggesting that DELETE be used to set allergies to unknown? after a DELETE operation I would expect a 404 at that resource. I think using 404 is an interesting idea here in the context of sub resources that have never been set. Or how about 204 no content? Caveat: I am no rest expert, just voicing my opinion
Based on the REST API Tutorials documentation on HTTP Status Codes, a 404 seems more appropriate than a 204, since a 204 No Content just means that the server is sending a response without content (not necessarily indicating why the message has no content).
To me, returning a 404 for missing data seems intuitive and RESTful. But, if handling 404’s in ajax code is extra work, then Darius’ approach could make sense – i.e., you always get a response and need to inspect the response and infer either from a status string or a missing property that the resource does not exist.
From peeking at the docs for Angular’s $http service, I think that a 404 is
treated as an error code, so it will probably be extra work (e.g. it’s the
errorCallback for the promise, not the successCallback). Someone can
explore this more in depth if they want, but I would lean towards
communicating “unknown allergies” via the response content, not the HTTP
status code.
We had to use the status (No known allergies|See list|Unknown) within Java because the API needed to return a single object regardless of state of the allergy list and throwing an exception for expected behavior isn’t best practice. In REST, however, I think using 404 for a missing resource is best practice, isn’t it?
Returning a 404 for a missing resource seems intuitive to me, but ultimately, we want to produce whatever is going to be easiest for a dev to understand & consume. I’ll stop painting this shed.
Returning 404 for NKA is a bit unintuitive to me. I would expect a 404 for calls that i should not be making because they are wrong, mispelt, or something that am doing but i should not. I find it ok, natural and acceptable (no need for 404) to make a get for a patient who may not yet have any known allergies.
Allergies will be a subresource of patient. Mostly this will behave like the existing personattribute and patientidentifier subresources.
The patient/allergy subresource will function like:
GET patient/uuid/allergy => gets current allergies (see below for what is returned)
POST patient/uuid/allergy => creates one new allergy
DELETE patient/uuid/allergy => sets allergyStatus to unknown and voids all allergies
this is “new” (i.e. there’s no equivalent mechanism in personattribute or patientidentifier)
GET patient/uuid/allergy/allergyuuid => { representation of one allergy }
POST patient/uuid/allergy/allergyuuid => updates a specific allergy
DELETE patient/uuid/allergy/allergyuuid => voids a specific allergy
What is returned when you fetch a patient’s complete allergy status (i.e. GET patient/uuid/allergy)
(We will take a leap of faith and follow @burke and @kristopherschmidt’s suggestion about how a REST-first API should behave, even though @dkayiwa and I think this is less intuitive.)
If the patient’s allergy status is “Unknown” => return a response status of 404
If the patient’s allergy status is “No known allergies” => return { results: [] }
If the patient’s allergy status is “See list” => return
{
results: [
{ /* representation of a single allergy */ },
...
],
links: [
/* if >1 page of results, we have next/prev links, just like other resources */
]
}
Personally I think that the patient resource should also have an “allergyStatus” field added to it (and the underlying Patient.java and Patient.hbm.xml should reflect this), but I’m not 100% certain about this. (And further discussion on this point should not block the bulk of the web service work from being done now.)
(given this logic, I suppose identifier could make sense as an independent resource… but allergies & conditions/problems are properties of the patient).
