Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(#8877): Look up users from their facility_id or contact_id #8928

Merged
merged 66 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
4b3a3a1
move the auth check up in the parent function
m5r Mar 13, 2024
5819151
support `facility_id` query parameter
m5r Mar 13, 2024
a007db9
support `facility_id` query parameter for user settings too
m5r Mar 14, 2024
14281f6
support filtering with `contact_id` query parameter
m5r Mar 14, 2024
c6ac0f3
cleaner approach with/without filters
m5r Mar 18, 2024
5684089
good catch sonar
m5r Mar 18, 2024
a993c0f
fix unit tests
m5r Mar 18, 2024
fd18a9c
unit test `getList()` with filters
m5r Mar 18, 2024
d117a49
test api controller with filters
m5r Mar 18, 2024
e93c001
remove .only
m5r Mar 18, 2024
f78e568
Merge remote-tracking branch 'origin/master' into 8877-lookup-single-…
m5r Mar 20, 2024
0beaf25
bulk create users instead of creating them one by one
m5r Mar 20, 2024
039f0cb
oops how could I miss that
m5r Mar 20, 2024
f2cfda3
fix facility.spec.js
m5r Mar 20, 2024
48030f1
add integration tests for `GET /api/v2/users` with filters
m5r Mar 20, 2024
3fba6d8
log how long it takes in CI
m5r Mar 20, 2024
f08a34c
debug savedUser
m5r Mar 21, 2024
4b0bf8a
lint
m5r Mar 21, 2024
3d5455c
okay other tests in the suite are leaving users with no place or cont…
m5r Mar 21, 2024
4ddab32
try speeding it up by running filtered queries in parallel
m5r Mar 21, 2024
b0ddc30
remove logs
m5r Mar 21, 2024
c319cad
comment the lodash magic
m5r Mar 21, 2024
cd0dff7
Update shared-libs/user-management/src/users.js
m5r Mar 25, 2024
a9967a1
switch to `setting.facility_id`
m5r Mar 25, 2024
0e4b82a
add other unsupported query parameters to unit test
m5r Mar 25, 2024
5a605d1
Update shared-libs/user-management/test/unit/users.spec.js
m5r Mar 25, 2024
7d44261
fix user-management shared lib unit tests
m5r Mar 25, 2024
61cb17b
stub lower level db calls to cover functions `getAllUserSettings` and…
m5r Mar 25, 2024
06f15c8
integration test for `GET /api/v2/users` with filters
m5r Mar 27, 2024
6ae5c45
g checkout origin/master `ddocs/medic-db/medic-client/views/doc_by_ty…
m5r Apr 1, 2024
4dbe6b1
query with `facility_id` & `contact_id` filters on `_users` instead o…
m5r Apr 1, 2024
ab09351
`add-contact-id-to-user-docs` migration
m5r Apr 2, 2024
1c0a1cb
okay I think I found the place where all the magic happens
m5r Apr 3, 2024
8a703cd
fix tests
m5r Apr 4, 2024
16c39e3
reduce function cognitive coplexity
m5r Apr 4, 2024
42fc2f4
use async await
m5r Apr 4, 2024
bab9d56
test that a deleted user doesn't get returned by the route
m5r Apr 4, 2024
fa55975
remove .only
m5r Apr 4, 2024
302730f
test that both user and user-settings documents get updated with `fac…
m5r Apr 4, 2024
0165b84
move `auth.check()` up the chain
m5r Apr 4, 2024
08d24bc
use `contact_id` from `user` doc
m5r Apr 4, 2024
4667bec
increase batch size to 100
m5r Apr 4, 2024
3ce0587
remove duplicate test
m5r Apr 4, 2024
9703957
Add integration tests for migration
jkuester Apr 9, 2024
6a4cfd9
Update whole batch of users at the same time in migration
jkuester Apr 9, 2024
13475b6
Add users_by_facility_and_contact view to _users db
jkuester Apr 10, 2024
03ab927
Split out into two facility/contact views. Call views from user-manag…
jkuester Apr 10, 2024
5729789
Fix unit tests to expect queries
jkuester Apr 10, 2024
11994ae
Fix api unit tests
jkuester Apr 11, 2024
b32fcf0
Tighten up code
jkuester Apr 11, 2024
6f720ea
Add additional user-management assertions for new functionality
jkuester Apr 11, 2024
9c6ed2e
Add user-settings contact_id assertions back in
jkuester Apr 11, 2024
febf3cc
Simplify from chained lodash calls
jkuester Apr 15, 2024
0ea6ac1
Fix indent
jkuester Apr 15, 2024
a8acee8
Use allDocs instead of bulkGet
jkuester Apr 15, 2024
6902ddd
Clean up allDocs call
jkuester Apr 15, 2024
348afc8
Filter allDocs call on _users db to avoid ddocs
jkuester Apr 15, 2024
f99cd6d
Fix indent
jkuester Apr 15, 2024
5325724
Reorder facilityId filtering logic
jkuester Apr 15, 2024
a7fbff1
Fix formatting
jkuester Apr 15, 2024
2671032
Clean up getUserSettings function and fix tests
jkuester Apr 15, 2024
2c5f740
Add assertion that facility_id not modified by migration
jkuester Apr 15, 2024
903a636
Fix broken api controller tests
jkuester Apr 15, 2024
d420d48
Combine views into users_by_field
jkuester Apr 16, 2024
3b75cca
Add unit tests for migration
jkuester Apr 16, 2024
fe7cf0c
Fix filtering bug in user-management
jkuester Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions api/src/controllers/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ const getAppUrl = (req) => `${req.protocol}://${req.hostname}`;

const getUserList = async (req) => {
await auth.check(req, 'can_view_users');
return await users.getList();
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();
jkuester marked this conversation as resolved.
Show resolved Hide resolved
return await users.getList(filters);
};

const getType = user => {
Expand Down Expand Up @@ -233,7 +237,7 @@ module.exports = {
v2: {
get: async (req, res) => {
try {
const body = await getUserList(req, res);
const body = await getUserList(req);
res.json(body);
} catch (err) {
serverUtils.error(err, req, res);
Expand Down
46 changes: 46 additions & 0 deletions api/src/migrations/add-contact-id-to-user-docs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const db = require('../db');
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 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) {
await processDocument(row.doc);
}

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,
};
74 changes: 67 additions & 7 deletions api/tests/mocha/controllers/users.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,24 @@ describe('Users Controller', () => {
afterEach(() => sinon.restore());

describe('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', () => {
Expand Down Expand Up @@ -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);
Expand All @@ -92,6 +100,58 @@ 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',
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' });
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');
});
});

});
Expand Down
8 changes: 3 additions & 5 deletions shared-libs/user-management/src/libs/facility.js
Original file line number Diff line number Diff line change
@@ -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?.doc?.facility_id);
}
for (const setting of settings) {
ids.add(setting?.contact_id);
ids.add(user?.facility_id);
ids.add(user?.contact_id);
}
ids.delete(undefined);
if (!ids.size) {
Expand Down
69 changes: 51 additions & 18 deletions shared-libs/user-management/src/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const RESTRICTED_USER_EDITABLE_FIELDS = [

const USER_EDITABLE_FIELDS = RESTRICTED_USER_EDITABLE_FIELDS.concat([
'place',
'contact',
'type',
'roles',
]);
Expand Down Expand Up @@ -108,15 +109,33 @@ const getDocID = doc => {
const getAllUserSettings = () => {
const opts = {
include_docs: true,
key: ['user-settings']
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);
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(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) => {
Expand Down Expand Up @@ -335,21 +354,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,
contact: getDoc(setting.contact_id, facilities),
place: getDoc(user.facility_id, facilities),
m5r marked this conversation as resolved.
Show resolved Hide resolved
roles: user.roles,
contact: getDoc(user.contact_id, facilities),
external_id: setting.external_id,
known: user.doc.known
known: user.known
};
});
};
Expand Down Expand Up @@ -395,7 +414,7 @@ const getSettingsUpdates = (username, data) => {
};

const getUserUpdates = (username, data) => {
const ignore = ['type', 'place'];
const ignore = ['type', 'place', 'contact'];

const user = {
name: username,
Expand All @@ -418,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;
};
Expand Down Expand Up @@ -544,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)) {
Expand All @@ -555,6 +577,7 @@ const validateUserContact = (data, user, userSettings) => {
{'field': 'Contact'}
));
}
user.contact_id = null;
userSettings.contact_id = null;
}
};
Expand Down Expand Up @@ -770,9 +793,19 @@ const getUserSettings = async({ name }) => {
*/
module.exports = {
deleteUser: username => deleteUser(createID(username)),
getList: async () => {
const [ users, settings ] = await Promise.all([ getAllUsers(), getAllUserSettings() ]);
const facilities = await facility.list(users, settings);
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 facilities = await facility.list(users);
return mapUsers(users, settings, facilities);
},
getUserSettings,
jkuester marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
9 changes: 3 additions & 6 deletions shared-libs/user-management/test/unit/libs/facility.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@ const db = require('../../../src/libs/db');

describe('facility', () => {

const userA = { doc: { facility_id: 'a' } };
const userB = { doc: { 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' };
Expand Down Expand Up @@ -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' ]);
Expand Down
Loading
Loading