feat(#10973): add phone qualifier

[witash]

Description

Adds a phones qualifier to cht-datasource and exposes it on the two contact list endpoints.
No call site is migrated — every existing medic-client/contacts_by_phone caller is
untouched, so no existing read moves onto the new path. One thing does change for existing API
users: the error returned when neither endpoint gets a qualifier. Either query param freetext or type is required becomes Either query param freetext, type or phone is required, on both
GET /api/v1/contact/uuid and GET /api/v1/contact.

Datasource

• PhonesQualifier / byPhones / isPhonesQualifier in qualifier.ts, reaching consumers through the existing export * as Qualifier re-export.
• Local: a dispatch arm in getUuidsPage and, via getContactDocsPageFn, in getPage. Both query the view with keys and both filter rows through isContact, so the id path and the doc path return the same contacts. Keys are
  deduped in a stable order — rows come back grouped in supplied-key order, which is what keeps the skip cursor meaningful between pages.
• queryDocsByKeys in local/libs/doc.ts, the docs twin of the queryDocIdsByKeys helper feat(#11151): add byForm qualifier to Report.v1.getUuidsPage #11327 added.
• Remote: a phone param on both list endpoints.
• Both assert helpers were renamed (assertContactTypeFreetextPhonesQualifier, assertContactTypeIdsPhonesQualifier) rather than duplicated, so no call site can still reach a two-qualifier assertion.
• getUuidsPageByPhones / getUuidsByPhones / getPageByPhones / getByPhones on the imperative getDatasource() surface, next to getPageByType — no new method on the Contact.v1 namespace. These have no in-repo consumer yet.

API

phone query param on GET /api/v1/contact/uuid and GET /api/v1/contact; no new route and no routing.js change. On /uuid it takes precedence over type and freetext; on /contact the precedence is ids, then phone, then
type. An object-shaped phone (?phone[a]=b, which qs.parse parses to an object) is rejected with a 400 rather than throwing.

Why the phones paths filter and the type path does not

contacts_by_type keys on the contact type and assertValidContactType gates the request, so that path can only ever emit configured types — it needs no filter. contacts_by_phone keys on the phone and emits any doc whose type is
in its hard-coded list, including type: contact docs whose contact_type is not in settings. So the phones arms filter explicitly, which costs the id path an include_docs it would not otherwise need. Emitting the type as the view
value would let the filter run on row.value instead, but that is a ddoc change and a reindex — worth revisiting if the phones uuid path ever gets a high-volume caller.

Without the filter the id path would hand back ids that GET /api/v1/contact/{id} then 404s, because get applies isContact too.

Decisions copied from #11151 (#11327)

• Plural array qualifier with a non-empty tuple type, verbatim matching, Set dedupe, and rejection of blank or whitespace-padded entries.
• Serialising over the existing list endpoint as a comma-joined query param instead of a dedicated route.
• The keys-based local query helper.

Deviations from the issue

• Plural, not singular. byPhones([...]) rather than byPhone(phone). The view is queried with keys, so a batch of numbers costs one query rather than one per number — which is what the admin message queue will need when it
  migrates.
• No /api/v1/contact/by-phone/:phone. Query param on the existing endpoints, so paging, permissions and the OpenAPI shape stay on the established contract.
• The doc-returning path is extended too. The issue scoped this to getUuidsPage; extending getPage keeps a doc lookup at one round trip and lets both paths agree on contact types.
• The call sites are not migrated. Review found a behaviour change in every one of them (below), so they move to a new PR and a new issue (Migrate the medic-client/contacts_by_phone call sites onto the phones qualifier #11442) and this PR ships the qualifier alone.
• Padded input is rejected rather than passed through. The qualifier normalises nothing, but a padded number can only ever match nothing, so byPhones throws instead of returning a silent empty page. Coercion and trimming remain
  the caller's job, as the issue requires.
• The shared assert helper is renamed rather than duplicated. feat(#11151): add byForm qualifier to Report.v1.getUuidsPage #11327 went the other way — it added assertFreetextOrFormsQualifier alongside assertFreetextQualifier and kept both. Renaming leaves no two-qualifier assertion for a
  call site to reach by mistake.

Why the call sites moved out

The acceptance criteria of #10973 was originally to migrate the existing view calls onto the datasource, but every one of those migrations carried a behaviour change that needs more analysis than this PR should carry. So this PR adds the
qualifier, and migrating the call sites onto it moves to a follow-up. See #11442

Call site	Behaviour change migration would cause
webapp/.../phone-widget.js	Online users would newly need can_view_contacts. That permission gates the UI and the /api/v1/contact|person|place endpoints, not the data — online users read contacts through the CouchDB proxy regardless — so the change restricts nothing while potentially breaking a custom role.
admin/.../message-queue.js	Same cause, larger blast radius: crfo reaches the admin app via can_configure/can_export_all with no can_view_contacts, and the rejection blanks the page rather than dropping names.
validation_utils.js uniquePhone	A missing field would go from failing to passing, so a config using uniquePhone as "must exist AND be unique" would silently stop enforcing the first half. String()/.trim() would also newly enforce uniqueness on numeric and padded values.
registration.js getParentByPhone	An undefined doc.from sends no key at all — PouchDB drops undefined params — so the query returns every phone-bearing contact and rows[0].doc becomes the parent. Migrating would make that parent_not_found. Rows would also be newly filtered by isContact.
update_sent_by.js	Rows would be newly filtered by isContact, where it currently reads rows[0].doc off the raw view.
api/.../contacts-by-phone.js	Rows would be newly filtered by isContact, where it currently hydrates the raw view ids.
self_report.js, update_clinics.js	These two already filter at hydration via getWithLineage, so isContact costs them nothing. They would still gain .trim() from the shared sender qualifier: a padded doc.from matches nothing today and would start matching.

Testing

Unit coverage for the qualifier, both local adapter arms (including the isContact filter on the id path), the remote adapter, the assert helpers and the controller; API integration tests for ?phone=, comma-separated, repeated-param
and the two 400 cases on both endpoints; datasource integration tests for getUuidsPage/getPage/the generator, non-normalisation, a three-page cursor walk asserting exact ids and cursors, and a type: contact doc with an
unconfigured contact_type that the view emits and neither phones path returns. eslint ., unit-api, unit-shared-lib green.

AI Disclosure: This PR was written with assistance from Claude Code.

closes #10973

Code review checklist

• UI/UX backwards compatible: Test it works for the new design (enabled by default). And test it works in the old design, enable can_view_old_navigation permission to see the old design. Test it has appropriate design for RTL languages.
• Readable: Concise, well named, follows the style guide
• Documented: Configuration and user documentation on cht-docs
• Tested: Unit and/or e2e where appropriate
• Internationalised: All user facing text
• Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.
• AI disclosure: Please disclose use of AI per the guidelines.

Compose URLs

If Build CI hasn't passed, these may 404:

• Core
• CouchDB Single
• CouchDB Cluster

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

[github-actions]

This PR is now marked "stale" after 30 days without activity. It will be closed automatically in 10 days unless you add a comment, push new changes or remove the "stale" label.

[sugat009]

Thank you, this is a clean pilot for the series. I read the diff, ran the unit suites, and tested the endpoints on a local instance with contacts that share a phone number. The cursor walk, dedupe, verbatim matching and the 400 cases all work as described.

I found no code defect. The inline comments hold the details:

• Three statements in the description do not match the code or the tests (cursor test, /contact tests, "rows consumed per site are unchanged").
• Three questions on intent (phone precedence, the id path vs the doc path, the new can_view_contacts requirement).
• Two small suggestions with committable code.

One more item: all seven checklist boxes are unchecked. Please complete the checklist, and say whether AI was used.

[sugat009]

Thank you. The reverts are correct, and both front-end files now match master. You applied the five suggestions, and the unit tests pass (cht-datasource 1267, transitions 893, validation 49, API contact controllers 35). The revert removes the permission problem.

Three items remain. No item is a code defect.

1. The PR does not obey the acceptance criterion of #10973.

The revert put message-queue.js and phone-widget.js back on the contacts_by_phone view. The search in the issue now finds two matches, not zero. The revert is correct, because of the permission problem we discussed. But the PR still says closes #10973. Please change the criterion and record the reason, or open a follow-up issue for the two sites. The issue owner can decide.

2. Please make the description agree with the code.

Four statements are now wrong:

• Callers row for message-queue.js: "one getUuidsByPhones call for all recipients". The code now uses the view.
• Callers row for phone-widget.js: "window.CHTCore.CHTDatasource (newly exposed via integration-api)". The code now uses the view, and you reverted the integration-api and cht-form changes.
• Deviations item: "Eight call sites, not six". The code now moves six sites.
• Callers note: "Rows consumed per site are unchanged". The two front-end sites no longer drain a generator. They read the rows from one view query.

Three more points:

• The from change is a breaking change. A report with no sender no longer matches every contact, and uniquePhone does the same. You agreed to add a release note. Please replace the "Rows consumed per site are unchanged" sentence, add the release note, and mention the new .trim().
• Please add one sentence for the reason. The two sites stay on the view, because the datasource path adds the can_view_contacts check and the isContact filter. Also say the four *Phones methods stay for the later work.
• The Testing section says the cursor test "straddles a key boundary". The test does not. Please change the sentence.

3. The checklist is not complete.

All seven boxes are empty, and there is no AI disclosure. Please complete the checklist and say if you used AI.

[sugat009]

Thank you. The revert is complete and clean. This round reverted 15 files, and each one is now the same as master. All eight call sites use the view again. The PR now changes 16 files. The revert left no orphan code. The unit tests pass: cht-datasource 1267, transitions 882, validation 132, and 35 across the two API contact controller specs. eslint is clean. You also closed every code item from the last two rounds: the object-shaped phone guard, the OpenAPI sentence, the JSDoc order note, the cursor walk test, and the three /contact tests. The new description is much more honest, and the AI disclosure is there.

One item needs an answer before this merges. Five smaller items are about the description and the body.

1. The id path does not filter, but the PR says it does. See the comment on shared-libs/cht-datasource/src/local/contact.ts:163. It holds the commands to reproduce this.

2. "No behaviour change" is not exact. Line 26 says the PR carries no behaviour change. Two error messages changed for existing API users. See the comment on api/src/controllers/contact.js:170.

3. Two rows in the behaviour table are wrong.

• Line 76 says contacts-by-phone.js was "Newly filtered by isContact". The reverted code used Contact.v1.getUuids, which is the unfiltered id path. That change never happened.
• Line 78 says self_report.js and update_clinics.js are the only two with unchanged behaviour. Both gained .trim() through senderPhoneQualifier. A padded doc.from matched nothing before, and it matches now. Line 78 also contradicts line 62.

4. The source in line 53 is backwards. Line 53 lists "Renaming the shared assert helper instead of adding a parallel one" as a decision copied from #11327. #11327 did the opposite. It added assertFreetextOrFormsQualifier next to assertFreetextQualifier, and both still exist. This PR renamed. The rename is complete, so only the source needs a change.

5. The body holds the template twice. There are two checklists (lines 90 to 98 empty, lines 110 to 118 complete), two License sections, two <!-- ISSUE NUMBER --> markers, and two compose blocks. The empty checklist shows first, so the page still looks incomplete. Please delete the first copy. Line 67 also has a typo: "accpetance".

6. closes #10973 no longer fits. The body says no call site is migrated. It also explains why the acceptance criteria are not met. The search in the issue now finds eight matches. Please change closes to refs, and let the follow-up PR close the issue.

[sugat009]

@witash you pushed 0387ccd6 after my last review. That commit looks like it answers my question about the isContact filter on the id path. Is the PR ready for another review? Please tell me when you want me to look again.

[github-actions]

All checks passed — thank you @witash! This PR is ready for review.

[witash]

@witash you pushed 0387ccd after my last review. That commit looks like it answers my question about the isContact filter on the id path. Is the PR ready for another review? Please tell me when you want me to look again.

Hi @sugat009 after changes are requested on a PR, I usually don't consider it ready for re-review until 're-requesting' a review, even if commits have been pushed in the meantime. Then you can push intermediate commits when they are ready, re-run ci, or leave a PR alone for a while and come back to it later, without needing it to be ready for review then whole time.

Anyway, its ready for review now; I pushed the missing isContact commit, edited the issue so that the accpetance criteria do not include migrate all the callers, and created the new issue for migrating the callers. #11442 , so its not ready for review again

[sugat009]

Approved. The blocker from my last review is fixed. I proved it on a local instance.

Commit 0387ccd6 changes the phones arm of getUuidsPage. It now reads the documents and filters them through isContact. Both paths return the same contacts.

I checked the fix three ways.

• The document that failed before now returns nothing on both endpoints. Before, the id path returned an id, and that id gave 404.
• I paged nine documents on one number. Contacts and non-contacts alternate by id. At limits 1, 2 and 3, both endpoints returned the same five contacts. No gaps. No repeats.
• The new unit tests fail against the old code. Six tests fail when I restore the round-3 file.

Everything else from the last three rounds is closed.

• All five description items are fixed. You rewrote the description rather than patch it.
• The body holds one checklist. All seven boxes are ticked. The AI disclosure is there.
• You rescoped issue #10973. All five new criteria are met, so closes #10973 is correct now.
• You raised #11442 for the call sites. It names all eight sites and the decision each one needs.
• All 17 review threads are resolved.
• CI passes: 53 checks, no failures.
• Tests pass: cht-datasource 1268, API contact controller 25, transitions 885, validation 132. eslint and tsc are clean.

Four notes follow. None of them blocks the merge.

1. A shared helper can drop a contact. See the comment on shared-libs/cht-datasource/src/local/contact.ts:169. This fault is not yours. Your fix routes a second path onto it.
2. x-since names a released version. See the comment on api/src/controllers/contact.js:135.
3. A comma inside a phone number splits it. See the comment on shared-libs/cht-datasource/src/remote/contact.ts:54.
4. Three small items. The four new getDatasource() methods omit the paging order warning that byPhones carries. No test covers ids against phone on /api/v1/contact. Line 27 of the body quotes the /uuid error message, but /contact had a different one, Either query param ids or type is required.

[sugat009]

note: a pre-existing fault in fetchAndFilter reaches these lines

The fix on these lines is right. I confirmed it on a local instance. Both paths now return the same contacts.

While I tested it, I found a fault in fetchAndFilter. It can lose a contact and still report the end of the results.

Put five non-contacts in front of three contacts on one number. Then ask for pages of two.

GET /api/v1/contact/uuid?phone=%2B955&limit=2
{"data":["b1","b2"],"cursor":null}

The third contact never arrives. The cursor is null, so the caller believes the list is complete. Limits 1, 3, 4 and 5 all return the three contacts. Only limit 2 loses one. The doc endpoint loses the same row.

This fault is not yours. Three checks say so.

• The PR does not touch fetchAndFilter. The only addition to local/libs/doc.ts is queryDocsByKeys.
• GET /api/v1/contact?ids=<five non-contacts>,<three contacts>&limit=2 loses a contact too. This PR never touches that path.
• The logic is unchanged since 2024, in feat(#9193): add functionality of getting places as pages or async iterables in cht-datasource #9368.

The doc path already had this fault. Your fix gives the uuid path the same helper, so both paths lose the same row. That is the parity you wanted.

The cause sits in the cursor arithmetic of fetchAndFilter. #11449 tracks it, because the fix belongs in that helper and it also reaches the ids qualifier and offline freetext search. Nothing is needed from you here.

One smaller point on the same lines. The old arm used fetchAndFilterIds, which removed duplicate ids with a Set. fetchAndFilter does not. phoneViewKeys removes duplicate keys, but CouchDB compares keys by collation, not by characters. Five ignorable characters make two different strings match one row, for example a zero width space or a left to right mark. The id then arrives twice. getPage already behaved this way, so your change makes the two paths agree. The trigger is narrow. I raise it for the record only.

[sugat009]

nit: x-since names a version that already shipped

This line says x-since: 5.3.0. The repository holds tags 5.3.0 and 5.3.1, and master reads 5.3.0 in package.json. So the phone parameter cannot appear in 5.3.0.

The phone parameter on /api/v1/contact has no x-since of its own (line 209). It therefore inherits 5.3.0 from the operation on line 190, which is wrong for the same reason.

Please give both parameters the next release number.

[sugat009]

question: should byPhones reject a comma?

This line joins the numbers with a comma. api/src/controllers/contact.js:26 then splits the value on a comma. So one number that holds a comma arrives at the server as two numbers.

The local adapter matches that same number as one whole string. One qualifier therefore gives two different answers, and the remote call can return contacts that the caller never asked for.

byIds joins the same way on line 80. Ids are generated, so they hold no commas. A phone field is free text that a user types.

A repeated phone parameter would close the gap. A check in byPhones would close it too. Which do you prefer?