From 6dec6344c60ca3c36ea267a475336a797a8b4172 Mon Sep 17 00:00:00 2001 From: Fred <10602047+freddieptf@users.noreply.github.com> Date: Wed, 13 Dec 2023 21:33:20 +0300 Subject: [PATCH] fix(#8674): assign parent place to new contacts (#8682) Contacts created via the places api don't get a parent place assigned which causes an error: The contact must be a child of the place, when trying to create a user for the new contact. This PR fixes that. #8674 --- .../extract-person-contacts.spec.js | 9 +- shared-libs/contacts/src/people.js | 1 + shared-libs/contacts/src/places.js | 50 +++-- shared-libs/contacts/test/unit/places.spec.js | 103 +++++++-- .../api/controllers/places.spec.js | 195 ++++++++++++++++++ 5 files changed, 318 insertions(+), 40 deletions(-) create mode 100644 tests/integration/api/controllers/places.spec.js diff --git a/api/tests/integration/migrations/extract-person-contacts.spec.js b/api/tests/integration/migrations/extract-person-contacts.spec.js index 574b9a08e52..1bd952aa0f0 100644 --- a/api/tests/integration/migrations/extract-person-contacts.spec.js +++ b/api/tests/integration/migrations/extract-person-contacts.spec.js @@ -70,7 +70,8 @@ describe('extract-person-contacts migration', () => { name: 'The Clinic', contact: { name: 'Clinic Contact', - phone: '555 1000' + phone: '555 1000', + type: 'person' }, parent: { _id: 'hc-id', @@ -145,7 +146,8 @@ describe('extract-person-contacts migration', () => { name: 'The Health Center', contact: { name: 'HC Contact', - phone: '555 2000' + phone: '555 2000', + type: 'person' }, parent: { _id: 'dh-id', @@ -194,7 +196,8 @@ describe('extract-person-contacts migration', () => { parent: {}, contact: { name: 'DH Contact', - phone: '555 3000' + phone: '555 3000', + type: 'person' } }; const districtHospitalContact = { diff --git a/shared-libs/contacts/src/people.js b/shared-libs/contacts/src/people.js index da9b0f8ba49..701c250c951 100644 --- a/shared-libs/contacts/src/people.js +++ b/shared-libs/contacts/src/people.js @@ -110,3 +110,4 @@ module.exports.isAPerson = isAPerson; module.exports._getPerson = getPerson; module.exports._validatePerson = validatePerson; +module.exports._getDefaultPersonType = getDefaultPersonType; diff --git a/shared-libs/contacts/src/places.js b/shared-libs/contacts/src/places.js index 14c6a715830..6674b2107d5 100644 --- a/shared-libs/contacts/src/places.js +++ b/shared-libs/contacts/src/places.js @@ -17,7 +17,7 @@ const getPlace = id => { }) .catch(err => { if (err.status === 404) { - err.message = 'Failed to find place.'; + err.message = 'Failed to find place.'; } throw err; }); @@ -75,6 +75,12 @@ const validatePlace = place => { if (!_.isString(place.contact) && !_.isObject(place.contact)) { return err(`Property "contact" on place ${placeId} must be an object or string.`); } + if (_.isObject(place.contact)) { + const errStr = people._validatePerson(place.contact); + if (errStr) { + return err(errStr); + } + } } if (place.parent && !_.isEmpty(place.parent)) { // validate parents @@ -83,23 +89,31 @@ const validatePlace = place => { return Promise.resolve(); }; -const createPlace = place => { - const self = module.exports; - return self._validatePlace(place) - .then(() => { - const date = place.reported_date ? utils.parseDate(place.reported_date) : new Date(); - place.reported_date = date.valueOf(); - if (place.parent) { - place.parent = lineage.minifyLineage(place.parent); - } - if (place.contact) { - // also validates contact if creating - return people.getOrCreatePerson(place.contact).then(person => { - place.contact = lineage.minifyLineage(person); - }); - } - }) - .then(() => db.medic.post(place)); +const createPlace = async (place) => { + if (place.contact && !place.contact.type) { + place.contact.type = people._getDefaultPersonType(); + } + await module.exports._validatePlace(place); + + const contact = place.contact; + delete place.contact; + + const date = place.reported_date ? utils.parseDate(place.reported_date) : new Date(); + place.reported_date = date.valueOf(); + if (place.parent) { + place.parent = lineage.minifyLineage(place.parent); + } + + const response = await db.medic.post(place); + if (!contact) { + return response; + } + const placeUUID = response.id; + + contact.place = placeUUID; + const person = await people.getOrCreatePerson(contact); + const result = await updatePlace(placeUUID, { contact: person._id }); + return { ...result, contact: { id: person._id } }; }; /* diff --git a/shared-libs/contacts/test/unit/places.spec.js b/shared-libs/contacts/test/unit/places.spec.js index 224b4ebe21d..720cf71fbb8 100644 --- a/shared-libs/contacts/test/unit/places.spec.js +++ b/shared-libs/contacts/test/unit/places.spec.js @@ -26,7 +26,7 @@ const contactTypes = [ group_key: 'contact.type.health_center.plural', create_key: 'contact.type.health_center.new', edit_key: 'contact.type.place.edit', - parents: [ 'district_hospital' ], + parents: ['district_hospital'], icon: 'medic-health-center', create_form: 'form:contact:health_center:create', edit_form: 'form:contact:health_center:edit' @@ -37,7 +37,7 @@ const contactTypes = [ group_key: 'contact.type.clinic.plural', create_key: 'contact.type.clinic.new', edit_key: 'contact.type.place.edit', - parents: [ 'health_center' ], + parents: ['health_center'], icon: 'medic-clinic', create_form: 'form:contact:clinic:create', edit_form: 'form:contact:clinic:edit', @@ -50,7 +50,7 @@ const contactTypes = [ create_key: 'contact.type.person.new', edit_key: 'contact.type.person.edit', primary_contact_key: 'clinic.field.contact', - parents: [ 'district_hospital', 'health_center', 'clinic' ], + parents: ['district_hospital', 'health_center', 'clinic'], icon: 'medic-person', create_form: 'form:contact:person:create', edit_form: 'form:contact:person:edit', @@ -263,14 +263,14 @@ describe('places controller', () => { }; db.medic.post.callsFake(doc => { if (doc.name === 'CHP Branch One') { - return Promise.resolve({id: 'abc'}); + return Promise.resolve({ id: 'abc' }); } if (doc.name === 'CHP Area One') { // the parent should be created/resolved, parent id should be set. chai.expect(doc.parent._id).to.equal('abc'); chai.expect(doc.parent.name).to.equal(undefined); // minified chai.expect(doc.parent.type).to.equal(undefined); // minified - return Promise.resolve({id: 'def'}); + return Promise.resolve({ id: 'def' }); } if (doc.name === 'CHP Family') { // both parents should be created/resolved @@ -280,7 +280,7 @@ describe('places controller', () => { chai.expect(doc.parent.parent._id).to.equal('abc'); chai.expect(doc.parent.parent.name).to.equal(undefined); // minified chai.expect(doc.parent.parent.type).to.equal(undefined); // minified - return Promise.resolve({id: 'ghi'}); + return Promise.resolve({ id: 'ghi' }); } }); fetchHydratedDoc.callsFake(id => { @@ -322,7 +322,7 @@ describe('places controller', () => { } }); return controller._createPlaces(place).then(actual => { - chai.expect(actual).to.deep.equal({id: 'ghi'}); + chai.expect(actual).to.deep.equal({ id: 'ghi' }); }); }); @@ -336,27 +336,92 @@ describe('places controller', () => { type: 'person' } }; + sinon.stub(people, 'getOrCreatePerson').resolves({ _id: 'qwe', + _rev: '1', name: 'Jim', type: 'person' }); - fetchHydratedDoc.resolves({ - _id: 'ad06d137', - name: 'CHP Branch One', - type: 'district_hospital' + db.medic.post.withArgs( + sinon.match((doc) => !doc.contact) + ).callsFake(doc => { + chai.expect(doc.name).to.equal('CHP Family'); + chai.expect(doc.parent._id).to.equal('ad06d137'); + return Promise.resolve({ id: 'hc', rev: '1' }); }); - db.medic.post.callsFake(doc => { + db.medic.post.withArgs( + sinon.match(doc => doc.contact) + ).callsFake(doc => { + chai.expect(doc.name).to.equal('CHP Family'); + chai.expect(doc.parent._id).to.equal('ad06d137'); chai.expect(doc.contact._id).to.equal('qwe'); chai.expect(doc.contact.name).to.equal(undefined); // minified - chai.expect(doc.contact.type).to.equal(undefined); // minified - return Promise.resolve({id: 'ghi'}); + chai.expect(doc.contact.type).to.equal(undefined); // + return Promise.resolve({ id: 'hc', rev: '2' }); }); + + fetchHydratedDoc.withArgs('hc').resolves({ + name: 'CHP Family', + type: 'health_center', + parent: { + _id: 'ad06d137', + name: 'CHP Branch One', + type: 'district_hospital' + } + }); + fetchHydratedDoc.withArgs('ad06d137').resolves({ + _id: 'ad06d137', + name: 'CHP Branch One', + type: 'district_hospital' + }); + return controller._createPlaces(place).then(actual => { - chai.expect(actual).to.deep.equal({id: 'ghi'}); + chai.expect(db.medic.post.callCount).to.equal(2); + chai.expect(actual).to.deep.equal({ + id: 'hc', + rev: '2', + contact: { + id: 'qwe', + } + }); + }); + }); + + it('returns err if contact does not have name', done => { + const place = { + name: 'HC', + type: 'district_hospital', + contact: { + type: 'person' + } + }; + const post = db.medic.post; + controller._createPlaces(place).catch(err => { + chai.expect(err.message).to.equal('Person is missing a "name" property.'); + chai.expect(post.callCount).to.equal(0); + done(); }); }); + it('rejects contacts with wrong type', done => { + const place = { + name: 'HC', + type: 'district_hospital', + contact: { + name: 'John Doe', + type: 'x' + } + }; + const post = db.medic.post; + controller._createPlaces(place).catch(err => { + chai.expect(err.message).to.equal('Wrong type, this is not a person.'); + chai.expect(post.callCount).to.equal(0); + done(); + }); + }); + + it('supports parents defined as uuids.', () => { const place = { name: 'CHP Area One', @@ -375,10 +440,10 @@ describe('places controller', () => { chai.expect(doc.parent._id).to.equal('ad06d137'); chai.expect(doc.parent.name).to.equal(undefined); // minified chai.expect(doc.parent.type).to.equal(undefined); // minified - return Promise.resolve({id: 'abc123'}); + return Promise.resolve({ id: 'abc123' }); }); return controller._createPlaces(place).then(actual => { - chai.expect(actual).to.deep.equal({id: 'abc123'}); + chai.expect(actual).to.deep.equal({ id: 'abc123' }); }); }); @@ -457,7 +522,7 @@ describe('places controller', () => { db.medic.post.callsFake(doc => { chai.expect(doc.contact._id).to.equal('a'); chai.expect(doc.contact.name).to.equal(undefined); // minified - return Promise.resolve({id: 'x', rev: 'y'}); + return Promise.resolve({ id: 'x', rev: 'y' }); }); return controller.updatePlace('123', data).then(actual => { chai.expect(actual).to.deep.equal({ id: 'x', rev: 'y' }); @@ -474,7 +539,7 @@ describe('places controller', () => { db.medic.post.callsFake(doc => { chai.expect(doc.parent._id).to.equal('a'); chai.expect(doc.parent.name).to.equal(undefined); // minified - return Promise.resolve({id: 'x', rev: 'y'}); + return Promise.resolve({ id: 'x', rev: 'y' }); }); return controller.updatePlace('123', data).then(actual => { chai.expect(actual).to.deep.equal({ id: 'x', rev: 'y' }); diff --git a/tests/integration/api/controllers/places.spec.js b/tests/integration/api/controllers/places.spec.js new file mode 100644 index 00000000000..2833ccb00e9 --- /dev/null +++ b/tests/integration/api/controllers/places.spec.js @@ -0,0 +1,195 @@ +const chai = require('chai'); +const utils = require('@utils'); +const chaiExclude = require('chai-exclude'); +chai.use(chaiExclude); + +const password = 'passwordSUP3RS3CR37!'; + +const users = [ + { + username: 'online', + password: password, + place: { + _id: 'fixture:online', + type: 'district_hospital', + name: 'Online place', + }, + contact: { + _id: 'fixture:user:online', + name: 'OnlineUser', + }, + roles: ['national_admin'], + }, +]; + +let onlineRequestOptions; + +describe('Places API', () => { + before(async () => { + const settings = await utils.getSettings(); + const permissions = { + ...settings.permissions, + 'can_create_places': ['national_admin'], + }; + await utils.updateSettings({ permissions }, true); + await utils.createUsers(users); + }); + + after(async () => { + await utils.deleteUsers(users); + await utils.revertSettings(true); + }); + + beforeEach(() => { + onlineRequestOptions = { path: '/api/v1/places', auth: { username: 'online', password }, }; + }); + + describe('POST', () => { + beforeEach(() => { + onlineRequestOptions.method = 'POST'; + }); + + it('should create place', () => { + onlineRequestOptions.body = { + name: 'CHP Branch One', + type: 'district_hospital' + }; + return utils.request(onlineRequestOptions) + .then(result => { + chai.expect(result.id).to.not.be.undefined; + return utils.getDoc(result.id); + }) + .then((place) => { + chai.expect(place).to.deep.include({ + name: 'CHP Branch One', + type: 'district_hospital' + }); + }); + }); + + it('should create place with parent', () => { + onlineRequestOptions.body = { + name: 'CHP Area One', + type: 'health_center', + parent: { + name: 'CHP Branch One', + type: 'district_hospital' + } + }; + return utils.request(onlineRequestOptions) + .then(result => { + chai.expect(result.id).to.not.be.undefined; + return utils.getDoc(result.id); + }) + .then((place) => { + chai.expect(place).to.deep.include({ + name: 'CHP Area One', + type: 'health_center', + }); + expect(place.parent._id).to.be.a('string'); + return utils.getDoc(place.parent._id); + }) + .then((parent) => { + chai.expect(parent).to.deep.include({ + name: 'CHP Branch One', + type: 'district_hospital' + }); + }); + }); + + it('should create place with contact', () => { + onlineRequestOptions.body = { + name: 'CHP Area One', + type: 'health_center', + parent: 'fixture:online', + contact: { + name: 'Paul', + phone: '+254883720611' + } + }; + return utils.request(onlineRequestOptions) + .then(result => { + chai.expect(result.id).to.not.be.undefined; + chai.expect(result.contact.id).to.not.be.undefined; + return utils.getDocs([result.id, result.contact.id]); + }) + .then(([place, contact]) => { + chai.expect(contact).to.deep.include({ + name: 'Paul', + phone: '+254883720611', + parent: { _id: place._id, parent: place.parent }, + type: 'person', + }); + chai.expect(place).to.deep.include({ + name: 'CHP Area One', + type: 'health_center', + contact: { _id: contact._id, parent: contact.parent }, + parent: { + _id: 'fixture:online' + } + }); + }); + }); + + it('should create place with contact uuid', () => { + onlineRequestOptions.body = { + name: 'DS', + type: 'district_hospital', + contact: 'fixture:user:online' + }; + return utils.request(onlineRequestOptions) + .then(result => { + chai.expect(result.id).to.not.be.undefined; + chai.expect(result.contact.id).to.not.be.undefined; + return utils.getDocs([result.id, result.contact.id]); + }) + .then(([place, contact]) => { + chai.expect(contact).to.deep.include({ + name: 'OnlineUser', + parent: { _id: 'fixture:online' }, + type: 'person', + }); + chai.expect(place).to.deep.include({ + name: 'DS', + type: 'district_hospital', + contact: { _id: contact._id, parent: { _id: contact.parent._id } } + }); + }); + }); + + it('should fail if place contact is not a person type', () => { + onlineRequestOptions.body = { + name: 'CHP Area One', + type: 'health_center', + parent: 'fixture:online', + contact: { + name: 'Paul', + phone: '+254883720611', + type: 'health_center', + } + }; + return utils.request(onlineRequestOptions) + .then(() => fail('Call should fail as contact type is not a person')) + .catch(err => { + chai.expect(err.responseBody.error).to.equal('Wrong type, this is not a person.'); + }); + + }); + + it('should fail if place contact does not exist', () => { + onlineRequestOptions.body = { + name: 'CHP Area One', + type: 'health_center', + parent: 'fixture:online', + contact: 'x' + }; + return utils.request(onlineRequestOptions) + .then(() => fail('Call should fail as contact does not exist')) + .catch(err => { + chai.expect(err.responseBody.error).to.equal('Failed to find person.'); + }); + + }); + }); + +});