From 4b3a3a15cbaef1bfd4f810a3427c40b50a5b4c3f Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 13 Mar 2024 17:53:00 +0100 Subject: [PATCH 01/65] move the auth check up in the parent function --- api/src/controllers/users.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index 48391920d76..853a31a396e 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -110,8 +110,7 @@ const getAllowedDocsCounts = async (userCtx) => { // In express4, req.host strips off the port number: https://expressjs.com/en/guide/migrating-5.html#req.host const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; -const getUserList = async (req) => { - await auth.check(req, 'can_view_users'); +const getUserList = async () => { return await users.getList(); }; @@ -132,7 +131,8 @@ const convertUserListToV1 = (users=[]) => { module.exports = { get: (req, res) => { - return getUserList(req) + return auth.check(req, 'can_view_users') + .then(() => getUserList()) .then(list => convertUserListToV1(list)) .then(body => res.json(body)) .catch(err => serverUtils.error(err, req, res)); @@ -233,7 +233,8 @@ module.exports = { v2: { get: async (req, res) => { try { - const body = await getUserList(req, res); + await auth.check(req, 'can_view_users'); + const body = await getUserList(); res.json(body); } catch (err) { serverUtils.error(err, req, res); From 581915181e1e69f3069f7d263bf7612f0bd0e27a Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 13 Mar 2024 17:54:14 +0100 Subject: [PATCH 02/65] support `facility_id` query parameter --- api/src/controllers/users.js | 10 ++++--- shared-libs/user-management/src/users.js | 34 +++++++++++++++--------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index 853a31a396e..98a4dd59b83 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -110,8 +110,8 @@ const getAllowedDocsCounts = async (userCtx) => { // In express4, req.host strips off the port number: https://expressjs.com/en/guide/migrating-5.html#req.host const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; -const getUserList = async () => { - return await users.getList(); +const getUserList = async (filters = {}) => { + return await users.getList(filters); }; const getType = user => { @@ -234,7 +234,11 @@ module.exports = { get: async (req, res) => { try { await auth.check(req, 'can_view_users'); - const body = await getUserList(); + const filters = _.chain(req.query) + .pick(['facility_id', 'contact_id']) + .mapKeys((value, key) => _.camelCase(key)) + .value(); + const body = await getUserList(filters); res.json(body); } catch (err) { serverUtils.error(err, req, res); diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 0bfbafb4c2c..7dcad7ee8b5 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -114,9 +114,17 @@ const getAllUserSettings = () => { .then(result => result.rows.map(row => row.doc)); }; -const getAllUsers = () => { - return db.users.allDocs({ include_docs: true }) - .then(result => result.rows); +const getAllUsers = ({ facilityId }) => { + if (!facilityId) { + return db.users.allDocs({ include_docs: true }) + .then(result => result.rows.map(({ doc }) => doc)); + } + + return db.users.find({ + selector: { + facility_id: facilityId, + }, + }).then(result => result.docs); }; const validateContact = (id, placeID) => { @@ -335,21 +343,21 @@ const hasParent = (facility, id) => { const mapUsers = (users, settings, facilities) => { users = users || []; return users - .filter(user => user.id.indexOf(USER_PREFIX) === 0) + .filter(user => user._id.indexOf(USER_PREFIX) === 0) .map(user => { - const setting = getDoc(user.id, settings) || {}; + const setting = getDoc(user._id, settings) || {}; return { - id: user.id, - rev: user.doc._rev, - username: user.doc.name, + id: user._id, + rev: user._rev, + username: user.name, fullname: setting.fullname, email: setting.email, phone: setting.phone, - place: getDoc(user.doc.facility_id, facilities), - roles: user.doc.roles, + place: getDoc(user.facility_id, facilities), + roles: user.roles, contact: getDoc(setting.contact_id, facilities), external_id: setting.external_id, - known: user.doc.known + known: user.known }; }); }; @@ -770,8 +778,8 @@ const getUserSettings = async({ name }) => { */ module.exports = { deleteUser: username => deleteUser(createID(username)), - getList: async () => { - const [ users, settings ] = await Promise.all([ getAllUsers(), getAllUserSettings() ]); + getList: async (filters) => { + const [ users, settings ] = await Promise.all([ getAllUsers(filters), getAllUserSettings() ]); const facilities = await facility.list(users, settings); return mapUsers(users, settings, facilities); }, From a007db94749d3cf8114773a83a31d8cbc549956e Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 14 Mar 2024 14:49:33 +0100 Subject: [PATCH 03/65] support `facility_id` query parameter for user settings too --- .../medic-db/medic-client/views/doc_by_type/map.js | 5 +++++ shared-libs/user-management/src/users.js | 13 ++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ddocs/medic-db/medic-client/views/doc_by_type/map.js b/ddocs/medic-db/medic-client/views/doc_by_type/map.js index b13e6644401..ca768a975f8 100644 --- a/ddocs/medic-db/medic-client/views/doc_by_type/map.js +++ b/ddocs/medic-db/medic-client/views/doc_by_type/map.js @@ -6,5 +6,10 @@ function(doc) { }); return; } + if (doc.type === 'user-settings') { + emit([ doc.type ]); + emit([ doc.type, doc.facility_id ]); + return; + } emit([ doc.type ]); } diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 7dcad7ee8b5..31bedb4e960 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -105,10 +105,15 @@ const getDocID = doc => { } }; -const getAllUserSettings = () => { +const getAllUserSettings = ({ facilityId }) => { + const key = ['user-settings']; + if (facilityId) { + key.push(facilityId); + } + const opts = { include_docs: true, - key: ['user-settings'] + key, }; return db.medic.query('medic-client/doc_by_type', opts) .then(result => result.rows.map(row => row.doc)); @@ -779,7 +784,9 @@ const getUserSettings = async({ name }) => { module.exports = { deleteUser: username => deleteUser(createID(username)), getList: async (filters) => { - const [ users, settings ] = await Promise.all([ getAllUsers(filters), getAllUserSettings() ]); + const [ users, settings ] = await Promise.all([ getAllUsers(filters), getAllUserSettings(filters) ]); + console.log("users", users); + console.log("settings", settings); const facilities = await facility.list(users, settings); return mapUsers(users, settings, facilities); }, From 14281f67c385016a1af58e4e5c4e7437da07b01a Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 14 Mar 2024 14:53:31 +0100 Subject: [PATCH 04/65] support filtering with `contact_id` query parameter --- ddocs/medic-db/medic-client/views/doc_by_type/map.js | 2 ++ shared-libs/user-management/src/users.js | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ddocs/medic-db/medic-client/views/doc_by_type/map.js b/ddocs/medic-db/medic-client/views/doc_by_type/map.js index ca768a975f8..52572ae6c74 100644 --- a/ddocs/medic-db/medic-client/views/doc_by_type/map.js +++ b/ddocs/medic-db/medic-client/views/doc_by_type/map.js @@ -9,6 +9,8 @@ function(doc) { if (doc.type === 'user-settings') { emit([ doc.type ]); emit([ doc.type, doc.facility_id ]); + emit([ doc.type, doc.contact_id ]); + emit([ doc.type, doc.facility_id, doc.contact_id ]); return; } emit([ doc.type ]); diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 31bedb4e960..6bf24996563 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -105,11 +105,14 @@ const getDocID = doc => { } }; -const getAllUserSettings = ({ facilityId }) => { +const getAllUserSettings = ({ facilityId, contactId }) => { const key = ['user-settings']; if (facilityId) { key.push(facilityId); } + if (contactId) { + key.push(contactId); + } const opts = { include_docs: true, @@ -119,7 +122,8 @@ const getAllUserSettings = ({ facilityId }) => { .then(result => result.rows.map(row => row.doc)); }; -const getAllUsers = ({ facilityId }) => { +const getAllUsers = ({ facilityId, /*contactId*/ }) => { + // TODO: ? do something with contactId if (!facilityId) { return db.users.allDocs({ include_docs: true }) .then(result => result.rows.map(({ doc }) => doc)); From c6ac0f3a621abc571cafe141a7dab454a96dd329 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 09:13:50 +0100 Subject: [PATCH 05/65] cleaner approach with/without filters --- shared-libs/user-management/src/users.js | 35 ++++++++++++++---------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 6bf24996563..ebb478fe289 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -122,18 +122,17 @@ const getAllUserSettings = ({ facilityId, contactId }) => { .then(result => result.rows.map(row => row.doc)); }; -const getAllUsers = ({ facilityId, /*contactId*/ }) => { - // TODO: ? do something with contactId - if (!facilityId) { - return db.users.allDocs({ include_docs: true }) - .then(result => result.rows.map(({ doc }) => doc)); - } +const getAllUsers = () => { + return db.users.allDocs({ include_docs: true }) + .then(result => result.rows.map(({ doc }) => doc)); +}; - return db.users.find({ - selector: { - facility_id: facilityId, - }, - }).then(result => result.docs); +const getUsersByIds = async (ids) => { + const docs = ids.map(id => ({ id })); + const { results } = await db.users.bulkGet({ docs }); + return results + .map(result => result.docs && result.docs[0] && result.docs[0].ok) + .filter(doc => doc); }; const validateContact = (id, placeID) => { @@ -788,9 +787,17 @@ const getUserSettings = async({ name }) => { module.exports = { deleteUser: username => deleteUser(createID(username)), getList: async (filters) => { - const [ users, settings ] = await Promise.all([ getAllUsers(filters), getAllUserSettings(filters) ]); - console.log("users", users); - console.log("settings", settings); + let users; + let settings; + + if (_.isEmpty(filters)) { + [users, settings] = await Promise.all([getAllUsers(filters), getAllUserSettings(filters)]); + } else { + settings = await getAllUserSettings(filters); + const ids = settings.map(({ _id }) => _id); + users = await getUsersByIds(ids); + } + const facilities = await facility.list(users, settings); return mapUsers(users, settings, facilities); }, From 568408942d48f8145cd83846133fbc1505a4ffd7 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 09:17:16 +0100 Subject: [PATCH 06/65] good catch sonar --- shared-libs/user-management/src/users.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index ebb478fe289..6b3161e11b1 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -105,7 +105,7 @@ const getDocID = doc => { } }; -const getAllUserSettings = ({ facilityId, contactId }) => { +const getAllUserSettings = ({ facilityId, contactId } = {}) => { const key = ['user-settings']; if (facilityId) { key.push(facilityId); @@ -791,7 +791,7 @@ module.exports = { let settings; if (_.isEmpty(filters)) { - [users, settings] = await Promise.all([getAllUsers(filters), getAllUserSettings(filters)]); + [users, settings] = await Promise.all([getAllUsers(), getAllUserSettings()]); } else { settings = await getAllUserSettings(filters); const ids = settings.map(({ _id }) => _id); From a993c0f094bb6269bfae707613ee0d5ee85225fc Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 09:45:11 +0100 Subject: [PATCH 07/65] fix unit tests --- .../user-management/test/unit/users.spec.js | 62 ++++++++----------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index c041adca326..784193f88c8 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -193,20 +193,16 @@ describe('Users service', () => { it('collects user infos', () => { const allUsers = [ { - id: 'org.couchdb.user:x', - doc: { - name: 'lucas', - facility_id: 'c', - roles: [ 'national-admin', 'data-entry' ] - } + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'] }, { - id: 'org.couchdb.user:y', - doc: { - name: 'milan', - facility_id: 'b', - roles: [ 'district-admin' ] - } + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'] } ]; const allUsersSettings = [ @@ -253,26 +249,22 @@ describe('Users service', () => { it('filters out non-users', () => { const allUsers = [ { - id: 'x', - doc: { - name: 'lucas', - facility_id: 'c', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - roles: [ 'national-admin', 'data-entry' ] - } + _id: 'x', + name: 'lucas', + facility_id: 'c', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', + roles: ['national-admin', 'data-entry'] }, { - id: 'org.couchdb.user:y', - doc: { - name: 'milan', - facility_id: 'b', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - roles: [ 'district-admin' ] - } + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + roles: ['district-admin'] } ]; const allUserSettings = [ @@ -311,10 +303,8 @@ describe('Users service', () => { it('handles minimal users', () => { const allUsers = [ { - id: 'org.couchdb.user:x', - doc: { - name: 'lucas' - } + _id: 'org.couchdb.user:x', + name: 'lucas' } ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); @@ -343,7 +333,7 @@ describe('Users service', () => { it('returns errors from facilities service', () => { const allUsers = [ { - id: 'x', + _id: 'x', doc: { name: 'lucas', facility_id: 'c', @@ -354,7 +344,7 @@ describe('Users service', () => { } }, { - id: 'org.couchdb.user:y', + _id: 'org.couchdb.user:y', doc: { name: 'milan', facility_id: 'b', From fd18a9cb709530476e7f038cd0de623b7147198f Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 13:38:42 +0100 Subject: [PATCH 08/65] unit test `getList()` with filters --- .../user-management/test/unit/users.spec.js | 428 ++++++++++++------ 1 file changed, 277 insertions(+), 151 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 784193f88c8..5e93a5bf978 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -16,6 +16,11 @@ const COMPLEX_PASSWORD = '23l4ijk3nSDELKSFnwekirh'; const facilitya = { _id: 'a', name: 'aaron' }; const facilityb = { _id: 'b', name: 'brian' }; const facilityc = { _id: 'c', name: 'cathy' }; +const contactMilan = { + _id: 'milan-contact', + type: 'person', + name: 'milan', +}; let userData; let clock; @@ -43,6 +48,7 @@ describe('Users service', () => { facilitya, facilityb, facilityc, + contactMilan, ]); sinon.stub(couchSettings, 'getCouchConfig').resolves(); userData = { @@ -190,176 +196,296 @@ describe('Users service', () => { describe('getList', () => { - it('collects user infos', () => { - const allUsers = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas', - facility_id: 'c', - roles: ['national-admin', 'data-entry'] - }, - { - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - roles: ['district-admin'] - } - ]; - const allUsersSettings = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789' - }, - { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093' - } - ]; - service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); - service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); - return service.getList().then(data => { - chai.expect(data.length).to.equal(2); - const lucas = data[0]; - chai.expect(lucas.id).to.equal('org.couchdb.user:x'); - chai.expect(lucas.username).to.equal('lucas'); - chai.expect(lucas.fullname).to.equal('Lucas M'); - chai.expect(lucas.email).to.equal('l@m.com'); - chai.expect(lucas.phone).to.equal('123456789'); - chai.expect(lucas.place).to.deep.equal(facilityc); - chai.expect(lucas.roles).to.deep.equal([ 'national-admin', 'data-entry' ]); - const milan = data[1]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); - chai.expect(milan.external_id).to.equal('LTT093'); - }); - }); - - it('filters out non-users', () => { - const allUsers = [ - { - _id: 'x', - name: 'lucas', - facility_id: 'c', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - roles: ['national-admin', 'data-entry'] - }, - { - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - roles: ['district-admin'] - } - ]; - const allUserSettings = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789' - }, - { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321' - } - ]; - service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); - service.__set__('getAllUserSettings', sinon.stub().resolves(allUserSettings)); - - return service.getList().then(data => { - chai.expect(data.length).to.equal(1); - const milan = data[0]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); + describe('with filters', () => { + it('with facility_id', () => { + const filters = { facilityId: 'c' }; + const usersByIds = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'] + }, + ]; + const allUsersSettings = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789' + }, + ]; + const getUsersByIds = sinon.stub().resolves(usersByIds); + service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); + service.__set__('getUsersByIds', getUsersByIds); + return service.getList(filters).then(data => { + chai.expect(data.length).to.equal(1); + chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:x'); + const lucas = data[0]; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal('Lucas M'); + chai.expect(lucas.email).to.equal('l@m.com'); + chai.expect(lucas.phone).to.equal('123456789'); + chai.expect(lucas.place).to.deep.equal(facilityc); + chai.expect(lucas.roles).to.deep.equal(['national-admin', 'data-entry']); + }); }); - }); + it('with contact_id', () => { + const filters = { contactId: 'c' }; + const usersByIds = [ + { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'], + }, + ]; + const allUsersSettings = [ + { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, + ]; + const getUsersByIds = sinon.stub().resolves(usersByIds); + service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); + service.__set__('getUsersByIds', getUsersByIds); + return service.getList(filters).then(data => { + chai.expect(data.length).to.equal(1); + chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:y'); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.contact._id).to.equal('milan-contact'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal(['district-admin']); + }); + }); - it('handles minimal users', () => { - const allUsers = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas' - } - ]; - service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); - service.__set__('getAllUserSettings', sinon.stub().resolves([])); - return service.getList().then(data => { - chai.expect(data.length).to.equal(1); - const lucas = data[0]; - chai.expect(lucas.id).to.equal('org.couchdb.user:x'); - chai.expect(lucas.username).to.equal('lucas'); - chai.expect(lucas.fullname).to.equal(undefined); - chai.expect(lucas.email).to.equal(undefined); - chai.expect(lucas.phone).to.equal(undefined); - chai.expect(lucas.facility).to.equal(undefined); - chai.expect(lucas.roles).to.equal(undefined); + it('with both facility_id and contact_id', () => { + const filters = { facilityId: 'b', contactId: 'c' }; + const usersByIds = [ + { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'], + }, + ]; + const allUsersSettings = [ + { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, + ]; + const getUsersByIds = sinon.stub().resolves(usersByIds); + service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); + service.__set__('getUsersByIds', getUsersByIds); + return service.getList(filters).then(data => { + chai.expect(data.length).to.equal(1); + chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:y'); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.contact._id).to.equal('milan-contact'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal(['district-admin']); + }); }); }); - it('returns errors from users service', () => { - service.__set__('getAllUsers', sinon.stub().rejects('not found')); - service.__set__('getAllUserSettings', sinon.stub().rejects('not found')); - return service.getList().catch(err => { - chai.expect(err.name).to.equal('not found'); + describe('without filters', () => { + it('collects user infos', () => { + const allUsers = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'] + }, + { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'] + } + ]; + const allUsersSettings = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789' + }, + { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093' + } + ]; + service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); + service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); + return service.getList().then(data => { + chai.expect(data.length).to.equal(2); + const lucas = data[0]; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal('Lucas M'); + chai.expect(lucas.email).to.equal('l@m.com'); + chai.expect(lucas.phone).to.equal('123456789'); + chai.expect(lucas.place).to.deep.equal(facilityc); + chai.expect(lucas.roles).to.deep.equal([ 'national-admin', 'data-entry' ]); + const milan = data[1]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); + chai.expect(milan.external_id).to.equal('LTT093'); + }); }); - }); - it('returns errors from facilities service', () => { - const allUsers = [ - { - _id: 'x', - doc: { + it('filters out non-users', () => { + const allUsers = [ + { + _id: 'x', name: 'lucas', facility_id: 'c', fullname: 'Lucas M', email: 'l@m.com', phone: '123456789', - roles: [ 'national-admin', 'data-entry' ] - } - }, - { - _id: 'org.couchdb.user:y', - doc: { + roles: ['national-admin', 'data-entry'] + }, + { + _id: 'org.couchdb.user:y', name: 'milan', facility_id: 'b', fullname: 'Milan A', email: 'm@a.com', phone: '987654321', - roles: [ 'district-admin' ] + roles: ['district-admin'] } - } - ]; - service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); - service.__set__('getAllUserSettings', sinon.stub().resolves([])); - service.__set__('getFacilities', sinon.stub().rejects('BOOM')); - return service.getList().catch(err => { - chai.expect(err.name).to.equal('BOOM'); + ]; + const allUserSettings = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789' + }, + { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321' + } + ]; + service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); + service.__set__('getAllUserSettings', sinon.stub().resolves(allUserSettings)); + + return service.getList().then(data => { + chai.expect(data.length).to.equal(1); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); + }); + + }); + + it('handles minimal users', () => { + const allUsers = [ + { + _id: 'org.couchdb.user:x', + name: 'lucas' + } + ]; + service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); + service.__set__('getAllUserSettings', sinon.stub().resolves([])); + return service.getList().then(data => { + chai.expect(data.length).to.equal(1); + const lucas = data[0]; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal(undefined); + chai.expect(lucas.email).to.equal(undefined); + chai.expect(lucas.phone).to.equal(undefined); + chai.expect(lucas.facility).to.equal(undefined); + chai.expect(lucas.roles).to.equal(undefined); + }); + }); + + it('returns errors from users service', () => { + service.__set__('getAllUsers', sinon.stub().rejects('not found')); + service.__set__('getAllUserSettings', sinon.stub().rejects('not found')); + return service.getList().catch(err => { + chai.expect(err.name).to.equal('not found'); + }); + }); + + it('returns errors from facilities service', () => { + const allUsers = [ + { + _id: 'x', + doc: { + name: 'lucas', + facility_id: 'c', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', + roles: [ 'national-admin', 'data-entry' ] + } + }, + { + _id: 'org.couchdb.user:y', + doc: { + name: 'milan', + facility_id: 'b', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + roles: [ 'district-admin' ] + } + } + ]; + service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); + service.__set__('getAllUserSettings', sinon.stub().resolves([])); + service.__set__('getFacilities', sinon.stub().rejects('BOOM')); + return service.getList().catch(err => { + chai.expect(err.name).to.equal('BOOM'); + }); }); }); From d117a4974ed9ca214053cddf3819227aa7f8fc4b Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 14:03:21 +0100 Subject: [PATCH 09/65] test api controller with filters --- api/tests/mocha/controllers/users.spec.js | 71 ++++++++++++++++++++--- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/api/tests/mocha/controllers/users.spec.js b/api/tests/mocha/controllers/users.spec.js index dbcb284191f..9286a28e87f 100644 --- a/api/tests/mocha/controllers/users.spec.js +++ b/api/tests/mocha/controllers/users.spec.js @@ -30,16 +30,25 @@ describe('Users Controller', () => { }); afterEach(() => sinon.restore()); - describe('get users list', () => { + describe.only('get users list', () => { + let userList; beforeEach(() => { + userList = [ + { id: 'org.couchdb.user:admin', roles: ['_admin'] }, + { + id: 'org.couchdb.user:chw', + roles: ['chw', 'district-admin'], + contact: { + _id: 'chw-contact', + parent: { _id: 'chw-facility' }, + } + }, + { id: 'org.couchdb.user:unknown' }, + ]; req = { }; res = { json: sinon.stub() }; - sinon.stub(users, 'getList').resolves([ - { id: 'org.couchdb.user:admin', roles: [ '_admin' ] }, - { id: 'org.couchdb.user:chw', roles: [ 'chw', 'district-admin' ] }, - { id: 'org.couchdb.user:unknown' }, - ]); + sinon.stub(users, 'getList').resolves(userList); }); describe('v1', () => { @@ -69,14 +78,13 @@ describe('Users Controller', () => { }); describe('v2', () => { - it('rejects if not permitted', async () => { sinon.stub(auth, 'check').rejects(new Error('nope')); await controller.v2.get(req, res); chai.expect(serverUtils.error.callCount).to.equal(1); }); - it('gets the list of users', async () => { + it('gets the list of users without filters', async () => { sinon.stub(auth, 'check').resolves(); await controller.v2.get(req, res); @@ -92,6 +100,53 @@ describe('Users Controller', () => { chai.expect(result[2].roles).to.be.undefined; }); + it('gets the list of users with facility_id filter', async () => { + sinon.stub(auth, 'check').resolves(); + users.getList.resolves([userList[1]]); + req.query = { facility_id: 'chw-facility' }; + + await controller.v2.get(req, res); + chai.expect(users.getList.firstCall.args[0]).to.deep.equal({ facilityId: 'chw-facility' }); + const result = res.json.args[0][0]; + chai.expect(result.length).to.equal(1); + chai.expect(result[0].id).to.equal('org.couchdb.user:chw'); + chai.expect(result[0].type).to.be.undefined; + chai.expect(result[0].roles).to.deep.equal(['chw', 'district-admin']); + chai.expect(result[0].contact._id).to.equal('chw-contact'); + chai.expect(result[0].contact.parent._id).to.equal('chw-facility'); + }); + + it('gets the list of users with facility_id and contact_id filters', async () => { + sinon.stub(auth, 'check').resolves(); + users.getList.resolves([userList[1]]); + req.query = { facility_id: 'chw-facility', contact_id: 'chw-contact' }; + + await controller.v2.get(req, res); + chai.expect(users.getList.firstCall.args[0]).to.deep.equal({ + contactId: 'chw-contact', + facilityId: 'chw-facility', + }); + const result = res.json.args[0][0]; + chai.expect(result.length).to.equal(1); + chai.expect(result[0].id).to.equal('org.couchdb.user:chw'); + chai.expect(result[0].type).to.be.undefined; + chai.expect(result[0].roles).to.deep.equal(['chw', 'district-admin']); + chai.expect(result[0].contact._id).to.equal('chw-contact'); + chai.expect(result[0].contact.parent._id).to.equal('chw-facility'); + }); + + it('gets the list of users and ignores unexpected filters', async () => { + sinon.stub(auth, 'check').resolves(); + req.query = { roles: ['chw'], name: 'admin' }; + + await controller.v2.get(req, res); + chai.expect(users.getList.firstCall.args[0]).to.deep.equal({}); + const result = res.json.args[0][0]; + chai.expect(result.length).to.equal(3); + chai.expect(result[0].id).to.equal('org.couchdb.user:admin'); + chai.expect(result[1].id).to.equal('org.couchdb.user:chw'); + chai.expect(result[2].id).to.equal('org.couchdb.user:unknown'); + }); }); }); From e93c0015471ce9524ffa1717e0d0957d203080e9 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 18 Mar 2024 14:07:39 +0100 Subject: [PATCH 10/65] remove .only --- api/tests/mocha/controllers/users.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/mocha/controllers/users.spec.js b/api/tests/mocha/controllers/users.spec.js index 9286a28e87f..909ec5b3824 100644 --- a/api/tests/mocha/controllers/users.spec.js +++ b/api/tests/mocha/controllers/users.spec.js @@ -30,7 +30,7 @@ describe('Users Controller', () => { }); afterEach(() => sinon.restore()); - describe.only('get users list', () => { + describe('get users list', () => { let userList; beforeEach(() => { From 0beaf25160fc01b6e618242987df4a37e8859e9d Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 20 Mar 2024 08:54:47 +0100 Subject: [PATCH 11/65] bulk create users instead of creating them one by one --- tests/integration/api/controllers/users.spec.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index a131da5f9fe..da691697d19 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1601,9 +1601,7 @@ describe('Users API', () => { })); const createUserOpts = { path: '/api/v2/users', method: 'POST' }; - for (const user of users) { - await utils.request({ ...createUserOpts, body: user }); - } + await utils.request({ ...createUserOpts, body: users }); const savedUsers = await utils.request({ path: '/api/v2/users' }); for (const user of users) { From 039f0cbfbfe0e6bbdd29a6e9abfd7fc828a24e77 Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 20 Mar 2024 08:55:15 +0100 Subject: [PATCH 12/65] oops how could I miss that --- shared-libs/user-management/src/libs/facility.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/libs/facility.js b/shared-libs/user-management/src/libs/facility.js index cac75ce6ae5..c5a7b8c2af6 100644 --- a/shared-libs/user-management/src/libs/facility.js +++ b/shared-libs/user-management/src/libs/facility.js @@ -3,7 +3,7 @@ const db = require('./db'); const list = async (users, settings) => { const ids = new Set(); for (const user of users) { - ids.add(user?.doc?.facility_id); + ids.add(user?.facility_id); } for (const setting of settings) { ids.add(setting?.contact_id); From f2cfda31e81e5639ac8048c3aecedc34457dc6bc Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 20 Mar 2024 09:18:46 +0100 Subject: [PATCH 13/65] fix facility.spec.js --- shared-libs/user-management/test/unit/libs/facility.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared-libs/user-management/test/unit/libs/facility.spec.js b/shared-libs/user-management/test/unit/libs/facility.spec.js index 68abde21ab2..a0b7a1c36c4 100644 --- a/shared-libs/user-management/test/unit/libs/facility.spec.js +++ b/shared-libs/user-management/test/unit/libs/facility.spec.js @@ -6,8 +6,8 @@ const db = require('../../../src/libs/db'); describe('facility', () => { - const userA = { doc: { facility_id: 'a' } }; - const userB = { doc: { facility_id: 'b' } }; + const userA = { facility_id: 'a' }; + const userB = { facility_id: 'b' }; const settingA = { contact_id: 'a' }; const settingB = { contact_id: 'e' }; From 48030f1f6fc49716fe6607701f79bbd17475cc0b Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 20 Mar 2024 10:04:00 +0100 Subject: [PATCH 14/65] add integration tests for `GET /api/v2/users` with filters --- .../integration/api/controllers/users.spec.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index da691697d19..1c72b5056ed 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1603,6 +1603,7 @@ describe('Users API', () => { const createUserOpts = { path: '/api/v2/users', method: 'POST' }; await utils.request({ ...createUserOpts, body: users }); + // GET without filters const savedUsers = await utils.request({ path: '/api/v2/users' }); for (const user of users) { const savedUser = savedUsers.find(savedUser => savedUser.username === user.username); @@ -1614,6 +1615,35 @@ describe('Users API', () => { 'contact.name': user.contact.name, }); } + + await Promise.all(savedUsers.map(async (savedUser) => { + // GET with facility_id filter + let filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { facility_id: savedUser.place._id }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + + // GET with contact_id filter + filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { contact_id: savedUser.contact._id }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + + // GET with facility_id AND contact_id filters + filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { + facility_id: savedUser.place._id, + contact_id: savedUser.contact._id, + }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + })); }); }); }); From 3fba6d88acbf72cfdab869548a67b80b7b08510e Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 20 Mar 2024 11:53:57 +0100 Subject: [PATCH 15/65] log how long it takes in CI --- tests/integration/api/controllers/users.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 1c72b5056ed..40eb9c9ad6d 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1616,7 +1616,8 @@ describe('Users API', () => { }); } - await Promise.all(savedUsers.map(async (savedUser) => { + const before = Date.now(); + for (const savedUser of savedUsers) { // GET with facility_id filter let filteredUsers = await utils.request({ path: `/api/v2/users`, @@ -1643,7 +1644,8 @@ describe('Users API', () => { }); expect(filteredUsers.length).to.equal(1); expect(filteredUsers[0]).to.deep.equal(savedUser); - })); + } + console.log(`took ${(Date.now() - before)}ms`); }); }); }); From f08a34cd840e0af2701058551af6838465b5839d Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 06:24:55 +0100 Subject: [PATCH 16/65] debug savedUser --- tests/integration/api/controllers/users.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 40eb9c9ad6d..595c61c9cda 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1618,6 +1618,7 @@ describe('Users API', () => { const before = Date.now(); for (const savedUser of savedUsers) { + console.log("savedUser", JSON.stringify(savedUser, null, 4)); // GET with facility_id filter let filteredUsers = await utils.request({ path: `/api/v2/users`, From 4b0bf8aeabaa39f0ca70efa6286a50802f3cf71d Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 06:29:12 +0100 Subject: [PATCH 17/65] lint --- tests/integration/api/controllers/users.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 595c61c9cda..f3e7a0bdf0c 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1618,7 +1618,7 @@ describe('Users API', () => { const before = Date.now(); for (const savedUser of savedUsers) { - console.log("savedUser", JSON.stringify(savedUser, null, 4)); + console.log('savedUser', JSON.stringify(savedUser, null, 4)); // GET with facility_id filter let filteredUsers = await utils.request({ path: `/api/v2/users`, From 3d5455cac89dfbdef37d5d63d0f05e14135eee8e Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 07:07:24 +0100 Subject: [PATCH 18/65] okay other tests in the suite are leaving users with no place or contact, we need to handle them gracefully --- .../integration/api/controllers/users.spec.js | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index f3e7a0bdf0c..a3868416bec 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1618,33 +1618,40 @@ describe('Users API', () => { const before = Date.now(); for (const savedUser of savedUsers) { - console.log('savedUser', JSON.stringify(savedUser, null, 4)); + let filteredUsers; + // GET with facility_id filter - let filteredUsers = await utils.request({ - path: `/api/v2/users`, - qs: { facility_id: savedUser.place._id }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); + if (savedUser.place?._id) { + filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { facility_id: savedUser.place._id }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + } // GET with contact_id filter - filteredUsers = await utils.request({ - path: `/api/v2/users`, - qs: { contact_id: savedUser.contact._id }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); + if (savedUser.contact?._id) { + filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { contact_id: savedUser.contact._id }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + } // GET with facility_id AND contact_id filters - filteredUsers = await utils.request({ - path: `/api/v2/users`, - qs: { - facility_id: savedUser.place._id, - contact_id: savedUser.contact._id, - }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); + if (savedUser.place?._id && savedUser.contact?._id) { + filteredUsers = await utils.request({ + path: `/api/v2/users`, + qs: { + facility_id: savedUser.place._id, + contact_id: savedUser.contact._id, + }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers[0]).to.deep.equal(savedUser); + } } console.log(`took ${(Date.now() - before)}ms`); }); From 4ddab320a02e362b723f819cbededfbf019cd512 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 07:09:43 +0100 Subject: [PATCH 19/65] try speeding it up by running filtered queries in parallel --- tests/integration/api/controllers/users.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index a3868416bec..bb856662b66 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1617,7 +1617,7 @@ describe('Users API', () => { } const before = Date.now(); - for (const savedUser of savedUsers) { + await Promise.all(savedUsers.map(async (savedUser) => { let filteredUsers; // GET with facility_id filter @@ -1652,7 +1652,7 @@ describe('Users API', () => { expect(filteredUsers.length).to.equal(1); expect(filteredUsers[0]).to.deep.equal(savedUser); } - } + })); console.log(`took ${(Date.now() - before)}ms`); }); }); From b0ddc300ab4c7f7ab5dd372082de4dce7c6d2b12 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 07:58:47 +0100 Subject: [PATCH 20/65] remove logs --- tests/integration/api/controllers/users.spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index bb856662b66..242668d9573 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1616,7 +1616,6 @@ describe('Users API', () => { }); } - const before = Date.now(); await Promise.all(savedUsers.map(async (savedUser) => { let filteredUsers; @@ -1653,7 +1652,6 @@ describe('Users API', () => { expect(filteredUsers[0]).to.deep.equal(savedUser); } })); - console.log(`took ${(Date.now() - before)}ms`); }); }); }); From c319cadd2a53a62e12e0744fb6fb023e66d0fe2e Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 21 Mar 2024 08:23:20 +0100 Subject: [PATCH 21/65] comment the lodash magic --- api/src/controllers/users.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index 98a4dd59b83..e28cb7571d3 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -235,8 +235,8 @@ module.exports = { try { await auth.check(req, 'can_view_users'); const filters = _.chain(req.query) - .pick(['facility_id', 'contact_id']) - .mapKeys((value, key) => _.camelCase(key)) + .pick(['facility_id', 'contact_id']) // keep only supported filtering properties + .mapKeys((value, key) => _.camelCase(key)) // rename them to keep consistent variable names .value(); const body = await getUserList(filters); res.json(body); From cd0dff7179cdb746d2003c9ee1c5292984b1aef8 Mon Sep 17 00:00:00 2001 From: Mokhtar Date: Mon, 25 Mar 2024 13:37:43 +0100 Subject: [PATCH 22/65] Update shared-libs/user-management/src/users.js Co-authored-by: Joshua Kuestersteffen --- shared-libs/user-management/src/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 6b3161e11b1..88665c1ece6 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -131,7 +131,7 @@ const getUsersByIds = async (ids) => { const docs = ids.map(id => ({ id })); const { results } = await db.users.bulkGet({ docs }); return results - .map(result => result.docs && result.docs[0] && result.docs[0].ok) + .map(result => result?.docs?.[0]?.ok) .filter(doc => doc); }; From a9967a1661745fcf993054d8b88419dc87268cb1 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 25 Mar 2024 13:39:23 +0100 Subject: [PATCH 23/65] switch to `setting.facility_id` --- shared-libs/user-management/src/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 88665c1ece6..021c5fe695d 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -361,7 +361,7 @@ const mapUsers = (users, settings, facilities) => { fullname: setting.fullname, email: setting.email, phone: setting.phone, - place: getDoc(user.facility_id, facilities), + place: getDoc(setting.facility_id, facilities), roles: user.roles, contact: getDoc(setting.contact_id, facilities), external_id: setting.external_id, From 0e4b82a3f192828047e9a56aa4c86aec36dd7520 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 25 Mar 2024 13:58:36 +0100 Subject: [PATCH 24/65] add other unsupported query parameters to unit test --- api/tests/mocha/controllers/users.spec.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/api/tests/mocha/controllers/users.spec.js b/api/tests/mocha/controllers/users.spec.js index 909ec5b3824..de3c1ed348c 100644 --- a/api/tests/mocha/controllers/users.spec.js +++ b/api/tests/mocha/controllers/users.spec.js @@ -103,7 +103,12 @@ describe('Users Controller', () => { it('gets the list of users with facility_id filter', async () => { sinon.stub(auth, 'check').resolves(); users.getList.resolves([userList[1]]); - req.query = { facility_id: 'chw-facility' }; + req.query = { + facility_id: 'chw-facility', + unsupported: 'nope', + contactId: 'not supported either', + this_wont_work: 123, + }; await controller.v2.get(req, res); chai.expect(users.getList.firstCall.args[0]).to.deep.equal({ facilityId: 'chw-facility' }); From 5a605d1c797681ba2be6f18b8aa92d03e87dd59f Mon Sep 17 00:00:00 2001 From: Mokhtar Date: Mon, 25 Mar 2024 14:00:26 +0100 Subject: [PATCH 25/65] Update shared-libs/user-management/test/unit/users.spec.js Co-authored-by: Joshua Kuestersteffen --- .../user-management/test/unit/users.spec.js | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 5e93a5bf978..8547bce3b08 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -459,25 +459,21 @@ describe('Users service', () => { const allUsers = [ { _id: 'x', - doc: { - name: 'lucas', - facility_id: 'c', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - roles: [ 'national-admin', 'data-entry' ] - } + name: 'lucas', + facility_id: 'c', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', + roles: [ 'national-admin', 'data-entry' ] }, { _id: 'org.couchdb.user:y', - doc: { - name: 'milan', - facility_id: 'b', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - roles: [ 'district-admin' ] - } + name: 'milan', + facility_id: 'b', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + roles: [ 'district-admin' ] } ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); From 7d44261a89ee5a1425229f8f645be5c45af27f9d Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 25 Mar 2024 14:08:10 +0100 Subject: [PATCH 26/65] fix user-management shared lib unit tests --- .../user-management/test/unit/users.spec.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 8547bce3b08..bd235ceafe5 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -204,7 +204,7 @@ describe('Users service', () => { _id: 'org.couchdb.user:x', name: 'lucas', facility_id: 'c', - roles: ['national-admin', 'data-entry'] + roles: ['national-admin', 'data-entry'], }, ]; const allUsersSettings = [ @@ -213,7 +213,8 @@ describe('Users service', () => { name: 'lucas', fullname: 'Lucas M', email: 'l@m.com', - phone: '123456789' + phone: '123456789', + facility_id: 'c', }, ]; const getUsersByIds = sinon.stub().resolves(usersByIds); @@ -336,7 +337,8 @@ describe('Users service', () => { name: 'lucas', fullname: 'Lucas M', email: 'l@m.com', - phone: '123456789' + phone: '123456789', + facility_id: 'c', }, { _id: 'org.couchdb.user:y', @@ -344,7 +346,8 @@ describe('Users service', () => { fullname: 'Milan A', email: 'm@a.com', phone: '987654321', - external_id: 'LTT093' + external_id: 'LTT093', + facility_id: 'b', } ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); @@ -398,14 +401,16 @@ describe('Users service', () => { name: 'lucas', fullname: 'Lucas M', email: 'l@m.com', - phone: '123456789' + phone: '123456789', + facility_id: 'c', }, { _id: 'org.couchdb.user:y', name: 'milan', fullname: 'Milan A', email: 'm@a.com', - phone: '987654321' + phone: '987654321', + facility_id: 'b', } ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); From 61cb17b1ee39842bf309388a54c4dc2ad59d020c Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 25 Mar 2024 15:23:33 +0100 Subject: [PATCH 27/65] stub lower level db calls to cover functions `getAllUserSettings` and `getUsersByIds` behaviors --- .../user-management/test/unit/users.spec.js | 172 ++++++++++-------- 1 file changed, 101 insertions(+), 71 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index bd235ceafe5..44e416cb3fc 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -38,7 +38,7 @@ describe('Users service', () => { db.init({ medic: { get: sinon.stub(), put: sinon.stub(), allDocs: sinon.stub(), query: sinon.stub() }, medicLogs: { get: sinon.stub(), put: sinon.stub(), }, - users: { get: sinon.stub(), put: sinon.stub() }, + users: { bulkGet: sinon.stub(), get: sinon.stub(), put: sinon.stub() }, }); lineage.init(require('@medic/lineage')(Promise, db.medic)); addMessage = sinon.stub(); @@ -199,30 +199,40 @@ describe('Users service', () => { describe('with filters', () => { it('with facility_id', () => { const filters = { facilityId: 'c' }; - const usersByIds = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas', - facility_id: 'c', - roles: ['national-admin', 'data-entry'], - }, - ]; - const allUsersSettings = [ - { - _id: 'org.couchdb.user:x', - name: 'lucas', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - facility_id: 'c', - }, - ]; - const getUsersByIds = sinon.stub().resolves(usersByIds); - service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); - service.__set__('getUsersByIds', getUsersByIds); + const userSettingsResponse = { + rows: [{ + doc: { + _id: 'org.couchdb.user:x', + name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', + facility_id: 'c', + }, + }], + }; + const queryOptions = { + include_docs: true, + key: ['user-settings', 'c'], + }; + db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + + const usersResponse = { + results: [{ + docs: [{ + ok: { + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'], + }, + }], + }], + }; + db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:x' }] }).resolves(usersResponse); + return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); - chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:x'); const lucas = data[0]; chai.expect(lucas.id).to.equal('org.couchdb.user:x'); chai.expect(lucas.username).to.equal('lucas'); @@ -236,32 +246,42 @@ describe('Users service', () => { it('with contact_id', () => { const filters = { contactId: 'c' }; - const usersByIds = [ - { - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - roles: ['district-admin'], - }, - ]; - const allUsersSettings = [ - { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, - ]; - const getUsersByIds = sinon.stub().resolves(usersByIds); - service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); - service.__set__('getUsersByIds', getUsersByIds); + const userSettingsResponse = { + rows: [{ + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, + }], + }; + const queryOptions = { + include_docs: true, + key: ['user-settings', 'c'], + }; + db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + + const usersResponse = { + results: [{ + docs: [{ + ok: { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'], + }, + }], + }], + }; + db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(usersResponse); + return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); - chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:y'); const milan = data[0]; chai.expect(milan.id).to.equal('org.couchdb.user:y'); chai.expect(milan.username).to.equal('milan'); @@ -276,32 +296,42 @@ describe('Users service', () => { it('with both facility_id and contact_id', () => { const filters = { facilityId: 'b', contactId: 'c' }; - const usersByIds = [ - { - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - roles: ['district-admin'], - }, - ]; - const allUsersSettings = [ - { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, - ]; - const getUsersByIds = sinon.stub().resolves(usersByIds); - service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); - service.__set__('getUsersByIds', getUsersByIds); + const userSettingsResponse = { + rows: [{ + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, + }], + }; + const queryOptions = { + include_docs: true, + key: ['user-settings', 'b', 'c'], + }; + db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + + const usersResponse = { + results: [{ + docs: [{ + ok: { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + roles: ['district-admin'], + }, + }], + }], + }; + db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(usersResponse); + return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); - chai.expect(getUsersByIds.firstCall.args[0][0]).to.equal('org.couchdb.user:y'); const milan = data[0]; chai.expect(milan.id).to.equal('org.couchdb.user:y'); chai.expect(milan.username).to.equal('milan'); From 06f15c833e016765f4be1bdb6d687e4c98b7835b Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 27 Mar 2024 14:54:13 +0100 Subject: [PATCH 28/65] integration test for `GET /api/v2/users` with filters --- .../integration/api/controllers/users.spec.js | 147 +++++++++++++++++- 1 file changed, 140 insertions(+), 7 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 242668d9573..547d1de080e 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1585,7 +1585,7 @@ describe('Users API', () => { await utils.revertDb([], true); }); - it('should create and get users', async () => { + it('should create and get all users', async () => { const users = Array.from({ length: 10 }).map(() => ({ username: uuid(), password: password, @@ -1600,10 +1600,8 @@ describe('Users API', () => { roles: ['district_admin', 'mm-online'] })); - const createUserOpts = { path: '/api/v2/users', method: 'POST' }; - await utils.request({ ...createUserOpts, body: users }); + await utils.request({ path: '/api/v2/users', method: 'POST', body: users }); - // GET without filters const savedUsers = await utils.request({ path: '/api/v2/users' }); for (const user of users) { const savedUser = savedUsers.find(savedUser => savedUser.username === user.username); @@ -1622,7 +1620,7 @@ describe('Users API', () => { // GET with facility_id filter if (savedUser.place?._id) { filteredUsers = await utils.request({ - path: `/api/v2/users`, + path: '/api/v2/users', qs: { facility_id: savedUser.place._id }, }); expect(filteredUsers.length).to.equal(1); @@ -1632,7 +1630,7 @@ describe('Users API', () => { // GET with contact_id filter if (savedUser.contact?._id) { filteredUsers = await utils.request({ - path: `/api/v2/users`, + path: '/api/v2/users', qs: { contact_id: savedUser.contact._id }, }); expect(filteredUsers.length).to.equal(1); @@ -1642,7 +1640,7 @@ describe('Users API', () => { // GET with facility_id AND contact_id filters if (savedUser.place?._id && savedUser.contact?._id) { filteredUsers = await utils.request({ - path: `/api/v2/users`, + path: '/api/v2/users', qs: { facility_id: savedUser.place._id, contact_id: savedUser.contact._id, @@ -1653,5 +1651,140 @@ describe('Users API', () => { } })); }); + + it('should create and query users using filters', async () => { + const facilityE = await utils.request({ + path: '/api/v1/places', + method: 'POST', + body: { type: 'health_center', name: 'Facility E', parent: 'PARENT_PLACE' }, + }); + const facilityF = await utils.request({ + path: '/api/v1/places', + method: 'POST', + body: { type: 'health_center', name: 'Facility F', parent: 'PARENT_PLACE' }, + }); + const contactA = await utils.request({ + path: '/api/v1/people', + method: 'POST', + body: { name: 'Contact A', place: facilityE.id }, + }); + const contactB = await utils.request({ + path: '/api/v1/people', + method: 'POST', + body: { name: 'Contact B', place: facilityE.id }, + }); + const contactC = await utils.request({ + path: '/api/v1/people', + method: 'POST', + body: { name: 'Contact C', place: facilityF.id }, + }); + + const userFactory = ({ contact, place }) => ({ + username: uuid(), + password: password, + roles: ['district_admin', 'mm-online'], + contact, + place, + }); + const user1 = userFactory({ contact: contactA.id, place: facilityE.id }); + const user2 = userFactory({ contact: contactA.id, place: facilityE.id }); + const user3 = userFactory({ contact: contactB.id, place: facilityE.id }); + const user4 = userFactory({ contact: contactC.id, place: facilityF.id }); + const [user1Response, user2Response, user3Response, user4Response] = await utils.request({ + path: '/api/v2/users', + method: 'POST', + body: [user1, user2, user3, user4], + }); + + let filteredUsers; + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { + facility_id: facilityE.id, + contact_id: contactA.id, + }, + }); + expect(filteredUsers.length).to.equal(2); + // using find instead of accessing array by index here because + // couch sorts results by their id, not by their creation order + expect(filteredUsers.find(user => user.id === user1Response.user.id)).to.deep.nested.include({ + id: user1Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + expect(filteredUsers.find(user => user.id === user2Response.user.id)).to.deep.nested.include({ + id: user2Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { facility_id: facilityE.id }, + }); + expect(filteredUsers.length).to.equal(3); + expect(filteredUsers.find(user => user.id === user1Response.user.id)).to.deep.nested.include({ + id: user1Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + expect(filteredUsers.find(user => user.id === user2Response.user.id)).to.deep.nested.include({ + id: user2Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + expect(filteredUsers.find(user => user.id === user3Response.user.id)).to.deep.nested.include({ + id: user3Response.user.id, + 'contact._id': contactB.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { contact_id: contactA.id }, + }); + expect(filteredUsers.length).to.equal(2); + expect(filteredUsers.find(user => user.id === user1Response.user.id)).to.deep.nested.include({ + id: user1Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + expect(filteredUsers.find(user => user.id === user2Response.user.id)).to.deep.nested.include({ + id: user2Response.user.id, + 'contact._id': contactA.id, + 'place._id': facilityE.id, + 'place.parent._id': parentPlace._id, + }); + + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { contact_id: contactC.id }, + }); + expect(filteredUsers.length).to.equal(1); + expect(filteredUsers.find(user => user.id === user4Response.user.id)).to.deep.nested.include({ + id: user4Response.user.id, + 'contact._id': contactC.id, + 'place._id': facilityF.id, + 'place.parent._id': parentPlace._id, + }); + + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { contact_id: 'non_existent_contact' }, + }); + expect(filteredUsers.length).to.equal(0); + + filteredUsers = await utils.request({ + path: '/api/v2/users', + qs: { facility_id: 'non_existent_facility' }, + }); + expect(filteredUsers.length).to.equal(0); + }); }); }); From 6ae5c45dcc59c2f369c741b873dd8228250fa6a7 Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 1 Apr 2024 15:49:27 +0200 Subject: [PATCH 29/65] g checkout origin/master `ddocs/medic-db/medic-client/views/doc_by_type/map.js` --- ddocs/medic-db/medic-client/views/doc_by_type/map.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ddocs/medic-db/medic-client/views/doc_by_type/map.js b/ddocs/medic-db/medic-client/views/doc_by_type/map.js index 52572ae6c74..b13e6644401 100644 --- a/ddocs/medic-db/medic-client/views/doc_by_type/map.js +++ b/ddocs/medic-db/medic-client/views/doc_by_type/map.js @@ -6,12 +6,5 @@ function(doc) { }); return; } - if (doc.type === 'user-settings') { - emit([ doc.type ]); - emit([ doc.type, doc.facility_id ]); - emit([ doc.type, doc.contact_id ]); - emit([ doc.type, doc.facility_id, doc.contact_id ]); - return; - } emit([ doc.type ]); } From 4dbe6b1615cce09b22b017865a0d67acb2d4579f Mon Sep 17 00:00:00 2001 From: m5r Date: Mon, 1 Apr 2024 16:39:25 +0200 Subject: [PATCH 30/65] query with `facility_id` & `contact_id` filters on `_users` instead of user-settings docs --- shared-libs/user-management/src/users.js | 42 +++++++++++++----------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 021c5fe695d..6b67df383bf 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -105,36 +105,38 @@ const getDocID = doc => { } }; -const getAllUserSettings = ({ facilityId, contactId } = {}) => { - const key = ['user-settings']; - if (facilityId) { - key.push(facilityId); - } - if (contactId) { - key.push(contactId); - } - +const getAllUserSettings = () => { const opts = { include_docs: true, - key, + key: ['user-settings'], }; return db.medic.query('medic-client/doc_by_type', opts) .then(result => result.rows.map(row => row.doc)); }; -const getAllUsers = () => { - return db.users.allDocs({ include_docs: true }) - .then(result => result.rows.map(({ doc }) => doc)); -}; - -const getUsersByIds = async (ids) => { +const getSettingsByIds = async (ids) => { const docs = ids.map(id => ({ id })); - const { results } = await db.users.bulkGet({ docs }); + const { results } = await db.medic.bulkGet({ docs }); return results .map(result => result?.docs?.[0]?.ok) .filter(doc => doc); }; +const getAllUsers = async (filters) => { + if (_.isEmpty(filters)) { + const { rows } = await db.users.allDocs({ include_docs: true }); + return rows.map(({ doc }) => doc); + } + + const { docs } = await db.users.find({ + selector: { + facility_id: filters.facilityId, + contact_id: filters.contactId, + }, + }); + return docs; +}; + const validateContact = (id, placeID) => { return db.medic.get(id) .then(doc => { @@ -793,9 +795,9 @@ module.exports = { if (_.isEmpty(filters)) { [users, settings] = await Promise.all([getAllUsers(), getAllUserSettings()]); } else { - settings = await getAllUserSettings(filters); - const ids = settings.map(({ _id }) => _id); - users = await getUsersByIds(ids); + users = await getAllUsers(filters); + const ids = users.map(({ _id }) => _id); + settings = await getSettingsByIds(ids); } const facilities = await facility.list(users, settings); From ab0935158fae906be7f980459dd08e8cdc81cd39 Mon Sep 17 00:00:00 2001 From: m5r Date: Tue, 2 Apr 2024 17:36:45 +0200 Subject: [PATCH 31/65] `add-contact-id-to-user-docs` migration --- .../migrations/add-contact-id-to-user-docs.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 api/src/migrations/add-contact-id-to-user-docs.js diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js new file mode 100644 index 00000000000..d44e0258d0b --- /dev/null +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -0,0 +1,43 @@ +const db = require('../db'); +const logger = require('../logger'); + +const BATCH_SIZE = 1; + +const runBatch = async (skip = 0) => { + const options = { + include_docs: true, + limit: BATCH_SIZE, + key: ['user-settings'], + skip, + }; + const { rows } = await db.medic.query('medic-client/doc_by_type', options); + if (!rows.length) { + return; + } + + for (const row of rows) { + const { _id, contact_id } = row.doc; + try { + const user = await db.users.get(_id); + user.contact_id = contact_id; + await db.users.put(user); + } catch (error) { + if (error.status !== 404) { + throw error; + } + logger.warn(`User with id "${_id}" does not exist anymore, skipping it.`); + } + } + + if (rows.length < BATCH_SIZE) { + return; + } + + return runBatch(skip + BATCH_SIZE); +}; + +module.exports = { + name: 'add-contact-id-to-user-docs', + created: new Date(2024, 5, 2), + run: runBatch, +}; From 1c0a1cb0e5fe91f94a30417d2dde7d1bf81fc43b Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 3 Apr 2024 13:45:49 +0200 Subject: [PATCH 32/65] okay I think I found the place where all the magic happens --- shared-libs/user-management/src/users.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 6b67df383bf..04d6f9a1360 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -45,6 +45,7 @@ const RESTRICTED_USER_EDITABLE_FIELDS = [ const USER_EDITABLE_FIELDS = RESTRICTED_USER_EDITABLE_FIELDS.concat([ 'place', + 'contact', 'type', 'roles', ]); @@ -413,7 +414,7 @@ const getSettingsUpdates = (username, data) => { }; const getUserUpdates = (username, data) => { - const ignore = ['type', 'place']; + const ignore = ['type', 'place', 'contact']; const user = { name: username, @@ -436,6 +437,9 @@ const getUserUpdates = (username, data) => { if (data.place) { user.facility_id = getDocID(data.place); } + if (data.contact) { + user.contact_id = getDocID(data.contact); + } return user; }; From 8a703cd3ebc79ccd6d08fe91ff50032f99265297 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 08:18:35 +0200 Subject: [PATCH 33/65] fix tests --- .../user-management/src/libs/facility.js | 6 +- shared-libs/user-management/src/users.js | 7 +- .../test/unit/libs/facility.spec.js | 9 +- .../user-management/test/unit/users.spec.js | 129 +++++++++--------- 4 files changed, 76 insertions(+), 75 deletions(-) diff --git a/shared-libs/user-management/src/libs/facility.js b/shared-libs/user-management/src/libs/facility.js index c5a7b8c2af6..29e4ab243c0 100644 --- a/shared-libs/user-management/src/libs/facility.js +++ b/shared-libs/user-management/src/libs/facility.js @@ -1,12 +1,10 @@ const db = require('./db'); -const list = async (users, settings) => { +const list = async (users) => { const ids = new Set(); for (const user of users) { ids.add(user?.facility_id); - } - for (const setting of settings) { - ids.add(setting?.contact_id); + ids.add(user?.contact_id); } ids.delete(undefined); if (!ids.size) { diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 04d6f9a1360..be1ad03348f 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -364,9 +364,9 @@ const mapUsers = (users, settings, facilities) => { fullname: setting.fullname, email: setting.email, phone: setting.phone, - place: getDoc(setting.facility_id, facilities), + place: getDoc(user.facility_id, facilities), roles: user.roles, - contact: getDoc(setting.contact_id, facilities), + contact: getDoc(user.contact_id, facilities), external_id: setting.external_id, known: user.known }; @@ -577,6 +577,7 @@ const validateUserContact = (data, user, userSettings) => { {'field': 'Contact'} )); } + user.contact_id = null; userSettings.contact_id = null; } }; @@ -804,7 +805,7 @@ module.exports = { settings = await getSettingsByIds(ids); } - const facilities = await facility.list(users, settings); + const facilities = await facility.list(users); return mapUsers(users, settings, facilities); }, getUserSettings, diff --git a/shared-libs/user-management/test/unit/libs/facility.spec.js b/shared-libs/user-management/test/unit/libs/facility.spec.js index a0b7a1c36c4..6c9b382748b 100644 --- a/shared-libs/user-management/test/unit/libs/facility.spec.js +++ b/shared-libs/user-management/test/unit/libs/facility.spec.js @@ -6,11 +6,8 @@ const db = require('../../../src/libs/db'); describe('facility', () => { - const userA = { facility_id: 'a' }; - const userB = { facility_id: 'b' }; - - const settingA = { contact_id: 'a' }; - const settingB = { contact_id: 'e' }; + const userA = { facility_id: 'a', contact_id: 'a' }; + const userB = { facility_id: 'b', contact_id: 'e' }; const facilityA = { _id: 'a' }; const facilityB = { _id: 'b' }; @@ -41,7 +38,7 @@ describe('facility', () => { { doc: facilityB }, facilityNotFound, ] }); - const result = await list([ userA, userB ], [ settingA, settingB ]); + const result = await list([userA, userB]); expect(result).to.deep.equal([ facilityA, facilityB ]); expect(allDocs.callCount).to.equal(1); expect(allDocs.args[0][0].keys).to.deep.equal([ 'a', 'b', 'e' ]); diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 44e416cb3fc..071eb4d51c3 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -36,9 +36,15 @@ describe('Users service', () => { getTransitionsLib: sinon.stub(), }); db.init({ - medic: { get: sinon.stub(), put: sinon.stub(), allDocs: sinon.stub(), query: sinon.stub() }, + medic: { + bulkGet: sinon.stub(), + get: sinon.stub(), + put: sinon.stub(), + allDocs: sinon.stub(), + query: sinon.stub(), + }, medicLogs: { get: sinon.stub(), put: sinon.stub(), }, - users: { bulkGet: sinon.stub(), get: sinon.stub(), put: sinon.stub() }, + users: { find: sinon.stub(), get: sinon.stub(), put: sinon.stub() }, }); lineage.init(require('@medic/lineage')(Promise, db.medic)); addMessage = sinon.stub(); @@ -199,37 +205,36 @@ describe('Users service', () => { describe('with filters', () => { it('with facility_id', () => { const filters = { facilityId: 'c' }; - const userSettingsResponse = { - rows: [{ - doc: { - _id: 'org.couchdb.user:x', - name: 'lucas', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - facility_id: 'c', - }, + const usersResponse = { + docs: [{ + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'], }], }; - const queryOptions = { - include_docs: true, - key: ['user-settings', 'c'], - }; - db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + db.users.find.withArgs({ + selector: { + facility_id: filters.facilityId, + contact_id: filters.contactId, + }, + }).resolves(usersResponse); - const usersResponse = { + const userSettingsResponse = { results: [{ docs: [{ ok: { _id: 'org.couchdb.user:x', name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', facility_id: 'c', - roles: ['national-admin', 'data-entry'], }, }], }], }; - db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:x' }] }).resolves(usersResponse); + db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:x' }] }).resolves(userSettingsResponse); return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); @@ -245,40 +250,40 @@ describe('Users service', () => { }); it('with contact_id', () => { - const filters = { contactId: 'c' }; - const userSettingsResponse = { - rows: [{ - doc: { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, + const filters = { contactId: 'milan-contact' }; + const usersResponse = { + docs: [{ + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + contact_id: 'milan-contact', + roles: ['district-admin'], }], }; - const queryOptions = { - include_docs: true, - key: ['user-settings', 'c'], - }; - db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + db.users.find.withArgs({ + selector: { + facility_id: filters.facilityId, + contact_id: filters.contactId, + }, + }).resolves(usersResponse); - const usersResponse = { + const userSettingsResponse = { results: [{ docs: [{ ok: { _id: 'org.couchdb.user:y', name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', facility_id: 'b', - roles: ['district-admin'], + contact_id: 'milan-contact', }, }], }], }; - db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(usersResponse); + db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); @@ -295,40 +300,40 @@ describe('Users service', () => { }); it('with both facility_id and contact_id', () => { - const filters = { facilityId: 'b', contactId: 'c' }; - const userSettingsResponse = { - rows: [{ - doc: { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, + const filters = { facilityId: 'b', contactId: 'milan-contact' }; + const usersResponse = { + docs: [{ + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + contact_id: 'milan-contact', + roles: ['district-admin'], }], }; - const queryOptions = { - include_docs: true, - key: ['user-settings', 'b', 'c'], - }; - db.medic.query.withArgs('medic-client/doc_by_type', queryOptions).resolves(userSettingsResponse); + db.users.find.withArgs({ + selector: { + facility_id: filters.facilityId, + contact_id: filters.contactId, + }, + }).resolves(usersResponse); - const usersResponse = { + const userSettingsResponse = { results: [{ docs: [{ ok: { _id: 'org.couchdb.user:y', name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', facility_id: 'b', - roles: ['district-admin'], + contact_id: 'milan-contact', }, }], }], }; - db.users.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(usersResponse); + db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); return service.getList(filters).then(data => { chai.expect(data.length).to.equal(1); From 16c39e3fa1e4223ab9515b26b4466a26fd71cb93 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 09:03:18 +0200 Subject: [PATCH 34/65] reduce function cognitive coplexity --- .../migrations/add-contact-id-to-user-docs.js | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js index d44e0258d0b..aef1c645417 100644 --- a/api/src/migrations/add-contact-id-to-user-docs.js +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -3,6 +3,19 @@ const logger = require('../logger'); const BATCH_SIZE = 1; +const processDocument = async ({ _id, contact_id }) => { + try { + const user = await db.users.get(_id); + user.contact_id = contact_id; + await db.users.put(user); + } catch (error) { + if (error.status !== 404) { + throw error; + } + logger.warn(`User with id "${_id}" does not exist anymore, skipping it.`); + } +}; + const runBatch = async (skip = 0) => { const options = { include_docs: true, @@ -16,17 +29,7 @@ const runBatch = async (skip = 0) => { } for (const row of rows) { - const { _id, contact_id } = row.doc; - try { - const user = await db.users.get(_id); - user.contact_id = contact_id; - await db.users.put(user); - } catch (error) { - if (error.status !== 404) { - throw error; - } - logger.warn(`User with id "${_id}" does not exist anymore, skipping it.`); - } + await processDocument(row.doc); } if (rows.length < BATCH_SIZE) { From 42fc2f4a3ffc45d155da9a5604b3e62d94bb324d Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 10:18:03 +0200 Subject: [PATCH 35/65] use async await --- .../user-management/test/unit/users.spec.js | 166 ++++++++---------- 1 file changed, 78 insertions(+), 88 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 071eb4d51c3..d54945d9a7a 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -201,9 +201,8 @@ describe('Users service', () => { }); describe('getList', () => { - describe('with filters', () => { - it('with facility_id', () => { + it('with facility_id', async () => { const filters = { facilityId: 'c' }; const usersResponse = { docs: [{ @@ -236,20 +235,19 @@ describe('Users service', () => { }; db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:x' }] }).resolves(userSettingsResponse); - return service.getList(filters).then(data => { - chai.expect(data.length).to.equal(1); - const lucas = data[0]; - chai.expect(lucas.id).to.equal('org.couchdb.user:x'); - chai.expect(lucas.username).to.equal('lucas'); - chai.expect(lucas.fullname).to.equal('Lucas M'); - chai.expect(lucas.email).to.equal('l@m.com'); - chai.expect(lucas.phone).to.equal('123456789'); - chai.expect(lucas.place).to.deep.equal(facilityc); - chai.expect(lucas.roles).to.deep.equal(['national-admin', 'data-entry']); - }); + const data = await service.getList(filters); + chai.expect(data.length).to.equal(1); + const lucas = data[0]; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal('Lucas M'); + chai.expect(lucas.email).to.equal('l@m.com'); + chai.expect(lucas.phone).to.equal('123456789'); + chai.expect(lucas.place).to.deep.equal(facilityc); + chai.expect(lucas.roles).to.deep.equal(['national-admin', 'data-entry']); }); - it('with contact_id', () => { + it('with contact_id', async () => { const filters = { contactId: 'milan-contact' }; const usersResponse = { docs: [{ @@ -285,21 +283,20 @@ describe('Users service', () => { }; db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); - return service.getList(filters).then(data => { - chai.expect(data.length).to.equal(1); - const milan = data[0]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.contact._id).to.equal('milan-contact'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal(['district-admin']); - }); + const data = await service.getList(filters); + chai.expect(data.length).to.equal(1); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.contact._id).to.equal('milan-contact'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal(['district-admin']); }); - it('with both facility_id and contact_id', () => { + it('with both facility_id and contact_id', async () => { const filters = { facilityId: 'b', contactId: 'milan-contact' }; const usersResponse = { docs: [{ @@ -335,23 +332,22 @@ describe('Users service', () => { }; db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); - return service.getList(filters).then(data => { - chai.expect(data.length).to.equal(1); - const milan = data[0]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.contact._id).to.equal('milan-contact'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal(['district-admin']); - }); + const data = await service.getList(filters); + chai.expect(data.length).to.equal(1); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.contact._id).to.equal('milan-contact'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal(['district-admin']); }); }); describe('without filters', () => { - it('collects user infos', () => { + it('collects user infos', async () => { const allUsers = [ { _id: 'org.couchdb.user:x', @@ -387,29 +383,27 @@ describe('Users service', () => { ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); service.__set__('getAllUserSettings', sinon.stub().resolves(allUsersSettings)); - return service.getList().then(data => { - chai.expect(data.length).to.equal(2); - const lucas = data[0]; - chai.expect(lucas.id).to.equal('org.couchdb.user:x'); - chai.expect(lucas.username).to.equal('lucas'); - chai.expect(lucas.fullname).to.equal('Lucas M'); - chai.expect(lucas.email).to.equal('l@m.com'); - chai.expect(lucas.phone).to.equal('123456789'); - chai.expect(lucas.place).to.deep.equal(facilityc); - chai.expect(lucas.roles).to.deep.equal([ 'national-admin', 'data-entry' ]); - const milan = data[1]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); - chai.expect(milan.external_id).to.equal('LTT093'); - }); - }); - - it('filters out non-users', () => { + const data = await service.getList(); + chai.expect(data.length).to.equal(2); + const [lucas, milan] = data; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal('Lucas M'); + chai.expect(lucas.email).to.equal('l@m.com'); + chai.expect(lucas.phone).to.equal('123456789'); + chai.expect(lucas.place).to.deep.equal(facilityc); + chai.expect(lucas.roles).to.deep.equal([ 'national-admin', 'data-entry' ]); + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); + chai.expect(milan.external_id).to.equal('LTT093'); + }); + + it('filters out non-users', async () => { const allUsers = [ { _id: 'x', @@ -451,21 +445,19 @@ describe('Users service', () => { service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); service.__set__('getAllUserSettings', sinon.stub().resolves(allUserSettings)); - return service.getList().then(data => { - chai.expect(data.length).to.equal(1); - const milan = data[0]; - chai.expect(milan.id).to.equal('org.couchdb.user:y'); - chai.expect(milan.username).to.equal('milan'); - chai.expect(milan.fullname).to.equal('Milan A'); - chai.expect(milan.email).to.equal('m@a.com'); - chai.expect(milan.phone).to.equal('987654321'); - chai.expect(milan.place).to.deep.equal(facilityb); - chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); - }); - + const data = await service.getList(); + chai.expect(data.length).to.equal(1); + const milan = data[0]; + chai.expect(milan.id).to.equal('org.couchdb.user:y'); + chai.expect(milan.username).to.equal('milan'); + chai.expect(milan.fullname).to.equal('Milan A'); + chai.expect(milan.email).to.equal('m@a.com'); + chai.expect(milan.phone).to.equal('987654321'); + chai.expect(milan.place).to.deep.equal(facilityb); + chai.expect(milan.roles).to.deep.equal([ 'district-admin' ]); }); - it('handles minimal users', () => { + it('handles minimal users', async () => { const allUsers = [ { _id: 'org.couchdb.user:x', @@ -474,17 +466,16 @@ describe('Users service', () => { ]; service.__set__('getAllUsers', sinon.stub().resolves(allUsers)); service.__set__('getAllUserSettings', sinon.stub().resolves([])); - return service.getList().then(data => { - chai.expect(data.length).to.equal(1); - const lucas = data[0]; - chai.expect(lucas.id).to.equal('org.couchdb.user:x'); - chai.expect(lucas.username).to.equal('lucas'); - chai.expect(lucas.fullname).to.equal(undefined); - chai.expect(lucas.email).to.equal(undefined); - chai.expect(lucas.phone).to.equal(undefined); - chai.expect(lucas.facility).to.equal(undefined); - chai.expect(lucas.roles).to.equal(undefined); - }); + const data = await service.getList(); + chai.expect(data.length).to.equal(1); + const lucas = data[0]; + chai.expect(lucas.id).to.equal('org.couchdb.user:x'); + chai.expect(lucas.username).to.equal('lucas'); + chai.expect(lucas.fullname).to.equal(undefined); + chai.expect(lucas.email).to.equal(undefined); + chai.expect(lucas.phone).to.equal(undefined); + chai.expect(lucas.facility).to.equal(undefined); + chai.expect(lucas.roles).to.equal(undefined); }); it('returns errors from users service', () => { @@ -524,7 +515,6 @@ describe('Users service', () => { }); }); }); - }); describe('getUserSettings', () => { From bab9d5686dfc8835f8075b77451f1f2a38c2ac4b Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 12:18:23 +0200 Subject: [PATCH 36/65] test that a deleted user doesn't get returned by the route --- tests/integration/api/controllers/users.spec.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 547d1de080e..7f38115e358 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1652,7 +1652,7 @@ describe('Users API', () => { })); }); - it('should create and query users using filters', async () => { + it.only('should create and query users using filters', async () => { const facilityE = await utils.request({ path: '/api/v1/places', method: 'POST', @@ -1690,10 +1690,17 @@ describe('Users API', () => { const user2 = userFactory({ contact: contactA.id, place: facilityE.id }); const user3 = userFactory({ contact: contactB.id, place: facilityE.id }); const user4 = userFactory({ contact: contactC.id, place: facilityF.id }); - const [user1Response, user2Response, user3Response, user4Response] = await utils.request({ + const user5 = userFactory({ contact: contactC.id, place: facilityF.id }); + const [user1Response, user2Response, user3Response, user4Response, user5Response] = await utils.request({ path: '/api/v2/users', method: 'POST', - body: [user1, user2, user3, user4], + body: [user1, user2, user3, user4, user5], + }); + + const user5Name = user5Response.user.id.replace('org.couchdb.user:', ''); + await utils.request({ + path: `/api/v1/users/${user5Name}`, + method: 'DELETE', }); let filteredUsers; @@ -1785,6 +1792,10 @@ describe('Users API', () => { qs: { facility_id: 'non_existent_facility' }, }); expect(filteredUsers.length).to.equal(0); + + const allUsers = await utils.request({ path: '/api/v2/users' }); + expect(allUsers.length).to.equal(4); + expect(allUsers.map(user => user.id)).to.not.include(user5Response.user.id); }); }); }); From fa55975c2b3efc20ccca5f176dc80d4d43413158 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 12:19:35 +0200 Subject: [PATCH 37/65] remove .only --- tests/integration/api/controllers/users.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 7f38115e358..eccc8ddd7ad 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1652,7 +1652,7 @@ describe('Users API', () => { })); }); - it.only('should create and query users using filters', async () => { + it('should create and query users using filters', async () => { const facilityE = await utils.request({ path: '/api/v1/places', method: 'POST', From 302730fbc86fdfa33f4f049fa6316e5ed706cbe5 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 16:08:40 +0200 Subject: [PATCH 38/65] test that both user and user-settings documents get updated with `facility_id` and `contact_id` --- .../integration/api/controllers/users.spec.js | 68 +++++++++++++------ 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index eccc8ddd7ad..065e862f264 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -66,6 +66,7 @@ describe('Users API', () => { name: username, password: password, facility_id: null, + contact_id: null, roles: [ 'chw', 'data_entry', @@ -73,6 +74,7 @@ describe('Users API', () => { }; const newPlaceId = 'NewPlaceId' + new Date().getTime(); + const newContactId = 'NewContactId' + new Date().getTime(); let cookie; @@ -92,7 +94,14 @@ describe('Users API', () => { { _id: newPlaceId, type: 'clinic' - } + }, + { + _id: newContactId, + type: 'person', + parent: { + _id: newPlaceId, + }, + }, ]; before(async () => { @@ -165,19 +174,28 @@ describe('Users API', () => { await utils.revertDb([], true); }); - it('Allows for admin users to modify someone', () => { - return utils - .request({ - path: `/api/v1/users/${username}`, - method: 'POST', - body: { - place: newPlaceId - } - }) - .then(() => utils.getDoc(getUserId(username))) - .then(doc => { - chai.expect(doc.facility_id).to.equal(newPlaceId); - }); + it('Allows for admin users to modify someone', async () => { + let userSettingsDoc = await utils.getDoc(getUserId(username)); + chai.expect(userSettingsDoc.facility_id).to.equal(null); + chai.expect(userSettingsDoc.contact_id).to.equal(null); + let userDoc = await utils.usersDb.get(getUserId(username)); + chai.expect(userDoc.facility_id).to.equal(null); + chai.expect(userDoc.contact_id).to.equal(null); + + await utils.request({ + path: `/api/v1/users/${username}`, + method: 'POST', + body: { + place: newPlaceId, + contact: newContactId, + }, + }); + userSettingsDoc = await utils.getDoc(getUserId(username)); + chai.expect(userSettingsDoc.facility_id).to.equal(newPlaceId); + chai.expect(userSettingsDoc.contact_id).to.equal(newContactId); + userDoc = await utils.usersDb.get(getUserId(username)); + chai.expect(userDoc.facility_id).to.equal(newPlaceId); + chai.expect(userDoc.contact_id).to.equal(newContactId); }); it('401s if a user without the right permissions attempts to modify someone else', () => { @@ -623,6 +641,7 @@ describe('Users API', () => { type: 'user', roles: ['district_admin'], facility_id: 'fixture:test', + contact_id: 'fixture:user:testuser', }; chai.expect(user).to.shallowDeepEqual(Object.assign(defaultProps, extra)); }; @@ -1015,9 +1034,14 @@ describe('Users API', () => { for (const user of users) { let [userInDb, userSettings] = await Promise.all([getUser(user), getUserSettings(user)]); - const extraProps = { facility_id: user.place._id, name: user.username, roles: user.roles }; + const extraProps = { + facility_id: user.place._id, + contact_id: user.contact._id, + name: user.username, + roles: user.roles, + }; expectCorrectUser(userInDb, extraProps); - expectCorrectUserSettings(userSettings, { ...extraProps, contact_id: user.contact._id }); + expectCorrectUserSettings(userSettings, extraProps); chai.expect(userInDb.token_login).to.be.undefined; chai.expect(userSettings.token_login).to.be.undefined; await expectPasswordLoginToWork(user); @@ -1040,7 +1064,6 @@ describe('Users API', () => { expectCorrectUser(userInDb, { ...extraProps, roles: ['new_role'] }); expectCorrectUserSettings(userSettings, { ...extraProps, - contact_id: user.contact._id, roles: ['new_role'], phone: '+40744898989', }); @@ -1125,9 +1148,14 @@ describe('Users API', () => { for (const user of users) { let [userInDb, userSettings] = await Promise.all([getUser(user), getUserSettings(user)]); - const extraProps = { facility_id: user.place._id, name: user.username, roles: user.roles }; + const extraProps = { + facility_id: user.place._id, + contact_id: user.contact._id, + name: user.username, + roles: user.roles, + }; expectCorrectUser(userInDb, extraProps); - expectCorrectUserSettings(userSettings, { ...extraProps, contact_id: user.contact._id }); + expectCorrectUserSettings(userSettings, extraProps); chai.expect(userInDb.token_login).to.be.ok; chai.expect(userInDb.token_login).to.have.keys(['active', 'token', 'expiration_date' ]); chai.expect(userInDb.token_login).to.include({ active: true }); @@ -1173,7 +1201,6 @@ describe('Users API', () => { expectCorrectUser(userInDb, { ...extraProps, roles: ['new_role'] }); expectCorrectUserSettings(userSettings, { ...extraProps, - contact_id: user.contact._id, roles: ['new_role'], phone: '+40744898989', }); @@ -1794,7 +1821,6 @@ describe('Users API', () => { expect(filteredUsers.length).to.equal(0); const allUsers = await utils.request({ path: '/api/v2/users' }); - expect(allUsers.length).to.equal(4); expect(allUsers.map(user => user.id)).to.not.include(user5Response.user.id); }); }); From 0165b84460d19b4910b96fb6875e701d000ce82c Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 16:22:29 +0200 Subject: [PATCH 39/65] move `auth.check()` up the chain --- api/src/controllers/users.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index e28cb7571d3..ef986b5ecd7 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -110,7 +110,12 @@ const getAllowedDocsCounts = async (userCtx) => { // In express4, req.host strips off the port number: https://expressjs.com/en/guide/migrating-5.html#req.host const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; -const getUserList = async (filters = {}) => { +const getUserList = async (req) => { + await auth.check(req, 'can_view_users'); + const filters = _.chain(req.query) + .pick(['facility_id', 'contact_id']) // keep only supported filtering properties + .mapKeys((value, key) => _.camelCase(key)) // rename them to keep consistent variable names + .value(); return await users.getList(filters); }; @@ -131,8 +136,7 @@ const convertUserListToV1 = (users=[]) => { module.exports = { get: (req, res) => { - return auth.check(req, 'can_view_users') - .then(() => getUserList()) + return getUserList(req) .then(list => convertUserListToV1(list)) .then(body => res.json(body)) .catch(err => serverUtils.error(err, req, res)); @@ -233,12 +237,7 @@ module.exports = { v2: { get: async (req, res) => { try { - await auth.check(req, 'can_view_users'); - const filters = _.chain(req.query) - .pick(['facility_id', 'contact_id']) // keep only supported filtering properties - .mapKeys((value, key) => _.camelCase(key)) // rename them to keep consistent variable names - .value(); - const body = await getUserList(filters); + const body = await getUserList(req); res.json(body); } catch (err) { serverUtils.error(err, req, res); From 08d24bcac242dba027da05d08958069d9c0a69f4 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 16:53:28 +0200 Subject: [PATCH 40/65] use `contact_id` from `user` doc --- shared-libs/user-management/src/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index be1ad03348f..59d33db6f8d 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -566,7 +566,7 @@ const validateUserFacility = (data, user, userSettings) => { const validateUserContact = (data, user, userSettings) => { if (data.contact) { - return validateContact(userSettings.contact_id, user.facility_id); + return validateContact(user.contact_id, user.facility_id); } if (_.isNull(data.contact)) { From 4667becfadf839ca73135db7c1813dcfbcd92a59 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 16:53:36 +0200 Subject: [PATCH 41/65] increase batch size to 100 --- api/src/migrations/add-contact-id-to-user-docs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js index aef1c645417..d5839cd6e32 100644 --- a/api/src/migrations/add-contact-id-to-user-docs.js +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -1,7 +1,7 @@ const db = require('../db'); const logger = require('../logger'); -const BATCH_SIZE = 1; +const BATCH_SIZE = 100; const processDocument = async ({ _id, contact_id }) => { try { From 3ce0587d9c5696332cd3652253214e4857674fe1 Mon Sep 17 00:00:00 2001 From: m5r Date: Thu, 4 Apr 2024 17:02:40 +0200 Subject: [PATCH 42/65] remove duplicate test --- .../integration/api/controllers/users.spec.js | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 065e862f264..366f7c0b7b4 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1640,43 +1640,6 @@ describe('Users API', () => { 'contact.name': user.contact.name, }); } - - await Promise.all(savedUsers.map(async (savedUser) => { - let filteredUsers; - - // GET with facility_id filter - if (savedUser.place?._id) { - filteredUsers = await utils.request({ - path: '/api/v2/users', - qs: { facility_id: savedUser.place._id }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); - } - - // GET with contact_id filter - if (savedUser.contact?._id) { - filteredUsers = await utils.request({ - path: '/api/v2/users', - qs: { contact_id: savedUser.contact._id }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); - } - - // GET with facility_id AND contact_id filters - if (savedUser.place?._id && savedUser.contact?._id) { - filteredUsers = await utils.request({ - path: '/api/v2/users', - qs: { - facility_id: savedUser.place._id, - contact_id: savedUser.contact._id, - }, - }); - expect(filteredUsers.length).to.equal(1); - expect(filteredUsers[0]).to.deep.equal(savedUser); - } - })); }); it('should create and query users using filters', async () => { From 970395726bf0a94cb6f11aab910b4e9cb3afce44 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Tue, 9 Apr 2024 14:17:30 -0500 Subject: [PATCH 43/65] Add integration tests for migration --- .../migrations/add-contact-id-to-user.js | 111 ++++++++++++++++++ api/tests/integration/migrations/utils.js | 5 + 2 files changed, 116 insertions(+) create mode 100644 api/tests/integration/migrations/add-contact-id-to-user.js diff --git a/api/tests/integration/migrations/add-contact-id-to-user.js b/api/tests/integration/migrations/add-contact-id-to-user.js new file mode 100644 index 00000000000..56e60ad88ec --- /dev/null +++ b/api/tests/integration/migrations/add-contact-id-to-user.js @@ -0,0 +1,111 @@ +const { expect } = require('chai'); +const utils = require('./utils'); +const db = require('../../../src/db'); +const logger = require('../../../src/logger'); +const sinon = require('sinon'); + +const createUserSettingsDoc = (id, contactId) => { + const userSettings = { + _id: id, + name: id.split(':')[1], + type: 'user-settings', + roles: ['chw'], + facility_id: 'abc' + }; + if (contactId) { + userSettings.contact_id = contactId; + } + return userSettings; +}; + +const createUserDoc = userSettings => { + return { + ...userSettings, + type: 'user', + contact_id: null, + }; +}; + +const writeUserDocs = userDocs => db.users.bulkDocs(userDocs); + +const getUserDoc = async (id) => db.users.get(id); + +describe('add-contact-id-to-user migration', function() { + afterEach(() => { + sinon.restore(); + utils.tearDown(); + }); + + it('migrates the contact_id value from user-settings to _users for all users', async () => { + const userDocTuples = Array + .from({ length: 299 }, (_, i) => i) + .map(i => { + const userSettingsDoc = createUserSettingsDoc(`org.couchdb.user:test-chw-${i}`, `contact-${i}`); + return [ + userSettingsDoc, + createUserDoc(userSettingsDoc) + ]; + }); + const userSettingsDocs = userDocTuples.map(([ userSettingsDoc ]) => userSettingsDoc); + await utils.initDb(userSettingsDocs); + await writeUserDocs(userDocTuples.map(([, userDoc]) => userDoc)); + + await utils.runMigration('add-contact-id-to-user-docs'); + + await utils.assertDb(userSettingsDocs); + for (const [userSettingsDoc, userDoc] of userDocTuples) { + const updatedUserDoc = await getUserDoc(userDoc._id); + expect(updatedUserDoc).to.deep.include({ ...userDoc, contact_id: userSettingsDoc.contact_id }); + } + }); + + it('skips users that do not exist in _users', async () => { + const userSettingsDoc = createUserSettingsDoc('org.couchdb.user:missing-chw', 'contact'); + await utils.initDb([ userSettingsDoc ]); + sinon.spy(logger, 'warn'); + + await utils.runMigration('add-contact-id-to-user-docs'); + + expect(logger.warn.calledOnce).to.equal(true); + expect(logger.warn.firstCall.args[0]).to + .equal(`User with id "${userSettingsDoc._id}" does not exist anymore, skipping it.`); + await utils.assertDb([ userSettingsDoc ]); + try { + await getUserDoc(userSettingsDoc._id); + expect.fail('Expected to throw'); + } catch (e) { + expect(e.status).to.equal(404); + } + }); + + it('overwrites any existing contact_id value in _users', async () => { + const userSettingsDocs = [ + createUserSettingsDoc('org.couchdb.user:different-contact', 'contact'), + createUserSettingsDoc('org.couchdb.user:no-contact') + ]; + await utils.initDb(userSettingsDocs); + const userDocs = [ + { + ...createUserDoc(userSettingsDocs[0]), + contact_id: 'old-contact' + }, + { + ...createUserDoc(userSettingsDocs[1]), + contact_id: 'old-contact' + }, + ]; + await writeUserDocs(userDocs); + + await utils.runMigration('add-contact-id-to-user-docs'); + + await utils.assertDb(userSettingsDocs); + + const updatedUserDoc0 = await getUserDoc(userDocs[0]._id); + expect(updatedUserDoc0).to.deep.include({ ...userDocs[0], contact_id: userSettingsDocs[0].contact_id }); + const updatedUserDoc1 = await getUserDoc(userDocs[1]._id); + const expectedUserDoc1 = { ...userDocs[1] }; + delete expectedUserDoc1.contact_id; + expect(updatedUserDoc1).to.deep.include(expectedUserDoc1); + expect(updatedUserDoc1.contact_id).to.be.undefined; + }); +}); diff --git a/api/tests/integration/migrations/utils.js b/api/tests/integration/migrations/utils.js index 0c188c2af3b..550fa4d2499 100644 --- a/api/tests/integration/migrations/utils.js +++ b/api/tests/integration/migrations/utils.js @@ -154,9 +154,11 @@ const matchDbs = (expected, actual) => { const realMedicDb = db.medic; const realSentinelDb = db.sentinel; +const realUsersDb = db.users; const switchToRealDbs = () => { db.medic = realMedicDb; db.sentinel = realSentinelDb; + db.users = realUsersDb; }; const switchToTestDbs = () => { @@ -166,6 +168,9 @@ const switchToTestDbs = () => { db.sentinel = new PouchDB( realSentinelDb.name.replace(/medic-sentinel$/, 'medic-sentinel-test') ); + db.users = new PouchDB( + realUsersDb.name.replace(/_users$/, 'users-test') + ); }; const initDb = content => { From 6a4cfd901e9f6bae3e09c4abc1e0ea9d4564677e Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Tue, 9 Apr 2024 15:23:54 -0500 Subject: [PATCH 44/65] Update whole batch of users at the same time in migration --- .../migrations/add-contact-id-to-user-docs.js | 53 ++++++++++++------- .../migrations/add-contact-id-to-user.js | 25 ++++----- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js index d5839cd6e32..ece8966bfe8 100644 --- a/api/src/migrations/add-contact-id-to-user-docs.js +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -3,36 +3,49 @@ const logger = require('../logger'); const BATCH_SIZE = 100; -const processDocument = async ({ _id, contact_id }) => { - try { - const user = await db.users.get(_id); - user.contact_id = contact_id; - await db.users.put(user); - } catch (error) { - if (error.status !== 404) { - throw error; - } - logger.warn(`User with id "${_id}" does not exist anymore, skipping it.`); - } -}; - -const runBatch = async (skip = 0) => { +const getUserSettingsDocs = async (skip = 0) => { const options = { include_docs: true, limit: BATCH_SIZE, key: ['user-settings'], skip, }; - const { rows } = await db.medic.query('medic-client/doc_by_type', options); - if (!rows.length) { - return; + return (await db.medic.query('medic-client/doc_by_type', options)) + .rows + .map(row => row.doc); +}; + +const getUpdatedUserDoc = (userSettingsDocs) => (userDoc, index) => { + const { _id, contact_id } = userSettingsDocs[index]; + if (!userDoc) { + logger.warn(`Could not find user with id "${_id}". Skipping it.`); + return null; } + return { ...userDoc, contact_id }; +}; - for (const row of rows) { - await processDocument(row.doc); +const updateUsersDatabase = async (userSettingsDocs) => { + const allDocsOptions = { + include_docs: true, + keys: userSettingsDocs.map(doc => doc._id), + }; + const updatedUsersDocs = (await db.users.allDocs(allDocsOptions)) + .rows + .map(row => row.doc) + .map(getUpdatedUserDoc(userSettingsDocs)) + .filter(Boolean); + await db.users.bulkDocs(updatedUsersDocs); +}; + +const runBatch = async (skip = 0) => { + const userSettingsDocs = await getUserSettingsDocs(skip); + if (!userSettingsDocs.length) { + return; } - if (rows.length < BATCH_SIZE) { + await updateUsersDatabase(userSettingsDocs); + + if (userSettingsDocs.length < BATCH_SIZE) { return; } diff --git a/api/tests/integration/migrations/add-contact-id-to-user.js b/api/tests/integration/migrations/add-contact-id-to-user.js index 56e60ad88ec..c74da40c672 100644 --- a/api/tests/integration/migrations/add-contact-id-to-user.js +++ b/api/tests/integration/migrations/add-contact-id-to-user.js @@ -60,22 +60,24 @@ describe('add-contact-id-to-user migration', function() { }); it('skips users that do not exist in _users', async () => { - const userSettingsDoc = createUserSettingsDoc('org.couchdb.user:missing-chw', 'contact'); - await utils.initDb([ userSettingsDoc ]); + const userSettingsDocMissing = createUserSettingsDoc('org.couchdb.user:missing-chw', 'contact'); + const userSettingsDocDeleted = createUserSettingsDoc('org.couchdb.user:user-deleted', 'contact'); + await utils.initDb([ userSettingsDocMissing, userSettingsDocDeleted ]); + const userDoc = { + ...createUserDoc(userSettingsDocDeleted), + _deleted: true + }; + await writeUserDocs([userDoc]); sinon.spy(logger, 'warn'); await utils.runMigration('add-contact-id-to-user-docs'); - expect(logger.warn.calledOnce).to.equal(true); + expect(logger.warn.calledTwice).to.equal(true); expect(logger.warn.firstCall.args[0]).to - .equal(`User with id "${userSettingsDoc._id}" does not exist anymore, skipping it.`); - await utils.assertDb([ userSettingsDoc ]); - try { - await getUserDoc(userSettingsDoc._id); - expect.fail('Expected to throw'); - } catch (e) { - expect(e.status).to.equal(404); - } + .equal(`Could not find user with id "${userSettingsDocMissing._id}". Skipping it.`); + expect(logger.warn.secondCall.args[0]).to + .equal(`Could not find user with id "${userSettingsDocDeleted._id}". Skipping it.`); + await utils.assertDb([ userSettingsDocMissing, userSettingsDocDeleted ]); }); it('overwrites any existing contact_id value in _users', async () => { @@ -99,7 +101,6 @@ describe('add-contact-id-to-user migration', function() { await utils.runMigration('add-contact-id-to-user-docs'); await utils.assertDb(userSettingsDocs); - const updatedUserDoc0 = await getUserDoc(userDocs[0]._id); expect(updatedUserDoc0).to.deep.include({ ...userDocs[0], contact_id: userSettingsDocs[0].contact_id }); const updatedUserDoc1 = await getUserDoc(userDocs[1]._id); From 13475b67be50b74ff6c2a6c5563e76b4574ef602 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Wed, 10 Apr 2024 15:37:26 -0500 Subject: [PATCH 45/65] Add users_by_facility_and_contact view to _users db --- api/src/services/setup/databases.js | 5 +++++ ddocs/users-db/users/_id | 1 + .../users/views/users_by_facility_and_contact/map.js | 11 +++++++++++ scripts/build/ddoc-compile.js | 1 + 4 files changed, 18 insertions(+) create mode 100644 ddocs/users-db/users/_id create mode 100644 ddocs/users-db/users/views/users_by_facility_and_contact/map.js diff --git a/api/src/services/setup/databases.js b/api/src/services/setup/databases.js index 37160c699a4..57781abb85d 100644 --- a/api/src/services/setup/databases.js +++ b/api/src/services/setup/databases.js @@ -33,6 +33,11 @@ const DATABASES = [ db: db.medicUsersMeta, jsonFileName: 'users-meta.json', }, + { + name: `${environment.db}-users`, + db: db.users, + jsonFileName: 'users.json', + }, ]; module.exports = { diff --git a/ddocs/users-db/users/_id b/ddocs/users-db/users/_id new file mode 100644 index 00000000000..b07941f6f20 --- /dev/null +++ b/ddocs/users-db/users/_id @@ -0,0 +1 @@ +_design/users diff --git a/ddocs/users-db/users/views/users_by_facility_and_contact/map.js b/ddocs/users-db/users/views/users_by_facility_and_contact/map.js new file mode 100644 index 00000000000..dc72fec8f7b --- /dev/null +++ b/ddocs/users-db/users/views/users_by_facility_and_contact/map.js @@ -0,0 +1,11 @@ +function(doc) { + if (doc.facility_id && doc.contact_id) { + emit([ doc.facility_id, doc.contact_id ]); + } + if (doc.facility_id) { + emit(doc.facility_id); + } + if (doc.contact_id) { + emit(doc.contact_id); + } +} diff --git a/scripts/build/ddoc-compile.js b/scripts/build/ddoc-compile.js index a03acccd252..671905b9aa5 100644 --- a/scripts/build/ddoc-compile.js +++ b/scripts/build/ddoc-compile.js @@ -17,6 +17,7 @@ const compilePrimary = async () => { await compile([ 'build/ddocs/sentinel-db/sentinel' ], 'build/ddocs/sentinel.json'); await compile([ 'build/ddocs/users-meta-db/users-meta' ], 'build/ddocs/users-meta.json'); await compile([ 'build/ddocs/logs-db/logs' ], 'build/ddocs/logs.json'); + await compile([ 'build/ddocs/users-db/users' ], 'build/ddocs/users.json'); }; const commands = { From 03ab927247ba7f27c7af46a04eeec58a3bb82788 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Wed, 10 Apr 2024 17:07:48 -0500 Subject: [PATCH 46/65] Split out into two facility/contact views. Call views from user-management --- .../users/views/users_by_contact_id/map.js | 5 ++ .../users_by_facility_and_contact/map.js | 11 ---- .../users/views/users_by_facility_id/map.js | 5 ++ shared-libs/user-management/src/users.js | 59 +++++++++---------- 4 files changed, 39 insertions(+), 41 deletions(-) create mode 100644 ddocs/users-db/users/views/users_by_contact_id/map.js delete mode 100644 ddocs/users-db/users/views/users_by_facility_and_contact/map.js create mode 100644 ddocs/users-db/users/views/users_by_facility_id/map.js diff --git a/ddocs/users-db/users/views/users_by_contact_id/map.js b/ddocs/users-db/users/views/users_by_contact_id/map.js new file mode 100644 index 00000000000..d61504b977c --- /dev/null +++ b/ddocs/users-db/users/views/users_by_contact_id/map.js @@ -0,0 +1,5 @@ +function(doc) { + if (doc.contact_id) { + emit(doc.contact_id); + } +} diff --git a/ddocs/users-db/users/views/users_by_facility_and_contact/map.js b/ddocs/users-db/users/views/users_by_facility_and_contact/map.js deleted file mode 100644 index dc72fec8f7b..00000000000 --- a/ddocs/users-db/users/views/users_by_facility_and_contact/map.js +++ /dev/null @@ -1,11 +0,0 @@ -function(doc) { - if (doc.facility_id && doc.contact_id) { - emit([ doc.facility_id, doc.contact_id ]); - } - if (doc.facility_id) { - emit(doc.facility_id); - } - if (doc.contact_id) { - emit(doc.contact_id); - } -} diff --git a/ddocs/users-db/users/views/users_by_facility_id/map.js b/ddocs/users-db/users/views/users_by_facility_id/map.js new file mode 100644 index 00000000000..f468b6354ed --- /dev/null +++ b/ddocs/users-db/users/views/users_by_facility_id/map.js @@ -0,0 +1,5 @@ +function(doc) { + if (doc.facility_id) { + emit(doc.facility_id); + } +} diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 59d33db6f8d..ad8bf3cf73e 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -106,14 +106,11 @@ const getDocID = doc => { } }; -const getAllUserSettings = () => { - const opts = { - include_docs: true, - key: ['user-settings'], - }; - return db.medic.query('medic-client/doc_by_type', opts) - .then(result => result.rows.map(row => row.doc)); -}; +const queryDocs = (db, view, key) => db + .query(view, { include_docs: true, key }) + .then(result => result.rows.map(row => row.doc)); + +const getAllUserSettings = () => queryDocs(db.medic, 'medic-client/doc_by_type', ['user-settings']); const getSettingsByIds = async (ids) => { const docs = ids.map(id => ({ id })); @@ -123,19 +120,31 @@ const getSettingsByIds = async (ids) => { .filter(doc => doc); }; -const getAllUsers = async (filters) => { - if (_.isEmpty(filters)) { - const { rows } = await db.users.allDocs({ include_docs: true }); - return rows.map(({ doc }) => doc); +const getAllUsers = async () => db.users + .allDocs({ include_docs: true }) + .then(({ rows }) => rows.map(({ doc }) => doc)); + +const getUsers = async ({ facilityId, contactId }) => { + if (!contactId) { + return queryDocs(db.users, 'users/users_by_facility_id', facilityId); } - const { docs } = await db.users.find({ - selector: { - facility_id: filters.facilityId, - contact_id: filters.contactId, - }, - }); - return docs; + const usersForContactId = await queryDocs(db.users, 'users/users_by_contact_id', contactId); + if (facilityId) { + return usersForContactId.filter(user => user.facility_id === facilityId); + } + + return usersForContactId; +}; + +const getUsersAndSettings = async (filters) => { + if (_.isEmpty(filters)) { + return Promise.all([getAllUsers(), getAllUserSettings()]); + } + const users = await getUsers(filters); + const ids = users.map(({ _id }) => _id); + const settings = await getSettingsByIds(ids); + return [users, settings]; }; const validateContact = (id, placeID) => { @@ -794,17 +803,7 @@ const getUserSettings = async({ name }) => { module.exports = { deleteUser: username => deleteUser(createID(username)), getList: async (filters) => { - let users; - let settings; - - if (_.isEmpty(filters)) { - [users, settings] = await Promise.all([getAllUsers(), getAllUserSettings()]); - } else { - users = await getAllUsers(filters); - const ids = users.map(({ _id }) => _id); - settings = await getSettingsByIds(ids); - } - + const [users, settings] = await getUsersAndSettings(filters); const facilities = await facility.list(users); return mapUsers(users, settings, facilities); }, From 5729789f1750289d79ccd7a6b46409d33049d004 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Wed, 10 Apr 2024 18:01:18 -0500 Subject: [PATCH 47/65] Fix unit tests to expect queries --- .../user-management/test/unit/users.spec.js | 80 +++++++++++-------- 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index d54945d9a7a..a560893e873 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -44,7 +44,7 @@ describe('Users service', () => { query: sinon.stub(), }, medicLogs: { get: sinon.stub(), put: sinon.stub(), }, - users: { find: sinon.stub(), get: sinon.stub(), put: sinon.stub() }, + users: { query: sinon.stub(), get: sinon.stub(), put: sinon.stub() }, }); lineage.init(require('@medic/lineage')(Promise, db.medic)); addMessage = sinon.stub(); @@ -205,18 +205,18 @@ describe('Users service', () => { it('with facility_id', async () => { const filters = { facilityId: 'c' }; const usersResponse = { - docs: [{ - _id: 'org.couchdb.user:x', - name: 'lucas', - facility_id: 'c', - roles: ['national-admin', 'data-entry'], + rows: [{ + doc: { + _id: 'org.couchdb.user:x', + name: 'lucas', + facility_id: 'c', + roles: ['national-admin', 'data-entry'], + } }], }; - db.users.find.withArgs({ - selector: { - facility_id: filters.facilityId, - contact_id: filters.contactId, - }, + db.users.query.withArgs('users/users_by_facility_id', { + include_docs: true, + key: filters.facilityId, }).resolves(usersResponse); const userSettingsResponse = { @@ -250,19 +250,19 @@ describe('Users service', () => { it('with contact_id', async () => { const filters = { contactId: 'milan-contact' }; const usersResponse = { - docs: [{ - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - contact_id: 'milan-contact', - roles: ['district-admin'], + rows: [{ + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + contact_id: 'milan-contact', + roles: ['district-admin'], + } }], }; - db.users.find.withArgs({ - selector: { - facility_id: filters.facilityId, - contact_id: filters.contactId, - }, + db.users.query.withArgs('users/users_by_contact_id', { + include_docs: true, + key: filters.contactId, }).resolves(usersResponse); const userSettingsResponse = { @@ -299,19 +299,30 @@ describe('Users service', () => { it('with both facility_id and contact_id', async () => { const filters = { facilityId: 'b', contactId: 'milan-contact' }; const usersResponse = { - docs: [{ - _id: 'org.couchdb.user:y', - name: 'milan', - facility_id: 'b', - contact_id: 'milan-contact', - roles: ['district-admin'], - }], + rows: [ + { + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + facility_id: 'b', + contact_id: 'milan-contact', + roles: ['district-admin'], + } + }, + { + doc: { + _id: 'org.couchdb.user:z', + name: 'bill', + facility_id: 'a', + contact_id: 'milan-contact', + roles: ['district-admin'], + } + } + ], }; - db.users.find.withArgs({ - selector: { - facility_id: filters.facilityId, - contact_id: filters.contactId, - }, + db.users.query.withArgs('users/users_by_contact_id', { + include_docs: true, + key: filters.contactId, }).resolves(usersResponse); const userSettingsResponse = { @@ -333,6 +344,7 @@ describe('Users service', () => { db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); const data = await service.getList(filters); + chai.expect(db.users.query.callCount).to.equal(1); chai.expect(data.length).to.equal(1); const milan = data[0]; chai.expect(milan.id).to.equal('org.couchdb.user:y'); From 11994ae228669a4c03a69ac5dd1a59c3b34f7fa6 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Thu, 11 Apr 2024 12:41:57 -0500 Subject: [PATCH 48/65] Fix api unit tests --- .../mocha/services/setup/databases.spec.js | 7 ++- api/tests/mocha/services/setup/utils.spec.js | 45 +++++++++++++++---- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/api/tests/mocha/services/setup/databases.spec.js b/api/tests/mocha/services/setup/databases.spec.js index 8e90ea708ae..7b89a5fe784 100644 --- a/api/tests/mocha/services/setup/databases.spec.js +++ b/api/tests/mocha/services/setup/databases.spec.js @@ -38,7 +38,12 @@ describe('databases', () => { name: 'thedb-users-meta', db: db.medicUsersMeta, jsonFileName: 'users-meta.json', - } + }, + { + name: `thedb-users`, + db: db.users, + jsonFileName: 'users.json', + }, ]); }); }); diff --git a/api/tests/mocha/services/setup/utils.spec.js b/api/tests/mocha/services/setup/utils.spec.js index c67c69cc147..8b9a4475bce 100644 --- a/api/tests/mocha/services/setup/utils.spec.js +++ b/api/tests/mocha/services/setup/utils.spec.js @@ -49,24 +49,27 @@ describe('Setup utils', () => { ddocsService.getStagedDdocs.withArgs(DATABASES[1]).resolves([{ _id: 'two' }]); ddocsService.getStagedDdocs.withArgs(DATABASES[2]).resolves([{ _id: 'three' }]); ddocsService.getStagedDdocs.withArgs(DATABASES[3]).resolves([{ _id: 'four' }]); + ddocsService.getStagedDdocs.withArgs(DATABASES[4]).resolves([{ _id: 'five' }]); db.saveDocs.resolves(); await utils.__get__('deleteStagedDdocs')(); - expect(ddocsService.getStagedDdocs.callCount).to.equal(4); + expect(ddocsService.getStagedDdocs.callCount).to.equal(5); expect(ddocsService.getStagedDdocs.args).to.deep.equal([ [DATABASES[0]], [DATABASES[1]], [DATABASES[2]], [DATABASES[3]], + [DATABASES[4]], ]); - expect(db.saveDocs.callCount).to.equal(4); + expect(db.saveDocs.callCount).to.equal(5); expect(db.saveDocs.args).to.deep.equal([ [DATABASES[0].db, [{ _id: 'one', _deleted: true }]], [DATABASES[1].db, [{ _id: 'two', _deleted: true }]], [DATABASES[2].db, [{ _id: 'three', _deleted: true }]], [DATABASES[3].db, [{ _id: 'four', _deleted: true }]], + [DATABASES[4].db, [{ _id: 'five', _deleted: true }]], ]); }); @@ -128,17 +131,19 @@ describe('Setup utils', () => { ddocsService.getStagedDdocs.withArgs(DATABASES[1]).resolves([]); ddocsService.getStagedDdocs.withArgs(DATABASES[2]).resolves([{ _id: 'three' }]); ddocsService.getStagedDdocs.withArgs(DATABASES[3]).resolves([]); + ddocsService.getStagedDdocs.withArgs(DATABASES[4]).resolves([]); db.saveDocs.resolves(); await utils.__get__('deleteStagedDdocs')(); - expect(ddocsService.getStagedDdocs.callCount).to.equal(4); + expect(ddocsService.getStagedDdocs.callCount).to.equal(5); expect(ddocsService.getStagedDdocs.args).to.deep.equal([ [DATABASES[0]], [DATABASES[1]], [DATABASES[2]], [DATABASES[3]], + [DATABASES[4]], ]); expect(db.saveDocs.callCount).to.equal(2); expect(db.saveDocs.args).to.deep.equal([ @@ -154,6 +159,7 @@ describe('Setup utils', () => { mockDb(db.sentinel); mockDb(db.medicLogs); mockDb(db.medicUsersMeta); + mockDb(db.users); utils.cleanup(); @@ -161,11 +167,13 @@ describe('Setup utils', () => { expect(db.sentinel.compact.callCount).to.equal(1); expect(db.medicLogs.compact.callCount).to.equal(1); expect(db.medicUsersMeta.compact.callCount).to.equal(1); + expect(db.users.compact.callCount).to.equal(1); expect(db.medic.viewCleanup.callCount).to.equal(1); expect(db.sentinel.viewCleanup.callCount).to.equal(1); expect(db.medicLogs.viewCleanup.callCount).to.equal(1); expect(db.medicUsersMeta.viewCleanup.callCount).to.equal(1); + expect(db.users.viewCleanup.callCount).to.equal(1); }); it('should catch errors', async () => { @@ -173,6 +181,7 @@ describe('Setup utils', () => { mockDb(db.sentinel); mockDb(db.medicLogs); mockDb(db.medicUsersMeta); + mockDb(db.users); db.sentinel.compact.rejects({ some: 'error' }); utils.cleanup(); @@ -208,19 +217,22 @@ describe('Setup utils', () => { { _id: '_design/meta1', views: { usersmeta1: {} } }, { _id: '_design/meta2', views: { usersmeta2: {} } }, ]; + const usersDdocs = [{ _id: '_design/users', views: { users1: {} } }]; fs.promises.readFile.withArgs('localDdocs/medic.json').resolves(genDdocsJson(medicDdocs)); fs.promises.readFile.withArgs('localDdocs/sentinel.json').resolves(genDdocsJson(sentinelDdocs)); fs.promises.readFile.withArgs('localDdocs/logs.json').resolves(genDdocsJson(logsDdocs)); fs.promises.readFile.withArgs('localDdocs/users-meta.json').resolves(genDdocsJson(usersMetaDdocs)); + fs.promises.readFile.withArgs('localDdocs/users.json').resolves(genDdocsJson(usersDdocs)); const result = await utils.getDdocDefinitions(); - expect(result.size).to.equal(4); + expect(result.size).to.equal(5); expect(result.get(DATABASES[0])).to.deep.equal(medicDdocs); expect(result.get(DATABASES[1])).to.deep.equal(sentinelDdocs); expect(result.get(DATABASES[2])).to.deep.equal(logsDdocs); expect(result.get(DATABASES[3])).to.deep.equal(usersMetaDdocs); + expect(result.get(DATABASES[4])).to.deep.equal(usersDdocs); expect(db.builds.get.callCount).to.equal(0); expect(fs.promises.readFile.args).to.deep.equal([ @@ -228,6 +240,7 @@ describe('Setup utils', () => { ['localDdocs/sentinel.json', 'utf-8'], ['localDdocs/logs.json', 'utf-8'], ['localDdocs/users-meta.json', 'utf-8'], + ['localDdocs/users.json', 'utf-8'], ]); }); @@ -439,12 +452,13 @@ describe('Setup utils', () => { { _id: '_design/meta1', views: { usersmeta1: {} } }, { _id: '_design/meta2', views: { usersmeta2: {} } }, ]); + ddocDefinitions.set(DATABASES[4], [{ _id: '_design/users', views: { users1: {} } }]); sinon.stub(db, 'saveDocs').resolves(); await utils.saveStagedDdocs(ddocDefinitions); - expect(db.saveDocs.callCount).to.equal(4); + expect(db.saveDocs.callCount).to.equal(5); expect(db.saveDocs.args[0]).to.deep.equal([db.medic, [ { _id: '_design/:staged:medic', views: { medic1: {}, medic2: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:medic-client', views: { client1: {}, client2: {} }, deploy_info: deployInfo }, @@ -459,6 +473,9 @@ describe('Setup utils', () => { { _id: '_design/:staged:meta1', views: { usersmeta1: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:meta2', views: { usersmeta2: {} }, deploy_info: deployInfo }, ]]); + expect(db.saveDocs.args[4]).to.deep.equal([db.users, [ + { _id: '_design/:staged:users', views: { users1: {} }, deploy_info: deployInfo } + ]]); }); it('should delete eventual _rev properties', async () => { @@ -477,12 +494,13 @@ describe('Setup utils', () => { { _id: '_design/meta1', _rev: 100, views: { usersmeta1: {} } }, { _id: '_design/meta2', _rev: 582, views: { usersmeta2: {} } }, ]); + ddocDefinitions.set(DATABASES[4], [{ _id: '_design/users', views: { users1: {} } }]); sinon.stub(db, 'saveDocs').resolves(); await utils.saveStagedDdocs(ddocDefinitions); - expect(db.saveDocs.callCount).to.equal(4); + expect(db.saveDocs.callCount).to.equal(5); expect(db.saveDocs.args[0]).to.deep.equal([db.medic, [ { _id: '_design/:staged:medic', views: { medic: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:medic-client', views: { clienta: {}, clientb: {} }, deploy_info: deployInfo }, @@ -497,6 +515,9 @@ describe('Setup utils', () => { { _id: '_design/:staged:meta1', views: { usersmeta1: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:meta2', views: { usersmeta2: {} }, deploy_info: deployInfo }, ]]); + expect(db.saveDocs.args[4]).to.deep.equal([db.users, [ + { _id: '_design/:staged:users', views: { users1: {} }, deploy_info: deployInfo } + ]]); }); it('should work when db has been removed from upgrade', async () => { @@ -514,12 +535,13 @@ describe('Setup utils', () => { { _id: '_design/meta1', views: { usersmeta1: {} } }, { _id: '_design/meta2', views: { usersmeta2: {} } }, ]); + ddocDefinitions.set(DATABASES[4], [{ _id: '_design/users', views: { users1: {} } }]); sinon.stub(db, 'saveDocs').resolves(); await utils.saveStagedDdocs(ddocDefinitions); - expect(db.saveDocs.callCount).to.equal(4); + expect(db.saveDocs.callCount).to.equal(5); expect(db.saveDocs.args[0]).to.deep.equal([db.medic, [ { _id: '_design/:staged:medic', views: { medic: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:medic-client', views: { clienta: {}, clientb: {} }, deploy_info: deployInfo }, @@ -532,6 +554,9 @@ describe('Setup utils', () => { { _id: '_design/:staged:meta1', views: { usersmeta1: {} }, deploy_info: deployInfo }, { _id: '_design/:staged:meta2', views: { usersmeta2: {} }, deploy_info: deployInfo }, ]]); + expect(db.saveDocs.args[4]).to.deep.equal([db.users, [ + { _id: '_design/:staged:users', views: { users1: {} }, deploy_info: deployInfo } + ]]); }); it('should throw error if staging fails', async () => { @@ -678,6 +703,7 @@ describe('Setup utils', () => { { doc: { _id: '_design/logs1', _rev: '3', field: 'b', deploy_info: deployInfoOld } }, ] }); // one extra staged ddoc sinon.stub(db.medicUsersMeta, 'allDocs').resolves({ rows: [] }); // no ddocs + sinon.stub(db.users, 'allDocs').resolves({ rows: [] }); // no ddocs sinon.stub(db, 'saveDocs').resolves(); @@ -693,8 +719,10 @@ describe('Setup utils', () => { expect(db.medicLogs.allDocs.args[0]).to.deep.equal(allDocsArgs); expect(db.medicUsersMeta.allDocs.callCount).to.equal(1); expect(db.medicUsersMeta.allDocs.args[0]).to.deep.equal(allDocsArgs); + expect(db.users.allDocs.callCount).to.equal(1); + expect(db.users.allDocs.args[0]).to.deep.equal(allDocsArgs); - expect(db.saveDocs.callCount).to.equal(4); + expect(db.saveDocs.callCount).to.equal(5); expect(db.saveDocs.args[0]).to.deep.equal([db.medic, [ { _id: '_design/medic', _rev: '2', new: true, deploy_info: deployInfoExpected }, { _id: '_design/medic-client', _rev: '3', new: true, deploy_info: deployInfoExpected }, @@ -707,6 +735,7 @@ describe('Setup utils', () => { { _id: '_design/logs2', deploy_info: deployInfoExpected }, ]]); expect(db.saveDocs.args[3]).to.deep.equal([db.medicUsersMeta, []]); + expect(db.saveDocs.args[4]).to.deep.equal([db.users, []]); }); it('should throw an error when getting ddocs fails', async () => { From b32fcf049002c6e5fae42bddbaa761ba1e58ca67 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Thu, 11 Apr 2024 12:52:38 -0500 Subject: [PATCH 49/65] Tighten up code --- .../migrations/add-contact-id-to-user-docs.js | 35 ++++++++----------- shared-libs/user-management/src/users.js | 4 +-- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js index ece8966bfe8..60fdd9a2475 100644 --- a/api/src/migrations/add-contact-id-to-user-docs.js +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -3,17 +3,12 @@ const logger = require('../logger'); const BATCH_SIZE = 100; -const getUserSettingsDocs = async (skip = 0) => { - const options = { - include_docs: true, - limit: BATCH_SIZE, - key: ['user-settings'], - skip, - }; - return (await db.medic.query('medic-client/doc_by_type', options)) - .rows - .map(row => row.doc); -}; +const getUserSettingsDocs = async (skip = 0) => db.medic.query('medic-client/doc_by_type', { + include_docs: true, + limit: BATCH_SIZE, + key: ['user-settings'], + skip, +}).then(({ rows }) => rows.map(({ doc }) => doc)); const getUpdatedUserDoc = (userSettingsDocs) => (userDoc, index) => { const { _id, contact_id } = userSettingsDocs[index]; @@ -24,18 +19,16 @@ const getUpdatedUserDoc = (userSettingsDocs) => (userDoc, index) => { return { ...userDoc, contact_id }; }; -const updateUsersDatabase = async (userSettingsDocs) => { - const allDocsOptions = { - include_docs: true, - keys: userSettingsDocs.map(doc => doc._id), - }; - const updatedUsersDocs = (await db.users.allDocs(allDocsOptions)) - .rows - .map(row => row.doc) +const updateUsersDatabase = async (userSettingsDocs) => db.users.allDocs({ + include_docs: true, + keys: userSettingsDocs.map(doc => doc._id), +}).then(({ rows }) => { + const updatedUsersDocs = rows + .map(({ doc }) => doc) .map(getUpdatedUserDoc(userSettingsDocs)) .filter(Boolean); - await db.users.bulkDocs(updatedUsersDocs); -}; + return db.users.bulkDocs(updatedUsersDocs); +}); const runBatch = async (skip = 0) => { const userSettingsDocs = await getUserSettingsDocs(skip); diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index ad8bf3cf73e..46942787d10 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -108,7 +108,7 @@ const getDocID = doc => { const queryDocs = (db, view, key) => db .query(view, { include_docs: true, key }) - .then(result => result.rows.map(row => row.doc)); + .then(({ rows }) => rows.map(({ doc }) => doc)); const getAllUserSettings = () => queryDocs(db.medic, 'medic-client/doc_by_type', ['user-settings']); @@ -117,7 +117,7 @@ const getSettingsByIds = async (ids) => { const { results } = await db.medic.bulkGet({ docs }); return results .map(result => result?.docs?.[0]?.ok) - .filter(doc => doc); + .filter(Boolean); }; const getAllUsers = async () => db.users From 6f720ea34c6dd591f9b4d63c2ffca5348faa4d0d Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Thu, 11 Apr 2024 13:35:20 -0500 Subject: [PATCH 50/65] Add additional user-management assertions for new functionality --- shared-libs/user-management/test/unit/users.spec.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index a560893e873..cd8d3dd4513 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -134,13 +134,16 @@ describe('Users service', () => { chai.expect(user.name ).to.equal('john'); }); - it('reassigns place field', () => { + it('reassigns place and contact fields', () => { const data = { - place: 'abc' + place: 'abc', + contact: 'xyz' }; const user = service.__get__('getUserUpdates')('john', data); chai.expect(user.place).to.equal(undefined); + chai.expect(user.contact).to.equal(undefined); chai.expect(user.facility_id).to.equal('abc'); + chai.expect(user.contact_id).to.equal('xyz'); }); }); @@ -2401,6 +2404,7 @@ describe('Users service', () => { }; service.__set__('validateUser', sinon.stub().resolves({ facility_id: 'maine', + contact_id: 1, roles: ['mm-online'] })); service.__set__('validateUserSettings', sinon.stub().resolves({ @@ -2418,6 +2422,7 @@ describe('Users service', () => { chai.expect(db.users.put.callCount).to.equal(1); const user = db.users.put.args[0][0]; chai.expect(user.facility_id).to.equal(null); + chai.expect(user.contact_id).to.equal(null); }); }); From 9c6ed2e47f81cffd8adc613bb2333cde43913012 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Thu, 11 Apr 2024 14:00:02 -0500 Subject: [PATCH 51/65] Add user-settings contact_id assertions back in --- tests/integration/api/controllers/users.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/api/controllers/users.spec.js b/tests/integration/api/controllers/users.spec.js index 366f7c0b7b4..2749de1e2bd 100644 --- a/tests/integration/api/controllers/users.spec.js +++ b/tests/integration/api/controllers/users.spec.js @@ -1041,7 +1041,7 @@ describe('Users API', () => { roles: user.roles, }; expectCorrectUser(userInDb, extraProps); - expectCorrectUserSettings(userSettings, extraProps); + expectCorrectUserSettings(userSettings, { ...extraProps, contact_id: user.contact._id }); chai.expect(userInDb.token_login).to.be.undefined; chai.expect(userSettings.token_login).to.be.undefined; await expectPasswordLoginToWork(user); @@ -1064,6 +1064,7 @@ describe('Users API', () => { expectCorrectUser(userInDb, { ...extraProps, roles: ['new_role'] }); expectCorrectUserSettings(userSettings, { ...extraProps, + contact_id: user.contact._id, roles: ['new_role'], phone: '+40744898989', }); @@ -1155,7 +1156,7 @@ describe('Users API', () => { roles: user.roles, }; expectCorrectUser(userInDb, extraProps); - expectCorrectUserSettings(userSettings, extraProps); + expectCorrectUserSettings(userSettings, { ...extraProps, contact_id: user.contact._id }); chai.expect(userInDb.token_login).to.be.ok; chai.expect(userInDb.token_login).to.have.keys(['active', 'token', 'expiration_date' ]); chai.expect(userInDb.token_login).to.include({ active: true }); @@ -1201,6 +1202,7 @@ describe('Users API', () => { expectCorrectUser(userInDb, { ...extraProps, roles: ['new_role'] }); expectCorrectUserSettings(userSettings, { ...extraProps, + contact_id: user.contact._id, roles: ['new_role'], phone: '+40744898989', }); From febf3cc9e308a2b78725571ee3ddf5a9cae3cfd4 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:11:30 -0500 Subject: [PATCH 52/65] Simplify from chained lodash calls Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- api/src/controllers/users.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index ef986b5ecd7..d1e759b2ced 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -112,10 +112,10 @@ const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; const getUserList = async (req) => { await auth.check(req, 'can_view_users'); - const filters = _.chain(req.query) - .pick(['facility_id', 'contact_id']) // keep only supported filtering properties - .mapKeys((value, key) => _.camelCase(key)) // rename them to keep consistent variable names - .value(); +const filters = { + facilityId: req.query.facility_id, + contactId: req.query.contact_id, +}; return await users.getList(filters); }; From 0ea6ac19ce7de54fb976270ae8a10d3a34f6b500 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:12:25 -0500 Subject: [PATCH 53/65] Fix indent --- api/src/controllers/users.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index d1e759b2ced..b58b65c0acb 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -112,10 +112,10 @@ const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; const getUserList = async (req) => { await auth.check(req, 'can_view_users'); -const filters = { - facilityId: req.query.facility_id, - contactId: req.query.contact_id, -}; + const filters = { + facilityId: req.query.facility_id, + contactId: req.query.contact_id, + }; return await users.getList(filters); }; From a8acee88c15ec0fdd179e405a047fe0e502ba431 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:17:05 -0500 Subject: [PATCH 54/65] Use allDocs instead of bulkGet Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- shared-libs/user-management/src/users.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 46942787d10..80e1e0933f5 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -114,10 +114,8 @@ const getAllUserSettings = () => queryDocs(db.medic, 'medic-client/doc_by_type', const getSettingsByIds = async (ids) => { const docs = ids.map(id => ({ id })); - const { results } = await db.medic.bulkGet({ docs }); - return results - .map(result => result?.docs?.[0]?.ok) - .filter(Boolean); +const { rows } = await db.medic.allDocs({ keys: ids, include_docs: true }); +return rows.map(row => row.doc).filter(Boolean); }; const getAllUsers = async () => db.users From 6902ddd392ef50feede4c9f77d7fc33ac65437e8 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:18:22 -0500 Subject: [PATCH 55/65] Clean up allDocs call --- shared-libs/user-management/src/users.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 80e1e0933f5..27c15db8dd7 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -113,9 +113,10 @@ const queryDocs = (db, view, key) => db const getAllUserSettings = () => queryDocs(db.medic, 'medic-client/doc_by_type', ['user-settings']); const getSettingsByIds = async (ids) => { - const docs = ids.map(id => ({ id })); -const { rows } = await db.medic.allDocs({ keys: ids, include_docs: true }); -return rows.map(row => row.doc).filter(Boolean); + const { rows } = await db.medic.allDocs({ keys: ids, include_docs: true }); + return rows + .map(row => row.doc) + .filter(Boolean); }; const getAllUsers = async () => db.users From 348afc86950a340c3a2f21b162ceed15eca08b9f Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:21:36 -0500 Subject: [PATCH 56/65] Filter allDocs call on _users db to avoid ddocs Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- shared-libs/user-management/src/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 27c15db8dd7..1132d2031fa 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -120,7 +120,7 @@ const getSettingsByIds = async (ids) => { }; const getAllUsers = async () => db.users - .allDocs({ include_docs: true }) +.allDocs({ include_docs: true, start_key: 'org.couchdb.user:', end_key: 'org.couchdb.user:\ufff0' }) .then(({ rows }) => rows.map(({ doc }) => doc)); const getUsers = async ({ facilityId, contactId }) => { From f99cd6dccf66fa98a8570ca1d11021b998cd5e5f Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:22:12 -0500 Subject: [PATCH 57/65] Fix indent --- shared-libs/user-management/src/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 1132d2031fa..f1b6dc9969f 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -120,7 +120,7 @@ const getSettingsByIds = async (ids) => { }; const getAllUsers = async () => db.users -.allDocs({ include_docs: true, start_key: 'org.couchdb.user:', end_key: 'org.couchdb.user:\ufff0' }) + .allDocs({ include_docs: true, start_key: 'org.couchdb.user:', end_key: 'org.couchdb.user:\ufff0' }) .then(({ rows }) => rows.map(({ doc }) => doc)); const getUsers = async ({ facilityId, contactId }) => { From 5325724bac9dfb993d9411832603bdbb4725b2b9 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:23:34 -0500 Subject: [PATCH 58/65] Reorder facilityId filtering logic Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- shared-libs/user-management/src/users.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index f1b6dc9969f..15bdffda28f 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -129,9 +129,10 @@ const getUsers = async ({ facilityId, contactId }) => { } const usersForContactId = await queryDocs(db.users, 'users/users_by_contact_id', contactId); - if (facilityId) { - return usersForContactId.filter(user => user.facility_id === facilityId); - } +if (!facilityId) { + return usersForContactId; +} +return usersForContactId.filter(user => user.facility_id === facilityId); return usersForContactId; }; From a7fbff19f1ae4ea49085ac432d7e0ab41f8feaee Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:24:26 -0500 Subject: [PATCH 59/65] Fix formatting --- shared-libs/user-management/src/users.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 15bdffda28f..66207600a49 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -129,12 +129,11 @@ const getUsers = async ({ facilityId, contactId }) => { } const usersForContactId = await queryDocs(db.users, 'users/users_by_contact_id', contactId); -if (!facilityId) { - return usersForContactId; -} -return usersForContactId.filter(user => user.facility_id === facilityId); + if (!facilityId) { + return usersForContactId; + } - return usersForContactId; + return usersForContactId.filter(user => user.facility_id === facilityId); }; const getUsersAndSettings = async (filters) => { From 2671032bc45330754f866af7a2077f01bccdfbc1 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:37:31 -0500 Subject: [PATCH 60/65] Clean up getUserSettings function and fix tests --- shared-libs/user-management/src/users.js | 2 +- .../user-management/test/unit/users.spec.js | 79 +++++++++---------- 2 files changed, 37 insertions(+), 44 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 66207600a49..1429af90a71 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -791,7 +791,7 @@ const getUserDocsByName = (name) => { const getUserSettings = async({ name }) => { const [ user, medicUser ] = await getUserDocsByName(name); - Object.assign(medicUser, _.pick(user, 'name', 'roles', 'facility_id')); + Object.assign(medicUser, _.pick(user, 'name', 'roles', 'facility_id', 'contact_id')); return hydrateUserSettings(medicUser); }; diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index cd8d3dd4513..086c066d421 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -37,7 +37,6 @@ describe('Users service', () => { }); db.init({ medic: { - bulkGet: sinon.stub(), get: sinon.stub(), put: sinon.stub(), allDocs: sinon.stub(), @@ -223,20 +222,18 @@ describe('Users service', () => { }).resolves(usersResponse); const userSettingsResponse = { - results: [{ - docs: [{ - ok: { - _id: 'org.couchdb.user:x', - name: 'lucas', - fullname: 'Lucas M', - email: 'l@m.com', - phone: '123456789', - facility_id: 'c', - }, - }], + rows: [{ + doc: { + _id: 'org.couchdb.user:x', + name: 'lucas', + fullname: 'Lucas M', + email: 'l@m.com', + phone: '123456789', + facility_id: 'c', + }, }], }; - db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:x' }] }).resolves(userSettingsResponse); + db.medic.allDocs.withArgs({ keys: ['org.couchdb.user:x'], include_docs: true }).resolves(userSettingsResponse); const data = await service.getList(filters); chai.expect(data.length).to.equal(1); @@ -269,22 +266,20 @@ describe('Users service', () => { }).resolves(usersResponse); const userSettingsResponse = { - results: [{ - docs: [{ - ok: { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, - }], + rows: [{ + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, }], }; - db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); + db.medic.allDocs.withArgs({ keys: ['org.couchdb.user:y'], include_docs: true }).resolves(userSettingsResponse); const data = await service.getList(filters); chai.expect(data.length).to.equal(1); @@ -329,22 +324,20 @@ describe('Users service', () => { }).resolves(usersResponse); const userSettingsResponse = { - results: [{ - docs: [{ - ok: { - _id: 'org.couchdb.user:y', - name: 'milan', - fullname: 'Milan A', - email: 'm@a.com', - phone: '987654321', - external_id: 'LTT093', - facility_id: 'b', - contact_id: 'milan-contact', - }, - }], + rows: [{ + doc: { + _id: 'org.couchdb.user:y', + name: 'milan', + fullname: 'Milan A', + email: 'm@a.com', + phone: '987654321', + external_id: 'LTT093', + facility_id: 'b', + contact_id: 'milan-contact', + }, }], }; - db.medic.bulkGet.withArgs({ docs: [{ id: 'org.couchdb.user:y' }] }).resolves(userSettingsResponse); + db.medic.allDocs.withArgs({ keys: ['org.couchdb.user:y'], include_docs: true }).resolves(userSettingsResponse); const data = await service.getList(filters); chai.expect(db.users.query.callCount).to.equal(1); @@ -535,8 +528,8 @@ describe('Users service', () => { describe('getUserSettings', () => { it('returns medic user doc with facility from couchdb user doc', () => { - db.users.get.resolves({ name: 'steve', facility_id: 'steveVille', roles: ['b'] }); - db.medic.get.resolves({ name: 'steve2', facility_id: 'otherville', contact_id: 'steve', roles: ['c'] }); + db.users.get.resolves({ name: 'steve', facility_id: 'steveVille', contact_id: 'steve', roles: ['b'] }); + db.medic.get.resolves({ name: 'steve2', facility_id: 'otherville', contact_id: 'not_steve', roles: ['c'] }); db.medic.allDocs.resolves({ rows: [ { id: 'steveVille', key: 'steveVille', doc: { _id: 'steveVille', place_id: 'steve_ville', name: 'steve V' } }, From 2c5f740a4152a2094e17b85af7fda90384e4975b Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 14:49:31 -0500 Subject: [PATCH 61/65] Add assertion that facility_id not modified by migration --- api/tests/integration/migrations/add-contact-id-to-user.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/tests/integration/migrations/add-contact-id-to-user.js b/api/tests/integration/migrations/add-contact-id-to-user.js index c74da40c672..03da1982bec 100644 --- a/api/tests/integration/migrations/add-contact-id-to-user.js +++ b/api/tests/integration/migrations/add-contact-id-to-user.js @@ -93,7 +93,8 @@ describe('add-contact-id-to-user migration', function() { }, { ...createUserDoc(userSettingsDocs[1]), - contact_id: 'old-contact' + contact_id: 'old-contact', + facility_id: 'different-facility' }, ]; await writeUserDocs(userDocs); @@ -108,5 +109,7 @@ describe('add-contact-id-to-user migration', function() { delete expectedUserDoc1.contact_id; expect(updatedUserDoc1).to.deep.include(expectedUserDoc1); expect(updatedUserDoc1.contact_id).to.be.undefined; + // The _users doc facility_id will not be updated + expect(updatedUserDoc1.facility_id).to.equal('different-facility'); }); }); From 903a6360d79791777b50b5480aa57273a25226ed Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Mon, 15 Apr 2024 16:27:33 -0500 Subject: [PATCH 62/65] Fix broken api controller tests --- api/src/controllers/users.js | 4 ++-- api/tests/mocha/controllers/users.spec.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/src/controllers/users.js b/api/src/controllers/users.js index b58b65c0acb..4c5978bbcac 100644 --- a/api/src/controllers/users.js +++ b/api/src/controllers/users.js @@ -113,8 +113,8 @@ const getAppUrl = (req) => `${req.protocol}://${req.hostname}`; const getUserList = async (req) => { await auth.check(req, 'can_view_users'); const filters = { - facilityId: req.query.facility_id, - contactId: req.query.contact_id, + facilityId: req.query?.facility_id, + contactId: req.query?.contact_id, }; return await users.getList(filters); }; diff --git a/api/tests/mocha/controllers/users.spec.js b/api/tests/mocha/controllers/users.spec.js index de3c1ed348c..f2091817477 100644 --- a/api/tests/mocha/controllers/users.spec.js +++ b/api/tests/mocha/controllers/users.spec.js @@ -111,7 +111,8 @@ describe('Users Controller', () => { }; await controller.v2.get(req, res); - chai.expect(users.getList.firstCall.args[0]).to.deep.equal({ facilityId: 'chw-facility' }); + chai.expect(users.getList.firstCall.args[0]) + .to.deep.equal({ facilityId: 'chw-facility', contactId: undefined }); const result = res.json.args[0][0]; chai.expect(result.length).to.equal(1); chai.expect(result[0].id).to.equal('org.couchdb.user:chw'); @@ -145,7 +146,7 @@ describe('Users Controller', () => { req.query = { roles: ['chw'], name: 'admin' }; await controller.v2.get(req, res); - chai.expect(users.getList.firstCall.args[0]).to.deep.equal({}); + chai.expect(users.getList.firstCall.args[0]).to.deep.equal({ facilityId: undefined, contactId: undefined }); const result = res.json.args[0][0]; chai.expect(result.length).to.equal(3); chai.expect(result[0].id).to.equal('org.couchdb.user:admin'); From d420d4835124ace6ae30e9c795ba4035e9f2f996 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Tue, 16 Apr 2024 09:48:31 -0500 Subject: [PATCH 63/65] Combine views into users_by_field --- .../users-db/users/views/users_by_contact_id/map.js | 5 ----- .../users-db/users/views/users_by_facility_id/map.js | 5 ----- ddocs/users-db/users/views/users_by_field/map.js | 8 ++++++++ shared-libs/user-management/src/users.js | 4 ++-- shared-libs/user-management/test/unit/users.spec.js | 12 ++++++------ 5 files changed, 16 insertions(+), 18 deletions(-) delete mode 100644 ddocs/users-db/users/views/users_by_contact_id/map.js delete mode 100644 ddocs/users-db/users/views/users_by_facility_id/map.js create mode 100644 ddocs/users-db/users/views/users_by_field/map.js diff --git a/ddocs/users-db/users/views/users_by_contact_id/map.js b/ddocs/users-db/users/views/users_by_contact_id/map.js deleted file mode 100644 index d61504b977c..00000000000 --- a/ddocs/users-db/users/views/users_by_contact_id/map.js +++ /dev/null @@ -1,5 +0,0 @@ -function(doc) { - if (doc.contact_id) { - emit(doc.contact_id); - } -} diff --git a/ddocs/users-db/users/views/users_by_facility_id/map.js b/ddocs/users-db/users/views/users_by_facility_id/map.js deleted file mode 100644 index f468b6354ed..00000000000 --- a/ddocs/users-db/users/views/users_by_facility_id/map.js +++ /dev/null @@ -1,5 +0,0 @@ -function(doc) { - if (doc.facility_id) { - emit(doc.facility_id); - } -} diff --git a/ddocs/users-db/users/views/users_by_field/map.js b/ddocs/users-db/users/views/users_by_field/map.js new file mode 100644 index 00000000000..a260317467c --- /dev/null +++ b/ddocs/users-db/users/views/users_by_field/map.js @@ -0,0 +1,8 @@ +function(doc) { + if (doc.contact_id) { + emit(['contact_id', doc.contact_id]); + } + if (doc.facility_id) { + emit(['facility_id', doc.facility_id]); + } +} diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 1429af90a71..93525dc52c4 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -125,10 +125,10 @@ const getAllUsers = async () => db.users const getUsers = async ({ facilityId, contactId }) => { if (!contactId) { - return queryDocs(db.users, 'users/users_by_facility_id', facilityId); + return queryDocs(db.users, 'users/users_by_field', ['facility_id', facilityId]); } - const usersForContactId = await queryDocs(db.users, 'users/users_by_contact_id', contactId); + const usersForContactId = await queryDocs(db.users, 'users/users_by_field', ['contact_id', contactId]); if (!facilityId) { return usersForContactId; } diff --git a/shared-libs/user-management/test/unit/users.spec.js b/shared-libs/user-management/test/unit/users.spec.js index 086c066d421..f6a0beb3cc8 100644 --- a/shared-libs/user-management/test/unit/users.spec.js +++ b/shared-libs/user-management/test/unit/users.spec.js @@ -216,9 +216,9 @@ describe('Users service', () => { } }], }; - db.users.query.withArgs('users/users_by_facility_id', { + db.users.query.withArgs('users/users_by_field', { include_docs: true, - key: filters.facilityId, + key: ['facility_id', filters.facilityId], }).resolves(usersResponse); const userSettingsResponse = { @@ -260,9 +260,9 @@ describe('Users service', () => { } }], }; - db.users.query.withArgs('users/users_by_contact_id', { + db.users.query.withArgs('users/users_by_field', { include_docs: true, - key: filters.contactId, + key: ['contact_id', filters.contactId], }).resolves(usersResponse); const userSettingsResponse = { @@ -318,9 +318,9 @@ describe('Users service', () => { } ], }; - db.users.query.withArgs('users/users_by_contact_id', { + db.users.query.withArgs('users/users_by_field', { include_docs: true, - key: filters.contactId, + key: ['contact_id', filters.contactId], }).resolves(usersResponse); const userSettingsResponse = { From 3b75cca9764654533f12e55b3b2ebccd10ab47df Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Tue, 16 Apr 2024 14:51:56 -0500 Subject: [PATCH 64/65] Add unit tests for migration --- .../migrations/add-contact-id-to-user-docs.js | 3 + ...user.js => add-contact-id-to-user-docs.js} | 0 .../add-contact-id-to-user-docs.spec.js | 178 ++++++++++++++++++ 3 files changed, 181 insertions(+) rename api/tests/integration/migrations/{add-contact-id-to-user.js => add-contact-id-to-user-docs.js} (100%) create mode 100644 api/tests/mocha/migrations/add-contact-id-to-user-docs.spec.js diff --git a/api/src/migrations/add-contact-id-to-user-docs.js b/api/src/migrations/add-contact-id-to-user-docs.js index 60fdd9a2475..042c810db83 100644 --- a/api/src/migrations/add-contact-id-to-user-docs.js +++ b/api/src/migrations/add-contact-id-to-user-docs.js @@ -27,6 +27,9 @@ const updateUsersDatabase = async (userSettingsDocs) => db.users.allDocs({ .map(({ doc }) => doc) .map(getUpdatedUserDoc(userSettingsDocs)) .filter(Boolean); + if (!updatedUsersDocs.length) { + return; + } return db.users.bulkDocs(updatedUsersDocs); }); diff --git a/api/tests/integration/migrations/add-contact-id-to-user.js b/api/tests/integration/migrations/add-contact-id-to-user-docs.js similarity index 100% rename from api/tests/integration/migrations/add-contact-id-to-user.js rename to api/tests/integration/migrations/add-contact-id-to-user-docs.js diff --git a/api/tests/mocha/migrations/add-contact-id-to-user-docs.spec.js b/api/tests/mocha/migrations/add-contact-id-to-user-docs.spec.js new file mode 100644 index 00000000000..d9023919764 --- /dev/null +++ b/api/tests/mocha/migrations/add-contact-id-to-user-docs.spec.js @@ -0,0 +1,178 @@ +const sinon = require('sinon'); +const { expect } = require('chai'); +const db = require('../../../src/db'); +const logger = require('../../../src/logger'); +const migration = require('../../../src/migrations/add-contact-id-to-user-docs'); + +const BATCH_SIZE = 100; + +const createUserSettingsDoc = (id, contactId) => { + const userSettings = { + _id: id, + type: 'user-settings', + }; + if (contactId) { + userSettings.contact_id = contactId; + } + return userSettings; +}; + +const createUserDoc = id => { + return { + _id: id, + type: 'user', + contact_id: null, + }; +}; + +const createCouchResponse = docs => ({ rows: docs.map(doc => ({ doc })) }); + +const assertDocByTypeQueryArgs = (args, skip) => expect(args).to.deep.equal([ + 'medic-client/doc_by_type', + { + include_docs: true, + limit: BATCH_SIZE, + key: ['user-settings'], + skip + } +]); + +const getExpectedUserDoc = userSettingsDocs => (userDoc, index) => { + return { + ...userDoc, + contact_id: userSettingsDocs[index].contact_id + }; +}; + +describe('add-contact-id-to-user-docs migration', () => { + let medicQuery; + let usersAllDocs; + let usersBulkDocs; + let loggerWarn; + + beforeEach(() => { + medicQuery = sinon.stub(db.medic, 'query'); + usersAllDocs = sinon.stub(db.users, 'allDocs'); + usersBulkDocs = sinon.stub(db.users, 'bulkDocs'); + loggerWarn = sinon.spy(logger, 'warn'); + }); + + afterEach(() => sinon.restore()); + + it('has basic properties', () => { + const expectedCreationDate = new Date(2024, 5, 2).toDateString(); + + expect(migration.name).to.equal('add-contact-id-to-user-docs'); + expect(migration.created).to.exist; + expect(migration.created.toDateString()).to.equal(expectedCreationDate); + expect(migration.run).to.be.a('function'); + }); + + it('migrates the contact_id value from user-settings to _users', async () => { + const userSettingsDoc = createUserSettingsDoc('org.couchdb.user:test-chw-1', 'contact-1'); + medicQuery.resolves(createCouchResponse([userSettingsDoc])); + const userDoc = createUserDoc(userSettingsDoc._id); + usersAllDocs.resolves(createCouchResponse([userDoc])); + + await migration.run(); + + expect(medicQuery.calledOnce).to.be.true; + assertDocByTypeQueryArgs(medicQuery.args[0], 0); + expect(usersAllDocs.calledOnce).to.be.true; + expect(usersAllDocs.args[0]).to.deep.equal([{ include_docs: true, keys: [userSettingsDoc._id] }]); + expect(usersBulkDocs.calledOnce).to.be.true; + expect(usersBulkDocs.args[0]).to.deep.equal([[{ ...userDoc, contact_id: userSettingsDoc.contact_id }]]); + }); + + it('migrates the contact_id value for all batches', async () => { + const userSettingsFirstBatch = Array.from( + { length: BATCH_SIZE }, + (_, i) => createUserSettingsDoc(`org.couchdb.user:test-chw-${i}`, `contact-${i}`) + ); + const userSettingsSecondBatch = Array.from( + { length: BATCH_SIZE }, + (_, i) => createUserSettingsDoc(`org.couchdb.user:test-chw-11${i}`, `contact-11${i}`) + ); + const userSettingsThirdBatch = [createUserSettingsDoc(`org.couchdb.user:test-chw-222`, `contact-222`)]; + medicQuery.onFirstCall().resolves(createCouchResponse(userSettingsFirstBatch)); + medicQuery.onSecondCall().resolves(createCouchResponse(userSettingsSecondBatch)); + medicQuery.onThirdCall().resolves(createCouchResponse(userSettingsThirdBatch)); + + const userDocsFirstBatch = userSettingsFirstBatch.map(doc => createUserDoc(doc._id)); + const userDocsSecondBatch = userSettingsSecondBatch.map(doc => createUserDoc(doc._id)); + const userDocsThirdBatch = userSettingsThirdBatch.map(doc => createUserDoc(doc._id)); + usersAllDocs.onFirstCall().resolves(createCouchResponse(userDocsFirstBatch)); + usersAllDocs.onSecondCall().resolves(createCouchResponse(userDocsSecondBatch)); + usersAllDocs.onThirdCall().resolves(createCouchResponse(userDocsThirdBatch)); + + await migration.run(); + + expect(medicQuery.calledThrice).to.be.true; + assertDocByTypeQueryArgs(medicQuery.args[0], 0); + assertDocByTypeQueryArgs(medicQuery.args[1], BATCH_SIZE); + assertDocByTypeQueryArgs(medicQuery.args[2], BATCH_SIZE * 2); + expect(usersAllDocs.calledThrice).to.be.true; + expect(usersAllDocs.args[0]).to.deep.equal([{ + include_docs: true, + keys: userSettingsFirstBatch.map(doc => doc._id) + }]); + expect(usersAllDocs.args[1]).to.deep.equal([{ + include_docs: true, + keys: userSettingsSecondBatch.map(doc => doc._id) + }]); + expect(usersAllDocs.args[2]).to.deep.equal([{ + include_docs: true, + keys: userSettingsThirdBatch.map(doc => doc._id) + }]); + expect(usersBulkDocs.calledThrice).to.be.true; + expect(usersBulkDocs.args[0]).to.deep.equal([userDocsFirstBatch.map(getExpectedUserDoc(userSettingsFirstBatch))]); + expect(usersBulkDocs.args[1]).to.deep.equal([userDocsSecondBatch.map(getExpectedUserDoc(userSettingsSecondBatch))]); + expect(usersBulkDocs.args[2]).to.deep.equal([userDocsThirdBatch.map(getExpectedUserDoc(userSettingsThirdBatch))]); + }); + + it('does nothing if no user-settings are found', async () => { + medicQuery.resolves(createCouchResponse([])); + + await migration.run(); + + expect(medicQuery.calledOnce).to.be.true; + assertDocByTypeQueryArgs(medicQuery.args[0], 0); + expect(usersAllDocs.notCalled).to.be.true; + expect(usersBulkDocs.notCalled).to.be.true; + }); + + it('does nothing if no _users docs are found', async () => { + const userSettingsDoc = createUserSettingsDoc('org.couchdb.user:test-chw-1', 'contact-1'); + medicQuery.resolves(createCouchResponse([userSettingsDoc])); + usersAllDocs.resolves(createCouchResponse([null])); + + await migration.run(); + + expect(medicQuery.calledOnce).to.be.true; + assertDocByTypeQueryArgs(medicQuery.args[0], 0); + expect(usersAllDocs.calledOnce).to.be.true; + expect(usersAllDocs.args[0]).to.deep.equal([{ include_docs: true, keys: [userSettingsDoc._id] }]); + expect(usersBulkDocs.notCalled).to.be.true; + expect(loggerWarn.calledOnce).to.be.true; + expect(loggerWarn.args[0]).to.deep.equal([`Could not find user with id "${userSettingsDoc._id}". Skipping it.`]); + }); + + it('overwrites any existing contact_id value in _users', async () => { + const userSettingsDoc = createUserSettingsDoc('org.couchdb.user:test-chw-1', 'contact-1'); + medicQuery.resolves(createCouchResponse([userSettingsDoc])); + const userDoc = createUserDoc(userSettingsDoc._id); + usersAllDocs.resolves(createCouchResponse([{ + ...userDoc, + contact_id: 'old-contact' + }])); + + await migration.run(); + + expect(medicQuery.calledOnce).to.be.true; + assertDocByTypeQueryArgs(medicQuery.args[0], 0); + expect(usersAllDocs.calledOnce).to.be.true; + expect(usersAllDocs.args[0]).to.deep.equal([{ include_docs: true, keys: [userSettingsDoc._id] }]); + expect(usersBulkDocs.calledOnce).to.be.true; + expect(usersBulkDocs.args[0]).to.deep.equal([[{ ...userDoc, contact_id: userSettingsDoc.contact_id }]]); + }); +}); From fe7cf0cd78800e6f5d50978ac497737f57db9044 Mon Sep 17 00:00:00 2001 From: Joshua Kuestersteffen Date: Wed, 17 Apr 2024 09:15:27 -0500 Subject: [PATCH 65/65] Fix filtering bug in user-management --- shared-libs/user-management/src/users.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shared-libs/user-management/src/users.js b/shared-libs/user-management/src/users.js index 93525dc52c4..893eb777d8e 100644 --- a/shared-libs/user-management/src/users.js +++ b/shared-libs/user-management/src/users.js @@ -123,7 +123,7 @@ const getAllUsers = async () => db.users .allDocs({ include_docs: true, start_key: 'org.couchdb.user:', end_key: 'org.couchdb.user:\ufff0' }) .then(({ rows }) => rows.map(({ doc }) => doc)); -const getUsers = async ({ facilityId, contactId }) => { +const getUsers = async (facilityId, contactId) => { if (!contactId) { return queryDocs(db.users, 'users/users_by_field', ['facility_id', facilityId]); } @@ -136,11 +136,11 @@ const getUsers = async ({ facilityId, contactId }) => { return usersForContactId.filter(user => user.facility_id === facilityId); }; -const getUsersAndSettings = async (filters) => { - if (_.isEmpty(filters)) { +const getUsersAndSettings = async ({ facilityId, contactId } = {}) => { + if (!facilityId && !contactId) { return Promise.all([getAllUsers(), getAllUserSettings()]); } - const users = await getUsers(filters); + const users = await getUsers(facilityId, contactId); const ids = users.map(({ _id }) => _id); const settings = await getSettingsByIds(ids); return [users, settings];