Cannot use /api/v1/people to create a person if any place in the lineage has an invalid primary contact

[kennsippell]

Describe the bug

To Reproduce

1. Create a grandparent place with no primary contact
2. Create a parent place with a primary contact
3. Attempt to use the POST api/v1/people API to create a person within the parent place

Expected behavior In 4.4, you could create this person. In 4.6, you see the error Wrong type, this is not a person. There is no UUID of the affending contact or place, no hints for a targeted followup. I thought the problem was in my payload; but it is in the hierarchy.

Environment

• Instance: test
• App: webapp
• Version: 4.6.0

Additional context For our Uganda eCHIS app: regions, districts, subdistricts, counties, subcounties all do not have primary contacts. This currently may block cht-user-management from using cht 4.6 via API.

I believe this was changed here

I'll just note that in the real world it's sometimes quite annoying to need a 100% valid hierarchy in order to complete task. When county-level ICT team staff get an error like this and are blocked from creating a user at this place, what are they supposed to do? It's quite burdensome to train every single person creating users in eCHIS to also manage the hierarchy - this app doesn't have forms to create/edit these places or their primary contacts.

[kennsippell]

Here are the steps that cause this problem in the ug-vht app:

1. Create a new health facility
2. Edit it
3. Now the document has "contact": { _id: "" }

So this only validates primary contacts if they exist and does not affect all existing places in VHT uganda - just edited places.

[jkuester]

Okay, looking closely at the code change Kenn linked to, the relevant change in logic was an additional validation for a place's primary contact. Originally, when validating a place, if the place had a contact, we validated the contact value was either a string or an object. With the updated code, if the place has a contact and the contact is an an object, when we call validatePerson with the contact object. Both before and after the change, the logic for validating a place would recursively validate place.parent if it is populated.

This causes the observed issues in the ug-vht scenario because they have a parent place with a contact value, but the contact is not a valid person....

My main question here is what is the proper "fix" for this issue? It seems kinda confusing to just flag out the place.contact check during the recursive place validations for the place.parent. Do we really want to be doing these recursive validations for place.parent and place.contact at all? It seems like we want to have strict validations for the data on the new place being added, but we for the place.parent/contact values we may just need to run a sub-set of the validations (just enough to ensure we are not writing bad data for the new place). E.g. make sure the parent/contact is a place/person, etc, but do not recurse to the grandparents or try to validate the rest of the fields on the parent/contact.

[mrjones-plip]

@kennsippell - throwing this on your AS Sprint project board - we're blocked on your answer. You can remove from project/unassigned when you're done!

[kennsippell]

My vote would be to the api/v1/people API should do a recursive check of the place's hierarchy: 1) that all places exist and 2) that it is a proper hierarchy following the rules of contact_type parentage rules.

My personal vote would be to do no validation of any child/descendant under that place (including the primary contact).

[garethbowen]

@freddieptf @kennsippell I note that the PR is blocked on a linting error. Is this something we should include in 4.7.0? How close is this to being ready? How can we help?

[freddieptf]

@garethbowen the error message on CI isn't really clear to me, both lint and tests passed before i pushed so not sure why they are failing on CI

[garethbowen]

@freddieptf The log was hard to find, but buried deep in there it looks like you've broken some tests:

  1) places controller
       createPlaces
         supports objects with name and right type.:
     TypeError: Cannot read properties of undefined (reading 'type')
      at preparePlaceContact (src/places.js:11:441)
      at Object.createPlace [as _createPlace] (src/places.js:11:991)

  2) places controller
       createPlaces
         supports parents defined as uuids.:
     TypeError: Cannot read properties of undefined (reading 'type')
      at preparePlaceContact (src/places.js:11:441)
      at Object.createPlace [as _createPlace] (src/places.js:11:991)

  3) places controller
       createPlaces
         accepts valid reported_date in ms since epoch:
     TypeError: Cannot read properties of undefined (reading 'type')
      at preparePlaceContact (src/places.js:11:441)
      at Object.createPlace [as _createPlace] (src/places.js:11:991)

  4) places controller
       createPlaces
         accepts valid reported_date in string format:
     TypeError: Cannot read properties of undefined (reading 'type')
      at preparePlaceContact (src/places.js:11:441)
      at Object.createPlace [as _createPlace] (src/places.js:11:991)

  5) places controller
       createPlaces
         sets a default reported_date.:
     TypeError: Cannot read properties of undefined (reading 'type')
      at preparePlaceContact (src/places.js:11:441)
      at Object.createPlace [as _createPlace] (src/places.js:11:991)

It could be a false negative, but because this is very close to the code you changed it seems legit to me. These should also fail locally if you go to ./shared-libs/contacts and run npm test

Let me know if I can help!

[freddieptf]

@garethbowen thanks for pointing that out, tests now fixed

[garethbowen]

Included in 4.7.0 milestone to get this regression fixed asap.