Add byForm qualifier to Report.v1 (replaces medic-client/reports_by_form page query)

title: Add byForm qualifier to Report.v1 (replaces medic-client/reports_by_form page query)
type: improvement
priority: medium
domain: data-sync
labels:

cht-datasource
migration

Description

Replace db.query('medic-client/reports_by_form', ...) page-query call sites in cht-core with a typed byForm(formCode) qualifier on Report.v1.getUuidsPage.

Scope is the page query only. Some callers also use this view with group: true for counts, or key: [form], limit: 1 for sampling — those aggregate variants are out of scope and require a different shaped operation (e.g. Report.v1.countByForm). They stay on db.query until a separate ticket addresses the aggregate API surface.

Part of the cht-datasource view→qualifier migration series; pattern established in #10973.

Technical Context

Components:

shared-libs/cht-datasource/src/qualifier.ts — add FormQualifier
shared-libs/cht-datasource/src/report.ts — extend getUuidsPage's declarative API
shared-libs/cht-datasource/src/local/report.ts — add dispatch arm
shared-libs/cht-datasource/src/remote/report.ts — add dispatch arm
api/src/controllers/report.js — new route GET /api/v1/report/by-form/:formCode
api/src/routing.js — wire the route

Existing References:

shared-libs/cht-datasource/src/local/report.ts — pattern for the getUuidsPage dispatch arms (already has isFreetextQualifier arm)
PR #10083 — added the create/update qualifiers + Local/Remote dispatch. Same pattern as this ticket.

Requirements

Adapter Additions

FormQualifier type:

export type FormQualifier = Readonly<{ form: string }>;
export const byForm = (form: string): FormQualifier => {
  if (!isString(form) || form.length === 0) {
    throw new InvalidArgumentError(`Invalid form [${JSON.stringify(form)}].`);
  }
  return { form };
};
export const isFormQualifier = (q: unknown): q is FormQualifier =>
  isRecord(q) && hasField(q, { name: 'form', type: 'string' });

The form code is the doc's form field (e.g. pregnancy, anc_followup). Do not normalise.

Extend Report.v1.getUuidsPage JSDoc for new qualifier shape.
Add dispatch arm to local/report.ts:
- When isFormQualifier(qualifier), query medic-client/reports_by_form with key: [qualifier.form]
Add dispatch arm to remote/report.ts calling the new endpoint.
Add API endpoint and wire it.
Expose byForm from cht-datasource's index.ts under Qualifier.

Caller Sweeps

Audit grep -rnE "query\(['\"]medic-client/reports_by_form['\"]" webapp/src admin/src shared-libs before drafting. Out-of-scope callers — those passing group: true or reduce: true or using single-result sampling — stay on db.query and are preserved with a comment referencing the follow-up aggregate-API ticket. Only the page-query callers swap to byForm.

Tests

Unit: test/qualifier.spec.ts — byForm accepts non-empty strings; rejects empty + non-string
Unit: test/local/report.spec.ts — dispatch arm hits the right view + key
Unit: test/remote/report.spec.ts — endpoint called with correct path
Caller-side: each modified caller keeps existing tests green
Pagination: walk two cursor pages with limit: 5
Aggregate-preservation: confirm aggregate callers (if any) are NOT touched by this ticket — grep -rnE "query\(['\"]medic-client/reports_by_form['\"][^)]*reduce" webapp/src admin/src shared-libs should return the same matches before and after

Acceptance Criteria

byForm qualifier in qualifier.ts with unit tests passing
Local and Remote adapters dispatch new qualifier with unit tests passing
GET /api/v1/report/by-form/:formCode endpoint returns { data, cursor }
Page-query callers swapped; aggregate-query callers explicitly preserved with a comment
npm run unit-shared-lib and npm run unit-api pass

Constraints

Backwards-compatible: existing calls continue unchanged
No new top-level Report.v1 method
Do not relax Input.v1.*Input._id?: never
Do not normalise the form code
Aggregate variants (group: true, reduce: true, limit: 1 sample) are out of scope. The qualifier-extension pattern doesn't fit count/sample operations — those need narrower operations like Report.v1.countByForm. File a follow-up ticket for the aggregate API surface; do not fold it into this ticket.

References

PR feat(#9835): add cht datasource apis for creation and update of contacts and reports #10083 — qualifier + dispatch pattern
PR feat(9586): implement freetext search in cht datasource #9625 — pagination discipline
Pilot: Add byPhones qualifier to Contact.v1 #10973

Picking this up — PR #11327 is open against master.

One thing worth surfacing before review: the caller sweep this ticket asks for turned up no in-scope page-query callers. All three callers of medic-client/reports_by_form are aggregate or single-result-sample queries, which the ticket puts out of scope, so each is preserved with a comment pointing at the aggregate API follow-up rather than swapped. The qualifier, adapters and endpoint are all still added as specified.

I hit a few problems in the ticket text itself while working through it, and they look like they come from the Datasource View Migration template rather than this ticket specifically — details in a follow-up comment so this one stays short.

As promised, the ticket-text problems I hit. All three appear to come from the template shared by the Datasource View Migration series, so they likely affect #11147, #11148, #11149, #11150 and #11152 as well — flagging here once rather than six times.

1. The audit grep searches the wrong directories, and silently finds nothing

The ticket says:

Audit grep -rnE "query\(['\"]medic-client/reports_by_form['\"]" webapp/src admin/src shared-libs before drafting.

Run verbatim on master, that returns zero matches. Every actual caller of this view lives in api/src, which isn't in the search path:

$ grep -rnE "query\(['\"]medic-client/reports_by_form['\"]" webapp/src admin/src shared-libs
(no output)

$ grep -rnE "query\(['\"]medic-client/reports_by_form['\"]" webapp/src admin/src shared-libs api/src
api/src/services/impact.js:6
api/src/services/export/report-mapper.js:59
api/src/services/export/report-mapper.js:72

This is the dangerous failure mode: it doesn't error, it reports "clean". Anyone following the ticket literally concludes there are no callers and skips the preservation requirement entirely. The same three-directory list appears in #11147, #11150 and #11152.

Suggest adding api/src (and probably sentinel/src) to the audit path across the series.

2. The aggregate-preservation check can never fire

The ticket asks for this before/after check:

grep -rnE "query\(['\"]medic-client/reports_by_form['\"][^)]*reduce" webapp/src admin/src shared-libs should return the same matches before and after

It returns zero matches before and after — partly because of the directory problem above, and partly because [^)]*reduce can't span lines while every one of these call sites puts reduce/group on a following line:

const result = await db.medic.query('medic-client/reports_by_form', {
  reduce: true,
  group: true,
});

It passes trivially while verifying nothing.

3. The `FormQualifier` snippet has a validation bug

The ticket supplies this:

export const byForm = (form: string): FormQualifier => {
  if (!isString(form) || form.length === 0) { throw ... }
  return { form };
};
export const isFormQualifier = (q: unknown): q is FormQualifier =>
  isRecord(q) && hasField(q, { name: 'form', type: 'string' });

The builder and the type guard disagree. hasField only checks typeof === 'string', so isFormQualifier({ form: '' }) is true while byForm('') throws — and it's the guard that getUuidsPage's assert runs, not the builder. Net effect as specified: GET /api/v1/report/by-form/%20 returns 200 {"data":[],"cursor":null} instead of 400.

The codebase already has the right helper — hasStringFieldWithValue (hasField plus a !value.trim() check), used for exactly this on doc.form in local/report.ts. PR #11327 uses that in both, so builder and guard can't drift. Rejecting blank values is validation, not normalisation, so it doesn't conflict with the "do not normalise" constraint — ' ANC_FollowUp ' is still preserved verbatim.

The same isString(...) || ....length === 0 + hasField pairing appears in #11150 and #11152, so the same gap is likely templated into those.

Two smaller notes

