Skip to content

docs: ADR for querystore indexing decisions - #176

Open
dkayiwa wants to merge 4 commits into
mainfrom
querystore-adr
Open

docs: ADR for querystore indexing decisions#176
dkayiwa wants to merge 4 commits into
mainfrom
querystore-adr

Conversation

@dkayiwa

@dkayiwa dkayiwa commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds docs/adr/0001-querystore-indexing.md documenting the load-bearing decisions behind the billing module's participation in the querystore SPI.
  • 16 decisions (D1-D16) covering resource type choice, field shape contracts, sorting for stable bytes, the shared BillingAuditFields and BillingDisplayNames helpers, trigger naming, lazy-init rationale, and intentional non-decisions.
  • 6 open issues catalogued with failure-mode sentences (OLD bill status drift on adjustment, closeOpenTimesheets bypasses indexing, lazy walks per save, percentage discount lazy nav, rename staleness, text-blob comma conflation).
  • Stands alone — does not depend on any in-flight implementation PR; the ADR can land before or after the implementation branches it describes.

Test plan

  • Read top-to-bottom — is the rationale for each decision clear without reading the code?
  • Spot-check D2 (BillableService exclusion) against AbstractIndexingAdvice<T extends BaseOpenmrsData> — type bound matches.
  • Spot-check D4 (List convention) against VisitRecordSerializer.FIELD_ENCOUNTER_UUIDS / AllergyRecordSerializer.FIELD_REACTIONS.
  • Spot-check D11 (self-call vs outer-call AOP) against BaseEntityDataServiceImpl.voidEntity calling save() internally.
  • Confirm the open-issues list is honest — each has a concrete failure-mode sentence, not a vague label.

🤖 Generated with Claude Code

dkayiwa and others added 2 commits May 20, 2026 18:28
Records the load-bearing decisions behind the four querystore resource
types this module exposes (bill, bill_refund, bill_discount, timesheet)
plus the explicit non-decision to skip BillableService — the querystore
SPI is BaseOpenmrsData-bounded and catalog metadata doesn't fit.

The file lives at docs/adr/0001-querystore-indexing.md so future
contributors planning a new field, a new resource type, or a new
consumer have one place to start. It also catalogues the deferred
items with their failure-mode sentences so the next maintainer can
judge urgency without rediscovering the same trade-offs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sixteen decision sections plus open issues and consequences is too much
to scan linearly — anchor links let a reader jump straight to the
specific decision they're checking on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented May 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.37%. Comparing base (16dbd6a) to head (1337738).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #176   +/-   ##
=========================================
  Coverage     34.37%   34.37%           
  Complexity      800      800           
=========================================
  Files           212      212           
  Lines          5144     5144           
  Branches        620      620           
=========================================
  Hits           1768     1768           
  Misses         3184     3184           
  Partials        192      192           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dkayiwa and others added 2 commits May 20, 2026 18:37
Four more load-bearing decisions surfaced while re-reading the slice:
the refund's deliberate audit-trail divergence on voided line items
(D17), the touchParentBill cross-resource propagation contract (D18),
the defensive null-guards that exist because AbstractIndexingAdvice
swallows RuntimeException per entity (D19), and the deliberate
rejection of a shared abstract serializer base (D20).

D18 is particularly load-bearing for anyone adding a new child resource
type that affects bill-level aggregates — without it, the parent bill's
index goes stale on every child save.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four more decisions surfaced on a fresh pass: the billing_* namespacing
convention that prevents cross-module document collisions (D21), the
text-vs-metadata contract that protects consumers from parsing the
prose blob (D22), the deliberate asymmetric denormalization boundary
(cashier/cashpoint names yes, patient/visit names no) so future
contributors don't chase false consistency at the cost of lazy fetches
on every save (D23), and the explicit "voided rows stay in the index"
default so audit queries keep working (D24).

D21 and D22 prevent specific failure modes (silent overwrites,
parsing-based coupling). D23 documents a real architectural boundary.
D24 is an affirmation of behavior consumers depend on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

| `RESOURCE_TYPE_BILL` | `billing_bill` | Yes | `BillService` |
| `RESOURCE_TYPE_BILL_REFUND` | `billing_bill_refund` | Yes | `BillRefundService` |
| `RESOURCE_TYPE_BILL_DISCOUNT`| `billing_bill_discount` | Yes | `BillDiscountService` |
| `RESOURCE_TYPE_TIMESHEET` | `billing_timesheet` | No (provider) | `ITimesheetService` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkayiwa we plan to remove the timesheet feature (since its more of a cashier HR feature and less of a billing feature). There is an open PR for it here.


## Context

The billing module persists `Bill`, `BillLineItem`, `Payment`, `BillDiscount`, `BillRefund`, `Timesheet`, `BillableService` and related entities. Free-text and structured search across this data — "find bills paid by Mobile Money", "approval queue for pending discounts", "find the bill for this lab order", "who was on duty between 2pm and 3pm" — was previously not feasible without scanning the persistence layer one row at a time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that I think would be helpful was if we could get an answer for a question like, "what was the patient billed for during the last visit” or “was this patient billed for an xray during their last visit"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants