Cht-core: Create an all-in-one user/user-settings/contact addition API

[ghost]

We need a one-shot API for creating working users, complete with contacts and places and user settings. It should create the user doc in the _users database, create the user-settings doc in the medic db, and create an appropriate contact in the medic database. Here's our current design:

Example request:

POST /api/v1/users
Content-Type: application/json

{
  "password": "secret",
  “username”: “mary”,
  “type”: “district-manager”,
  "fullname": "Mary Anyango",
  "place": "eeb17d6d-5dde-c2c0-62c4a1a0ca17e342", // required as an id
  “contact”: { "phone": "...", “parent”: “...”, ... } // phone and parent are required
}

Example response:

HTTP/1.1 200 
Content-Type: application/json

{
  "user-settings": { "id": "...", "rev": "..." },
  "user":  { "id": "...", "rev": "..." },
  "contact":  { "id": "...", "rev": "..." }
}

This creates a contact based on the JSON object in the contact property, and saves the generated contact identifier to the user-settings doc. Contact is a "free form" JSON object, based on whatever form definition happens to be related to contacts. The contact and contact.parent fields are required if you want to submit forms.

Validate that place already exists. Validate that the contact.parent is the same as the place or a child of the place.

Use validate_doc_update where possible.

This issue supersedes #1744 and #1764, by splitting user addition and user editing features into two separate issues. This issue is a prerequisite for #1709.

[garethbowen]

I've added a few comments to the PR. Looks really good.

[mandric]

Ok added a few more comments too... I think we just need to decide how to handle the validate_doc_update and permissions business.

[abbyad]

From what I can tell this creates the person and user, but not the person's place. For our current CHW app projects we have the following place hierarchy:

• Branch (type: district_hospital)
• CHP Area (type: health_center)
• Family (type: clinic)

For each CHP user we create a CHP Area. For example, the CHP "Marc Green" would be associated to the place "Marc Green Area". The API additions in this issue help us create the CHP and user, but we'd still manually have to create the "CHP Area" in the UI.

It would be great to have the option of creating the corresponding place via the Users API. For instance, we could specify that we want a user in a new place, and specify the parent place. We could also have an API call for creating places - which may be needed regardless - but it would make scripting user creation a little trickier.

Depending on which we choose we can break this out into a new issue. Either way, this would be very useful for setting up projects so that we dont have to set up dozens and hundreds of places manually using the UI, and getting each ID to create the users.

[ghost]

Bumping back to active work per Marc's comment. @mandric: Should we spin off a new issue or are you happy handling the CHP Area creation in this one?

[mandric]

I'm fine with creating the person's place too. But @abbyad is there a specific way you envision using the API, can you maybe mock up an example usage? Maybe something like this, so if you specify place as an object (instead of a string) then we create the facility and also automatically assign it to the person/contact (set contact.parent to it), you would need to specify one of our internal types too. e.g.

POST /api/v1/users
Content-Type: application/json

{
  "password": "secret",
  "username": "mary",
  "type": "district-manager",
  "place": {
    "name": "CHP Area for Mary",
    "type": "health_center",
    "parent": "eeb17d6d-5dde-c2c0-62c4a1a0ca17e342"
  },
  "contact": {
    "name": "Mary Anyango",
    "phone": "[phone]"
  }
}

So this means we can create one level of place. Do we want to support being able to create more than one level, so you could also do for example below Mary's District would also be created.

POST /api/v1/users
Content-Type: application/json

{
  "password": "secret",
  "username": "mary",
  "type": "district-manager",
  "place": {
    "name": "CHP Area for Mary",
    "type": "health_center",
    "parent": {
      "name": "Mary's District"
    },
  },
  "contact": {
    "name": "Mary Anyango",
    "phone": "[phone]"
  }
}

I might be neat if both "place" and "parent" are special properties so they could be specified as a string or object at any level and the API does the right thing. I also wish we could remove "health_center" from this API entirely.

[abbyad]

I really like that idea @mandric, as it would be flexible enough for foreseeable use.

Would posting with the place as an object always create a new place, or would it first search for a place of the same name first? If it is the latter that would likely make it easiest for scripting.

[mandric]

I'm not sure what makes more sense... maybe add the basic place creation support first and then see how it goes?

[mandric]

So I will modify the API so that if you specify place but not contact.parent then the contact parent automatically gets assigned the place that is created.

[garethbowen]

Looks good. I've added a few code readability comments to the PR.

[mandric]

Thanks, will clean it up a bit tomorrow.

[garethbowen]

LGTM

[mandric]

Related documenation https://github.com/medic/medic-docs/blob/master/md/dev/users-bulk-load.md