A fourth reference the grep can't find. shared-libs/search/src/generate-search-requests.js:66 uses this view via getRequestForMultidropdown('medic-client/reports_by_form', ...) rather than calling db.query directly, so no query(...)-shaped grep will surface it. It builds a request descriptor over many form keys for the generic search executor to combine, so it can't use a single-form qualifier. Left untouched — worth deciding whether it belongs in the migration series.

Endpoint shape. The ticket specifies GET /api/v1/report/by-form/:formCode, which is what I implemented. Worth noting it diverges from the established pattern for this operation, where freetext is a query param on /api/v1/report/uuid. A ?form= param on the existing route would be more consistent with the rest of getUuidsPage; happy to switch if you'd prefer that.

The aggregate follow-up this ticket asks for is filed as #11328.

Could someone assign this issue to me? AndraBot on #11327 is asking for it:

You are not assigned to the linked issue (#11151). Please comment on the issue asking to be assigned to it, so we know the work is not being duplicated.

Happy to confirm I'm not duplicating anyone — the issue was unassigned when I picked it up. Thanks!

test(medic#11151): use dedicated null matcher for cursor assertions

@sugat009

@sugat009 — sorry to pull you in on an unrelated ticket, but you were able to assign #11324 and this one has the same problem.

PR #11327 is open against this issue and AndraBot is holding it at Waiting for contributor because I'm not assigned here. It's the assignment check rather than anything I can fix from my side. Could you assign me, or point me at whoever triages the Datasource View Migration series?

Worth flagging while you're here, since it bears on that series rather than just this ticket: the caller sweep this issue prescribes finds nothing. The grep it specifies searches webapp/src admin/src shared-libs, and every caller of medic-client/reports_by_form lives in api/src, so it returns zero matches and reads as "clean" rather than erroring. All three real callers turned out to be aggregate or single-result-sample queries that this ticket puts out of scope anyway, so the PR preserves them with comments instead of swapping them — but anyone following the ticket literally would conclude there were no callers at all and skip that requirement. The same directory list appears in #11147, #11150 and #11152. Details are in an earlier comment here, including two other defects that look templated across the series.

No rush on the review itself — it's the label that blocks, not the queue.

feat(medic#11151): query report uuids by form on the shared uuid route

fix(medic#11151): add integration coverage and fix review findings on…

fix(medic#11151): clear SonarCloud findings on the new integration specs

feat(#11151): add byForm qualifier to Report.v1.getUuidsPage (#11327)

@mrjones-plip

FYI @mrjones-plip @sugat009, looks like this issue was missing from the 5.3 milestone, so I just added it. We might need to re-generate the release notes to include this ticket and its contributor.

@andrablaj

Thanks @andrablaj ! I've updated the release notes to ensure this ticket and its contributors are present..

Navigation Menu

Repository navigation

Add `byForm` qualifier to `Report.v1` (replaces `medic-client/reports_by_form` page query) #11151

Description

Technical Context

Requirements

Adapter Additions

Caller Sweeps

Tests

Acceptance Criteria

Constraints

References

1. The audit grep searches the wrong directories, and silently finds nothing

2. The aggregate-preservation check can never fire

3. The `FormQualifier` snippet has a validation bug

Two smaller notes

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add byForm qualifier to Report.v1 (replaces medic-client/reports_by_form page query) #11151

Description

Description

Technical Context

Requirements

Adapter Additions

Caller Sweeps

Tests

Acceptance Criteria

Constraints

References

Activity

ken-talltree-io commented on Aug 6, 2026

ken-talltree-io commented on Aug 6, 2026

1. The audit grep searches the wrong directories, and silently finds nothing

2. The aggregate-preservation check can never fire

3. The FormQualifier snippet has a validation bug

Two smaller notes

ken-talltree-io commented on Aug 6, 2026

ken-talltree-io commented on Aug 7, 2026

andrablaj commented on Aug 26, 2026

mrjones-plip commented on Aug 26, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Add `byForm` qualifier to `Report.v1` (replaces `medic-client/reports_by_form` page query) #11151

3. The `FormQualifier` snippet has a validation bug