Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 8 additions & 7 deletions admin/src/js/services/message-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ angular.module('services').factory('MessageQueue',
$q,
$translate,
DB,
DataContext,
Languages,
MessageQueueUtils,
Settings
Expand All @@ -34,6 +35,8 @@ angular.module('services').factory('MessageQueue',
'use strict';
'ngInject';

const datasourcePromise = DataContext.then(dataContext => dataContext.getDatasource());

const findSummary = function(summaries, message) {
if (!message.sms || !message.sms.to) {
return;
Expand Down Expand Up @@ -94,13 +97,11 @@ angular.module('services').factory('MessageQueue',
return messages;
}

return DB({ remote: true })
.query('medic-client/contacts_by_phone', { keys: phoneNumbers })
.then(function(contactsByPhone) {
const ids = contactsByPhone.rows.map(function(row) {
return row.id;
});

return datasourcePromise
.then(function(datasource) {
return datasource.v1.contact.collectUuidsByPhones(phoneNumbers);
})
.then(function(ids) {
return DB({ remote: true }).query('medic/doc_summaries_by_id', { keys: ids });
})
.then(function(summaries) {
Expand Down
61 changes: 23 additions & 38 deletions admin/tests/unit/services/message-queue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ describe('MessageQueue service', function() {
let query;
let translate;
let clock;
let collectUuidsByPhones;
let DataContext;

beforeEach(() => {
Settings = sinon.stub();
Expand All @@ -34,6 +36,9 @@ describe('MessageQueue service', function() {
fillParentsInDocs: sinon.stub()
}
};
collectUuidsByPhones = sinon.stub().resolves([]);
const datasource = { v1: { contact: { collectUuidsByPhones } } };
DataContext = Promise.resolve({ getDatasource: () => datasource });


module('adminApp');
Expand All @@ -43,6 +48,7 @@ describe('MessageQueue service', function() {
$provide.value('Settings', Settings);
$provide.value('Languages', Languages);
$provide.value('MessageQueueUtils', utils);
$provide.value('DataContext', DataContext);
$provide.factory('DB', KarmaUtils.mockDB({ query: query }));
});

Expand Down Expand Up @@ -285,9 +291,7 @@ describe('MessageQueue service', function() {
.withArgs('medic-admin/message_queue', sinon.match({ reduce: false }))
.resolves({ rows: messages });

query.withArgs('medic-client/contacts_by_phone').resolves({
rows: [{ id: 'contact1', value: 'contact1', key: 'phone1' }]
});
collectUuidsByPhones.withArgs(['phone1', 'phone2']).resolves(['contact1']);

query.withArgs('medic/doc_summaries_by_id').resolves({
rows: [{ id: 'contact1', value: { name: 'James', phone: 'phone1' } }]
Expand All @@ -303,14 +307,11 @@ describe('MessageQueue service', function() {

return service.query('due').then(result => {
chai.expect(result.total).to.equal(2);
chai.expect(query.callCount).to.equal(4);
chai.expect(query.callCount).to.equal(3);

chai.expect(query.args[2]).to.deep.equal([
'medic-client/contacts_by_phone',
{ keys: ['phone1', 'phone2'] }
]);
chai.expect(collectUuidsByPhones.calledOnceWithExactly(['phone1', 'phone2'])).to.be.true;

chai.expect(query.args[3]).to.deep.equal([
chai.expect(query.args[2]).to.deep.equal([
'medic/doc_summaries_by_id',
{ keys: ['contact1'] }
]);
Expand Down Expand Up @@ -408,9 +409,7 @@ describe('MessageQueue service', function() {
.withArgs('medic-admin/message_queue', sinon.match({ reduce: false }))
.resolves({ rows: messages });

query
.withArgs('medic-client/contacts_by_phone')
.resolves({ rows: [ { value: 'contact1', key: 'phone1' }, { value: 'contact2', key: 'phone2' } ] });
collectUuidsByPhones.withArgs(['phone1', 'phone2']).resolves(['contact1', 'contact2']);

query
.withArgs('medic/doc_summaries_by_id')
Expand All @@ -422,10 +421,8 @@ describe('MessageQueue service', function() {
});

return service.query('due').then(result => {
chai.expect(query.callCount).to.equal(4);
chai.expect(query.args[2]).to.deep.equal([
'medic-client/contacts_by_phone', { keys: [ 'phone1', 'phone2' ]}
]);
chai.expect(query.callCount).to.equal(3);
chai.expect(collectUuidsByPhones.calledOnceWithExactly(['phone1', 'phone2'])).to.be.true;

chai.expect(result.messages[0].recipient).to.equal('contact one');
chai.expect(result.messages[1].recipient).to.equal('contact one');
Expand Down Expand Up @@ -587,16 +584,14 @@ describe('MessageQueue service', function() {
to: recipient
}]));

query
.withArgs('medic-client/contacts_by_phone')
.resolves({ rows: [{ key: 'recipient_id', value: 'recipient' }]});
collectUuidsByPhones.withArgs(['recipient']).resolves(['recipient_id']);
query
.withArgs('medic/doc_summaries_by_id')
.resolves({ rows: [{ key: 'recipient_id', value: { phone: 'recipient' }}]});

return service.query('tab').then(result => {
chai.expect(result.messages.length).to.equal(15);
chai.expect(query.callCount).to.equal(6);
chai.expect(query.callCount).to.equal(5);
chai.expect(query.args[2]).to.deep.equal([
'medic-client/contacts_by_reference',
{
Expand All @@ -620,10 +615,7 @@ describe('MessageQueue service', function() {
[ 'patient1', 'patient2', 'patient3', 'place1', 'place2', 'place3' ],
]);

chai.expect(query.args[4]).to.deep.equal([
'medic-client/contacts_by_phone',
{ keys: [ 'recipient' ]}
]);
chai.expect(collectUuidsByPhones.calledOnceWithExactly(['recipient'])).to.be.true;
});
});

Expand Down Expand Up @@ -754,13 +746,9 @@ describe('MessageQueue service', function() {
.withArgs(sinon.match({ type: 'valid' })).returns(true)
.withArgs(sinon.match({ type: 'invalid' })).returns(false);

query
.withArgs('medic-client/contacts_by_phone')
.resolves({ rows: [
{ key: 'recipient1', id: 'recipient1_id' },
{ key: 'recipient2', id: 'recipient2_id' },
{ key: 'recipient3', id: 'recipient3_id' }
]});
collectUuidsByPhones
.withArgs(['recipient1', 'recipient2', 'recipient3'])
.resolves(['recipient1_id', 'recipient2_id', 'recipient3_id']);
query
.withArgs('medic/doc_summaries_by_id')
.resolves({ rows: [
Expand Down Expand Up @@ -931,10 +919,9 @@ describe('MessageQueue service', function() {
}
]);

chai.expect(query.withArgs('medic-client/contacts_by_phone').callCount).to.equal(1);
chai.expect(query.withArgs('medic-client/contacts_by_phone').args[0][1]).to.deep.equal({
keys: ['recipient1', 'recipient2', 'recipient3']
});
chai.expect(collectUuidsByPhones.calledOnceWithExactly([
'recipient1', 'recipient2', 'recipient3'
])).to.be.true;

chai.expect(result.messages).to.deep.equal([
{
Expand Down Expand Up @@ -1076,9 +1063,7 @@ describe('MessageQueue service', function() {
.withArgs(sinon.match({ type: 'valid' })).returns(true)
.withArgs(sinon.match({ type: 'invalid' })).returns(false);

query
.withArgs('medic-client/contacts_by_phone')
.resolves({ rows: [{ key: 'recipient1', id: 'recipien_id' }]});
collectUuidsByPhones.withArgs(['recipient1']).resolves(['recipient_id']);
query
.withArgs('medic/doc_summaries_by_id')
.resolves({ rows: [{ key: 'recipient_id', value: { phone: 'recipient1', name: 'recipient' }}]});
Expand Down
140 changes: 134 additions & 6 deletions api/src/controllers/contact.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@
* operationId: v1ContactUuidGet
* description: >
* Returns a paginated array of contact identifier strings matching the given filter criteria.
* At least one of `type` or `freetext` must be provided.
* At least one qualifier param (`type`, `freetext`, or `phone`) must be provided. Qualifier
* params are mutually exclusive — the only combination allowed is `type` + `freetext`
* (backed by a dedicated view). Any other combination returns 400.
* tags: [Contact]
* x-since: 4.18.0
* x-permissions:
Expand All @@ -82,16 +84,22 @@
* schema:
* type: string
* description: >
* The contact_type id for the type of contacts to fetch. Required if `freetext` is not provided
* and may be combined with `freetext`.
* The contact_type id for the type of contacts to fetch. May be combined with `freetext`.
* - in: query
* name: freetext
* schema:
* type: string
* minLength: 3
* description: >
* A search term for filtering contacts. Must be at least 3 characters and not contain whitespace.
* Required if `type` is not provided and may be combined with `type`.
* May be combined with `type`.
* - in: query
* name: phone
* schema:
* type: string
* description: >
* A phone number to match exactly against the contact's `phone` field. Passed as-is — no
* normalization is performed. Mutually exclusive with `type` / `freetext`.
* - $ref: '#/components/parameters/cursor'
* - $ref: '#/components/parameters/limitId'
* responses:
Expand All @@ -117,12 +125,39 @@
* '403':
* $ref: '#/components/responses/Forbidden'
*/
getUuids: serverUtils.doOrError(async (req, res) => {

Check failure on line 128 in api/src/controllers/contact.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 6 to the 5 allowed.

See more on https://sonarcloud.io/project/issues?id=medic_cht-core&issues=AZ5LcYuH68hX41I6qCxY&open=AZ5LcYuH68hX41I6qCxY&pullRequest=11101
await auth.assertPermissions(req, { isOnline: true, hasAll: ['can_view_contacts'] });
if (!req.query.freetext && !req.query.type) {
return serverUtils.error({ status: 400, message: 'Either query param freetext or type is required' }, req, res);

// Qualifier params are mutually exclusive by default. `type` and `freetext` are the only
// pair that may be combined (backed by the contacts_by_type_freetext view). When a new
// qualifier is added, extend QUALIFIER_PARAMS — exclusivity is automatic.
const QUALIFIER_PARAMS = ['type', 'freetext', 'phone'];
const COMBINABLE = ['type', 'freetext'];
const present = QUALIFIER_PARAMS.filter(name => req.query[name]);

if (!present.length) {
return serverUtils.error(
{ status: 400, message: `At least one of query params ${QUALIFIER_PARAMS.join(', ')} is required` },
req,
res
);
}
if (present.length > 1 && !present.every(name => COMBINABLE.includes(name))) {
return serverUtils.error(
{
status: 400,
message: `Query params ${present.join(', ')} are mutually exclusive `
+ `(only ${COMBINABLE.join(' and ')} may be combined)`,
},
req,
res
);
}

const qualifier = {};
if (req.query.phone) {
Object.assign(qualifier, Qualifier.byPhone(req.query.phone));
}
if (req.query.freetext) {
Object.assign(qualifier, Qualifier.byFreetext(req.query.freetext));
}
Expand All @@ -132,5 +167,98 @@
const docs = await getContactIds(qualifier, req.query.cursor, req.query.limit);
return res.json(docs);
}),

/**
* @openapi
* /api/v1/contact/uuid:
* post:
* summary: Get contact UUIDs (bulk variant)
* operationId: v1ContactUuidPost
* description: >
* Bulk variant of the GET endpoint. Accepts array-valued qualifiers in a JSON body — used
* when the array would not fit safely in a query string. The only multi-value qualifier
* currently accepted is `phones`. Returns the same paginated `{ data, cursor }` shape as
* the GET endpoint.
* tags: [Contact]
* x-since: 4.21.0
* x-permissions:
* hasAll: [can_view_contacts]
* requestBody:
* required: true
* content:
* application/json:
* schema:
* type: object
* properties:
* phones:
* type: array
* items:
* type: string
* description: >
* Phone numbers to match exactly against the contact's `phone` field. Passed
* as-is — no normalization. One CouchDB round trip regardless of array size.
* cursor:
* type: string
* nullable: true
* limit:
* type: integer
* required: [phones]
* responses:
* '200':
* description: A page of contact UUIDs
* content:
* application/json:
* schema:
* type: object
* properties:
* data:
* type: array
* items:
* type: string
* cursor:
* $ref: '#/components/schemas/PageCursor'
* required: [data, cursor]
* '400':
* $ref: '#/components/responses/BadRequest'
* '401':
* $ref: '#/components/responses/Unauthorized'
* '403':
* $ref: '#/components/responses/Forbidden'
*/
postUuids: serverUtils.doOrError(async (req, res) => {
await auth.assertPermissions(req, { isOnline: true, hasAll: ['can_view_contacts'] });

// POST body mirrors the GET query-param shape, with array values where the qualifier accepts
// many. Mutual-exclusivity rules match the GET path: list the multi-value params here and
// mirror COMBINABLE.
const QUALIFIER_PARAMS = ['phones'];
const COMBINABLE = [];

Check warning on line 235 in api/src/controllers/contact.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

`COMBINABLE` should be a `Set`, and use `COMBINABLE.has()` to check existence or non-existence.

See more on https://sonarcloud.io/project/issues?id=medic_cht-core&issues=AZ5eegymVJBT9wqP8JR0&open=AZ5eegymVJBT9wqP8JR0&pullRequest=11101
const present = QUALIFIER_PARAMS.filter(name => req.body && req.body[name] !== undefined);

Check warning on line 236 in api/src/controllers/contact.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer using an optional chain expression instead, as it's more concise and easier to read.

See more on https://sonarcloud.io/project/issues?id=medic_cht-core&issues=AZ5eegymVJBT9wqP8JR1&open=AZ5eegymVJBT9wqP8JR1&pullRequest=11101

if (!present.length) {
return serverUtils.error(
{ status: 400, message: `At least one of body params ${QUALIFIER_PARAMS.join(', ')} is required` },
req,
res
);
}
if (present.length > 1 && !present.every(name => COMBINABLE.includes(name))) {

Check warning on line 245 in api/src/controllers/contact.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Review this usage of "COMBINABLE" as it can only be empty here.

See more on https://sonarcloud.io/project/issues?id=medic_cht-core&issues=AZ5eegymVJBT9wqP8JR2&open=AZ5eegymVJBT9wqP8JR2&pullRequest=11101
return serverUtils.error(
{
status: 400,
message: `Body params ${present.join(', ')} are mutually exclusive`,
},
req,
res
);
}

const qualifier = {};
if (req.body.phones) {
Object.assign(qualifier, Qualifier.byPhones(req.body.phones));
}
const docs = await getContactIds(qualifier, req.body.cursor, req.body.limit);
return res.json(docs);
}),
},
};
1 change: 1 addition & 0 deletions api/src/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ app.postJson('/api/v1/person', person.v1.create);
app.putJson('/api/v1/person/:uuid', person.v1.update);

app.get('/api/v1/contact/uuid', contact.v1.getUuids);
app.postJson('/api/v1/contact/uuid', contact.v1.postUuids);
app.get('/api/v1/contact/:uuid', contact.v1.get);

app.get('/api/v1/report/uuid', report.v1.getUuids);
Expand Down
Loading
Loading