[mandric]

So I'm assigning the new place that gets created to contact.parent but should I also be assigning the new contact to place.contact? Seems like a lot of duplicated data?

[garethbowen]

Yeah you're not wrong. I think the point of it is to support workflows in both directions. Sometimes you need to know what area this contact is is, and sometimes you need to know who is the primary contact for a given area.

We should think about linking contacts vs inlining again...

[mandric]

Ok shoot, should we re-open and add this?

[mandric]

Oh it's still open.

[mandric]

Ok I'll add this unless I hear otherwise.

[alxndrsn]

Hey @mandric any idea what i'm doing wrong here?

$ curl -X POST -d'{"username":"ABC123","password":"pass","type":"user-settings","place":{"name":"Jeremy Fisher Family","type":"clinic","parent":"1234567890abcdefabcdef"},"contact":{"name":"Jeremy Fisher","phone":"[phone]"}}' http://admin:pass@localhost:5988/api/v1/users
Missing required fields: username, password, place, contact

[mandric]

Is that on develop?

[mandric]

Reproduced here on develop. Looking...

[alxndrsn]

thanks

[mandric]

Actually @alxndrsn it seems to work correctly on develop but on the feature branch it's broken.

[alxndrsn]

The output above was from 5b8544d4e5aa24939905db3d7416e19d61eafb3f - am I behind?

[alxndrsn]

(apparently not - https://github.com/medic/medic-api/commits/develop)

[mandric]

-H “content-type:application/json"

[mandric]

Actually on the feature branch this is fixed, you get a better warning when we see an empty json object.

[alxndrsn]

Thanks, adding the header makes sense :-)

[mandric]

In you future you would see "Request body is empty, check your content-type header."

[alxndrsn]

In you future you would see "Request body is empty, check your content-type header."

This error seems a little misleading, as the request body definitely isn't empty. What are non-JSON posts expected to contain?

[mandric]

In you future you would see "Request body is empty, check your content-type header."

This error seems a little misleading, as the request body definitely isn't empty. What are non-JSON posts expected to contain?

A non-JSON request (does not specify application/json content-type header) body is converted to an empty object {} regardless of what is in the body. That's how we have the jsonParser middleware configured, or just how it works. I'm open to suggestions.

[mandric]

Example: https://github.com/medic/medic-api/blob/2046-places-and-contacts-linkage/server.js#L463

[alxndrsn]

@mandric how about

Request body is empty or Content-Type header was not set to application/json.

instead of

Request body is empty, check your content-type header.

?

[alxndrsn]

@mandric any thoughts on this one?

curl -X POST -d '{"username":"LIR10025","password":"TODO-secret","type":"user-settings","place":{"name":"Opio Andrew Family","type":"clinic","parent":"6d0d00539ef8bec5b5fcc1f8c200887d"},"contact":{"name":"Opio Andrew","phone":"[phone]"}}' -H'Content-Type: application/json' http://admin:pass@localhost:5988/api/v1/users
doc.roles can only contain strings

[mandric]

@alxndrsn type should be one of 'district-manager', 'national-manager', etc. It should be pretty clear if you read the docs. ;-)

You are trying to set permissions/roles to 'user-settings' which is not allowed and I should fix the error message for that too.

[alxndrsn]

@mandric Can you give me an example of a valid request, based on this?

curl -X POST -d '{"username":"LIR10025","password":"TODO-secret","type":"user-settings","place":{"name":"Opio Andrew Family","type":"clinic","parent":"6d0d00539ef8bec5b5fcc1f8c200887d"},"contact":{"name":"Opio Andrew","phone":"[phone]"}}' -H'Content-Type: application/json' http://admin:pass@localhost:5988/api/v1/users

[mandric]

Sure:

$ curl -H 'content-type:application/json' -d '{"username": "lir10025", "password": "TODO-secret", "place": { "name": "Opio Andrew Family", "type": "clinic", "parent": "71df9d25ed6732ea3b44358625258417" }, "contact": { "name": "Opio Andrew", "phone": "[phone]" }}' http://admin:secret@localhost:5988/api/v1/users

[mandric]

Request body is empty or Content-Type header was not set to application/json.

Much better, will update. Thanks!

[alxndrsn]

Looks like there is a little bug if creating a contact with non-lowercase username:

