Cht-core: Creating user via fails due to invalid `reported_date`

Created on 1 Jul 2016  路  13Comments  路  Source: medic/cht-core

The same script to create demo users on 2.6.3 fails on the same instance now running 2.7.0.

Here is the data that is sent as a POST to https://admin:[email protected]/api/v1/users:

{
  "username": "demo",
  "password": "password",
  "fullname": "Demo User",
  "phone": "+16505551234",
  "email": "[email protected]",
  "type": "district-manager",
  "place": {
    "name": "Demo Area",
    "type": "health_center",
    "parent": "241b1612f20190239e78b9725428034d"
  },
  "language": "en",
  "known": true,
  "contact": {
    "name": "Demo User",
    "phone": "+16505551234"
  }
}

Response:
Reported date on place 241b1612f20190239e78b9725428034d is invalid: 1464382858500

Bug

All 13 comments

Yeah the format is invalid it should be clear in the docs. Let me know if you have problems.

The POST was working with 2.6.3 and I don't see what is invalid about the format. I had rechecked the documentation before posting the issue, so it would be helpful to get some indication as to what is incorrect in the format.

As for the reported date, we have the following in the develop API docs (but not yet in master API):

Records

Unix or Moment.js compatible timestamp of when the message was received on the gateway. Default: Date.now()

People and Places

Date string that is compatible with this format: "YYYY-MM-DDTHH:mm:ssZ". Where "Z" is a timezone value like "-03" or "+1245". Example: "2011-10-10T14:48:00-0300". If omitted the current time is used. A compatible date string can be generated using the toISOString method on a Javascript Date object.

In my case here, I am not creating the users with reported_date anymore, so the default should be used. However, the places on this instance already had a reported_date set in ms. The "invalid date" is for a place that was previously created. It sounds like the code is now expecting this parent's date to be in ISO string. Should there have been a migration when upgrading to 2.7.0?

Also, we have tasks relying on reported_date being in ms since that was the format used already for reports. We should probably use a single format consistently throughout the app, and update the task rules accordingly. cc @sglangevin

I wasn't expecting reported_date being set on the places/people... I thought it was a new field. Do all of our 2.x instance use reported_date on places and people, trying to decide if we need a migration. If the reported_date field is new and still in development then we don't need a migration and can just do a bulk update on the instance you are developing on. I might need a little history on the reported_date field, maybe a quick call is worth it. cc @sglangevin

In existing v2 instances we have reported_date set in all families. I think my demo instance is the only one with it for branches and CHW areas.

Oh that's right... maybe it's easiest to just allow ms since epoch (because that is the stored format) but not advertise it in the API docs because it's not the most clear/standard?

Will give this a go...

Thanks @mandric, I think that would work well.

Sounds good - thanks for getting to the bottom of this and let me know how I can help with testing.

LGTM, moving to acceptance testing.

@mandric This needs to be merged into develop or we'll lose it. Currently I'm not bothering with testing or master branches in api because there's no point as you can branch from any commit if you need to at a later date.

You are correct sir I failed to clean this up. We should just merge testing and delete it. Feel free to assign to me or do eeet.. We should also merge master or cherry pick if the graph makes us cry. I think there is an issue open for that task and I will probably need to do the final sign off on that one.

I was planning to clean up sentinel and api as part of #2410 but that may be some time away...

Works well on 2.7.1-beta.3547, moving to ready.

Was this page helpful?
0 / 5 - 0 ratings