add openspec config, dataApprovals implementation and unit/integration test by eperedo · Pull Request #186 · EyeSeeTea/d2-api

eperedo · 2026-04-22T22:03:11Z

📌 References

Issue: Closes https://app.clickup.com/t/869ckwad6 #869ckwad6

📝 Implementation

add openspec and claude configuration
add dataApprovals endpoints https://docs.dhis2.org/en/develop/using-the-api/dhis-core-version-242/data-approval.html
add unit and integration testing

📹 Screenshots/Screen capture

🔥 Notes to the tester

gqcorneby

Cool! I just have minor comments and then I ask claude to do some verification and pointed out a few potential corrections

There's still one openspec left unarchived. I think archive timing is something that should be discussed, just pointing it out here because there 2 already archived :)
Maybe we can add a section in the PR to note down breaking changes. I think there are a few mentioned in openspec files so maybe these would be good to highlight in the PR description
Nice to have tests here! A small update would be to follow convention from other projects move tests and integration tests inside __tests__

src/api/
  dataApprovals.ts
  trackerEvents.ts
  __tests__/
    common.spec.ts
    dataApprovals.spec.ts
    integration/
      trackerEnrollments.spec.ts
      trackerEvents.spec.ts
      trackerTrackedEntities.spec.ts

gqcorneby · 2026-04-23T03:42:11Z

+
+export interface DataApprovalByCategoryOptionCombo {
+    readonly id: Id;
+    readonly level?: Id;


This is typed as an Id but, after verifying the response, it should be an object.
readonly level?: { readonly id?: Id; readonly level?: number; readonly name?: string }

https://play.im.dhis2.org/stable-2-42-4/api/dataApprovals/categoryOptionCombos?wf=rIUL3hYOjJc&pe=20240
1&ou=O6uvpzGd5pu

[ { "level": {}, "ou": "O6uvpzGd5pu", "permissions": { "mayApprove": false, "mayUnapprove": false, "mayAccept": false, "mayUnaccept": false, "mayReadData": true }, "accepted": false, "id": "HllvX50cXC0", "ouName": "Bo" } ]

gqcorneby · 2026-04-23T03:44:41Z

+    approvals: { ou: Id; aoc: Id }[];
+}>;
+
+export interface DataApprovalStatus {


The type omits mayReadData, but after verifying the response it is always returned. Add:

readonly mayReadData: boolean;

https://play.im.dhis2.org/stable-2-42-4/api/dataApprovals?wf=rIUL3hYOjJc&pe=202401&ou=O6uvpzGd5pu

{ "mayApprove": false, "mayUnapprove": false, "mayAccept": false, "mayUnaccept": false, "mayReadData": true, "state": "UNAPPROVED_WAITING" }

This property is not in the documentation. I'll included anyway because it's been used in our apps. Thanks!

gqcorneby · 2026-04-23T04:03:06Z

+
+export type DataApprovalState = (typeof dataApprovalStates)[number];
+
+type WorkflowOrDataSet<T extends { wf?: unknown; ds?: unknown }> = RequireAtLeastOne<


RequireAtLeastOne requires one of the keys but permits both. The design.md (§2) claims "if both are passed, both are forwarded — DHIS2 resolves the ambiguity server-side (workflow wins)". Verified against the live server: that is wrong. When both are present, ds wins and wf is ignored. A consumer who passes both thinking wf is authoritative silently gets a different workflow's approval state, with no type error and no runtime error.

Suggested fix — discriminated union:

type ApprovalSelector = | { wf: Id; ds?: never; pe: string; ou: Id; aoc?: Id } | { wf?: never; ds: Id; pe: string; ou: Id; aoc?: Id };

Same treatment for BulkApprovalSelector and BulkApprovalPayload.

Reproduction

Requires one dataset to be attached to a different workflow than the one passed as wf. The default play state has only one workflow (rIUL3hYOjJc) and no dataset attached to it besides pBOMPrpg1QX, so the test needs a second workflow that maps through Child Health (BfMAe6Itzgt). All four steps below are reversible; step 4 restores the play server.

# 1. create a second workflow curl -s -u 'admin:district' -X POST \ -H 'Content-Type: application/json' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovalWorkflows' \ -d '{"name":"Test Review Workflow","periodType":"Monthly","dataApprovalLevels":[{"id":"VS3XyxNvrer"}]}' # → 201 Created, uid: S5hy9kO44OL (yours will differ — substitute in the next step) # 2. attach it to Child Health dataset (BfMAe6Itzgt) curl -s -u 'admin:district' -X PATCH \ -H 'Content-Type: application/json-patch+json' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataSets/BfMAe6Itzgt' \ -d '[{"op":"add","path":"/workflow","value":{"id":"S5hy9kO44OL"}}]' # 3. the three queries that expose the override # 3a. wf only → baseline for rIUL3hYOjJc curl -s -u 'admin:district' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovals?wf=rIUL3hYOjJc&pe=202401&ou=O6uvpzGd5pu' # → {"state":"UNAPPROVED_WAITING","mayApprove":false,...} # 3b. ds only → baseline for the Test workflow (S5hy9kO44OL) curl -s -u 'admin:district' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovals?ds=BfMAe6Itzgt&pe=202401&ou=O6uvpzGd5pu' # → {"state":"UNAPPROVED_READY","mayApprove":true,...} # 3c. BOTH → response matches 3b, NOT 3a curl -s -u 'admin:district' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovals?wf=rIUL3hYOjJc&ds=BfMAe6Itzgt&pe=202401&ou=O6uvpzGd5pu' # → {"state":"UNAPPROVED_READY","mayApprove":true,...} ← ds silently wins # 4. cleanup — restore play to original state curl -s -u 'admin:district' -X PATCH \ -H 'Content-Type: application/json-patch+json' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataSets/BfMAe6Itzgt' \ -d '[{"op":"remove","path":"/workflow"}]' curl -s -u 'admin:district' -X PATCH \ -H 'Content-Type: application/json-patch+json' \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovalWorkflows/S5hy9kO44OL' \ -d '[{"op":"replace","path":"/dataApprovalLevels","value":[]}]' curl -s -u 'admin:district' -X DELETE \ 'https://play.im.dhis2.org/stable-2-42-4/api/dataApprovalWorkflows/S5hy9kO44OL'

tokland

Looks sane to me. I added some notes (non-blocking), just for discussion.

tokland · 2026-04-23T11:54:55Z

@@ -0,0 +1,136 @@
+## Context
+
+The `/api/dataApprovals` and `/api/dataAcceptances` surface in DHIS2 v42 is [documented here](https://docs.dhis2.org/en/develop/using-the-api/dhis-core-version-242/data-approval.html#data-approval). It covers four concerns over the same selector tuple `(workflow|dataSet, period, orgUnit, attributeOptionCombo)`:


In addition of using the documentation, we have the OpenAPI specification

$ curl-play 'https://play.im.dhis2.org/dev-2-42/api/openapi.json' | tee openapi.json | jq -r '.paths | keys[]' | grep dataApprovals /api/dataApprovals/ /api/dataApprovals/approvals /api/dataApprovals/categoryOptionCombos /api/dataApprovals/dataAcceptances /api/dataApprovals/dataAcceptances/acceptances /api/dataApprovals/dataAcceptances/unacceptances /api/dataApprovals/multiple /api/dataApprovals/status /api/dataApprovals/unapprovals $ cat openapi.json | jq '.paths["/api/dataApprovals/approvals"]' | head -n10 { "get": { "operationId": "DataApproval.getMultipleApprovalPermissions", "parameters": [ { "name": "aoc", "in": "query", "schema": { "type": "array", "items": {

I'm not sure what the best approach would be - I imagine Claude can draw insights from both sources.

A lot of possibilities open up from here, but they're out of scope for this exploratory PR; it's really another project in itself.

For example, just as we have an auto-generated schemas.ts for entities (generated from /api/schemas), we could have an auto-generated, fully typed endpoints.ts (generated from /api/openapi.json). Our code would then do things like get("/api/dataApprovals/approvals", params), where params would be fully typed because it would come from the auto-generated types.

From there, a lot of practical decisions follow: the structure of the endpoints (flat or nested, as we have now), backward compatibility, and how to integrate the custom logic we've added — like our fully type-safe, nested fields metadata request, which we'd definitely want to keep.

tokland · 2026-04-23T12:04:01Z

+### 7. Testing strategy
+
+-   File: `src/api/dataApprovals.test.ts`, colocated next to the module.
+-   Approach: `getMockApiFromClass(D2Api)` from `src/testing.ts` produces a `D2Api` + `axios-mock-adapter` pair.


I think this method was meant to be used by applications, not d2-api itself. (And I don't think we are really using it in the projects either).

tokland · 2026-04-29T08:27:37Z

+
+#### Scenario: Bulk unaccept posts to unacceptances
+- **WHEN** called with `{ wf: ["w1"], pe: ["201601"], approvals: [{ ou: "o1", aoc: "a1" }] }`
+- **THEN** the request is `POST /dataAcceptances/unacceptances` with that JSON body


This is a exploratory project, so I'll make also exploratory comments :)

In the particular case of an API — where the source of truth isn't what we want to do but what the API provides — maybe it should be enough for the spec to point to some document (plus OpenAPI in the future)?

In other words: I'm quite worried about having duplicated knowledge/goals scattered all over the place.

tokland · 2026-04-29T08:32:04Z

+const AOC = "HllvX50cXC0";
+const AOC_2 = "ranftQIH5M9";
+
+function getMockApi(): { api: D2Api; mock: MockAdapter } {


So Claude decided he didn't want to use our getMockApiFromClass either :)

tokland · 2026-04-29T08:37:43Z

+    program: "IpHINAT79UW",
+    trackedEntityType: "nEenWmSyUEp",
+    orgUnit: "DiszpKrYNg8",
+} as const;


minor, the AI tends to overuse as const.

tokland · 2026-04-29T08:38:11Z

+# claude
+.claude/settings.local.json
+
+.DS_Store


EOF missing (maybe we should add some comment to some AI file)

tokland · 2026-04-29T08:41:22Z

@@ -1 +1 @@
-v18.14.2
+v22


I'm unsure about the granularity. When asked, Claude seems to prefer the full version, for reproducibility.

tokland · 2026-04-29T08:44:18Z

        "prerelease": "yarn build",
        "release": "bash scripts/publish.sh",
-        "test": "echo no-op"
+        "test": "yarn test:unit && yarn test:integration",


It's debatable whether the base yarn test - which will be the one executed in CI - should include the integration tests, since a problem with the Play server would mean red flags on PRs. On the other hand, if it's a manual-only action, it may end up being ignored. So I don't have a strong opinion, just raising the subject.

eperedo added 6 commits April 7, 2026 18:22

add AI setup

10c27ad

update openspec config

5087c18

add test for serialization functions

2ed8ad6

added dataApprovals endpoint

6b6bfe1

integration test (wip)

6bd16d8

fixed tracker endpoints and added integration tests

82a972c

eperedo requested a review from gqcorneby April 22, 2026 22:03

gqcorneby reviewed Apr 23, 2026

View reviewed changes

eperedo requested a review from tokland April 23, 2026 11:05

eperedo added 2 commits April 27, 2026 19:11

make wf/ds mutually exclusive

14e1f36

move tests into __tests__ folder

fc2d2ba

tokland approved these changes Apr 29, 2026

View reviewed changes

eperedo requested a review from gqcorneby April 30, 2026 00:51


		export type DataApprovalState = (typeof dataApprovalStates)[number];

		type WorkflowOrDataSet<T extends { wf?: unknown; ds?: unknown }> = RequireAtLeastOne<

		@@ -0,0 +1,136 @@
		## Context

		The `/api/dataApprovals` and `/api/dataAcceptances` surface in DHIS2 v42 is [documented here](https://docs.dhis2.org/en/develop/using-the-api/dhis-core-version-242/data-approval.html#data-approval). It covers four concerns over the same selector tuple `(workflow\|dataSet, period, orgUnit, attributeOptionCombo)`:

		@@ -1 +1 @@
		v18.14.2
		v22

Conversation

eperedo commented Apr 22, 2026

📌 References

📝 Implementation

📹 Screenshots/Screen capture

🔥 Notes to the tester

Uh oh!

gqcorneby left a comment

Choose a reason for hiding this comment

Uh oh!

gqcorneby Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

gqcorneby Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

eperedo Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

gqcorneby Apr 23, 2026

Choose a reason for hiding this comment

Reproduction

Uh oh!

tokland left a comment

Choose a reason for hiding this comment

Uh oh!

tokland Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

tokland Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tokland Apr 23, 2026 •

edited

Loading

tokland Apr 23, 2026 •

edited

Loading

tokland Apr 29, 2026 •

edited

Loading

tokland Apr 29, 2026 •

edited

Loading

tokland Apr 29, 2026 •

edited

Loading

tokland Apr 29, 2026 •

edited

Loading