$ curl -vvv -X POST -H 'Content-Type: application/json' -d '{"username":"LIRXXXXXX10025","password":"TODO-secret","place":{"name":"Opio Andrew Family","type":"clinic","parent":"b532d1ad29d5f3e24702fd10e41a502f"},"contact":{"name":"Opio Andrew","phone":"[phone]"}}' http://admin:pass@localhost:5988/api/v1/users
*   Trying ::1...
* Connected to localhost (::1) port 5988 (#0)
* Server auth using Basic with user 'admin'
> POST /api/v1/users HTTP/1.1
> Host: localhost:5988
> Authorization: Basic YWRtaW46cGFzcw==
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 209
>
* upload completely sent off: 209 out of 209 bytes
< HTTP/1.1 403 Forbidden
< X-Powered-By: Express
< Content-Type: text/plain
< Date: Wed, 27 Apr 2016 12:10:45 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
<
* Connection #0 to host localhost left intact
_id must be lower case. e.g. "org.couchdb.user:sally"

$ curl -vvv -X POST -H 'Content-Type: application/json' -d '{"username":"LIRXXXXXX10025","password":"TODO-secret","place":{"name":"Opio Andrew Family","type":"clinic","parent":"b532d1ad29d5f3e24702fd10e41a502f"},"contact":{"name":"Opio Andrew","phone":"[phone]"}}' http://admin:pass@localhost:5988/api/v1/users
*   Trying ::1...
* Connected to localhost (::1) port 5988 (#0)
* Server auth using Basic with user 'admin'
> POST /api/v1/users HTTP/1.1
> Host: localhost:5988
> Authorization: Basic YWRtaW46cGFzcw==
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 209
>
* upload completely sent off: 209 out of 209 bytes
< HTTP/1.1 409 Conflict
< X-Powered-By: Express
< Content-Type: text/plain
< Date: Wed, 27 Apr 2016 12:10:49 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
<
* Connection #0 to host localhost left intact
Document update conflict.

I.e. the first create returned an error, but actually created something somewhere in couch.

[garethbowen]

Looks awesome!

Merge away.

[abbyad]

Works well during initial testing.

[abbyad]

Noticed that the user is partially created if I get one of the error response:

• _id must be lower case. e.g. "org.couchdb.user:sally"
• known is not a boolean.

The user can be seen in the Users config page, but it has a warning A call to facility with the expectation of having person data and deleting the user shows a Error deleting document 404, but actually deletes the (partial) user.

[abbyad]

I was able to create a username with a space, but that user could not log in.

[abbyad]

Notes from testing:

• When creating a user the place is automatically created, but the response does not give you the ID of the place created for the user's contact.
• The parent is automatically created even if a top level place of the same name exists. For instance, if you have place.parent.name set to "Demo Branch" you will have one "Demo Branch" created for each user you create. The alternative is to create the Demo Branch separately and use the ID, but that only works for a fresh branch. If it exists already, eg when adding more users for a training, the branch ID must be manually searched and used in the script.

[abbyad]

The reported_date is not set automatically. It is set when a person or place is created using the UI, and is used in our task rules as well as in our delete script, so it would be good for the Users API to set them by default for the people and places it creates.

In the meantime, the reported_date can be specified as:
place.reported_date for the place
contact.reported_date for the person

[sglangevin]

@abbyad I believe that @mandric added support for reported_date for the clinic and person - not sure if that's in the LG import script or will be added to the Users API.

[abbyad]

From my tests the reported_date field is not created in the person or place unless you specify it in the json that is passed. I think it should be done automatically in API because someone writing a task rule could expect it to be there for all contacts.

If for some reason we'd want to overwrite it we should be able to set it in the JSON passed in the API call.

[sglangevin]

Yeah, you're right. That's a good idea - we should do it automatically.

[mandric]

Yeah good idea. Feel free to create an issue or I'll just add it next time I'm under the hood. ;-)

[mandric]

Is there a specific format we should be validating when accepting date values for reported_date? Because for example 1 is a valid date if you use moment(1) or new Date(1), or any number for that matter. Should we constrain input somehow or just test any value that gets passed in with moment(val).isValid() or something? I'm thinking keep it simple/less strict for now until we find a need for more validation. We could for example validate based on format like accept dates in these specific formats: YYYY-MM-DD, etc.

[mandric]

To be more strict we would standardize on only accepting ISO 8601 formatted dates and rejecting others by doing http://momentjs.com/docs/#/parsing/string/ Example, require format that includes a timezone:

> moment("2011-10-10T14:48:00+0400", "YYYY-MM-DDTHH:mm:ssZ", true).isValid()
true
> moment("2011-10-10T14:48:00", "YYYY-MM-DDTHH:mm:ssZ", true).isValid()
false

[mandric]

PR over here https://github.com/medic/medic-api/pull/78

[garethbowen]

Nice one.

[mandric]

Shoot just pushed a refactor...

[mandric]

Fixed up a couple details: https://github.com/medic/medic-api/pull/79

[mandric]

Thanks!