Skip to content

Add documentation for Patients with Billable Charge Item and Encounter/Appointment Details query #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sonzsara wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add documentation for Patients with Billable Charge Item and Encounter/Appointment Details query #147

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 57 additions & 0 deletions ...patients_with_billable_chargeitem_and_encounter_and_appointment_details_ssmm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@

# Patients with Billable Charge Item and Encounter/Appointment Details - SSMM

> Patient-level list of encounters linked to appointments that have billable charge items

## Purpose

Returns encounter records at SSMM where the linked appointment (`emr_tokenbooking`) has a charge item in `billable` status. The result includes patient details, SSMM identifier, encounter and appointment statuses, charge item status/value, and encounter date.
Comment thread
sonzsara marked this conversation as resolved.

## Parameters

| Parameter | Type | Description | Example |
|-----------|------|-------------|---------|
| `date_filter` | DATE / range | Metabase date filter (typically bound to `emr_encounter.created_date`) | `'2026-08-01'` |
| `encounter_class` | TEXT | Filter by encounter class (exact match on `emr_encounter.encounter_class`) | `'amb'` |
| `encounter_status` | TEXT | Filter by encounter status (exact match on `emr_encounter.status`) | `'in-progress'` |
| `ssmm_id` | TEXT | Filter by patient SSMM identifier value (exact match on `emr_patientidentifier.value`) | `'SSMM-100245'` |

---

## Query

```sql
SELECT
emr_patient.name AS patient_name,
emr_patientidentifier.value AS ssmm_id,
emr_encounter.status AS encounter_status,
emr_encounter.encounter_class AS encounter_class,
emr_tokenbooking.status AS appointment_status,
emr_chargeitem.status AS charge_item_status,
emr_chargeitem.total_price AS total_price,
emr_encounter.created_date AS encounter_date
FROM emr_encounter
JOIN emr_patient
ON emr_patient.id = emr_encounter.patient_id
LEFT JOIN emr_patientidentifier
ON emr_patientidentifier.patient_id = emr_patient.id
AND emr_patientidentifier.config_id = 21
JOIN emr_tokenbooking
ON emr_tokenbooking.associated_encounter_id = emr_encounter.id
JOIN emr_chargeitem
ON emr_chargeitem.id = emr_tokenbooking.charge_item_id
WHERE emr_chargeitem.status = 'billable'

@github-actions github-actions Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Critical] No facility scoping — the _ssmm suffix promises a single-facility query, but nothing in the SQL enforces it.

Facility boundaries are normally enforced by the application, not by Postgres — a raw query has to add it explicitly (care-sql-code-review skill, inversion 2). This query has no facility_id predicate anywhere: not on emr_encounter, not on emr_tokenbooking, not on emr_chargeitem. As written it returns billable charge items across every facility in the deployment, not just SSMM, which is both a wrong-number bug for a per-facility dashboard and a cross-tenant data exposure.

Fix — scope through the encounter (or another table that carries facility_id):

WHERE emr_chargeitem.status = 'billable'
  AND emr_chargeitem.total_price > 0
  AND emr_encounter.facility_id = <ssmm_facility_id>
  ...

If the intent is genuinely "all facilities, but named _ssmm for another reason," please say so in ## Notes — as it stands the filename and the SQL disagree.

@sonzsara sonzsara Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, there is only one facility and thats ssmm

@github-actions github-actions Bot Sep 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Understood — thanks for confirming there's only one facility in this deployment today.

One thing worth flagging even so: this repo's convention for other single-facility assumptions (e.g. the config_id = 21 identifier mapping right below) is to say so explicitly in ## Notes, precisely because these facts aren't visible from the SQL itself and can silently stop being true (a second facility gets provisioned, a new encounter type is added elsewhere, etc.) without anyone noticing until the dashboard numbers are wrong. Right now ## Notes documents the identifier mapping but says nothing about the facility assumption.

Suggest adding a line such as:

Facility scope: This deployment currently has only one facility (SSMM); no explicit facility_id filter is applied. If a second facility is ever added to this instance, add AND emr_encounter.facility_id = <ssmm_facility_id> to keep this report scoped correctly.

That turns a query-level assumption into a documented, checkable one rather than an implicit fact a future reader has to rediscover. Not blocking — resolving this thread since the underlying data-scope concern is a conscious, confirmed decision for the current environment.

Generated by Analytics SQL Reviewer for #147 · auto · 30.9 AIC · ⌖ 2.38 AIC · ⊞ 14.7K

AND emr_chargeitem.total_price > 0
Comment thread
sonzsara marked this conversation as resolved.
--[[AND {{date_filter}}]]
--[[AND {{encounter_class}}]]
--[[AND {{encounter_status}}]]
--[[AND emr_patientidentifier.value = {{ssmm_id}}]]
ORDER BY emr_encounter.created_date DESC, emr_patient.name;
```

@github-actions github-actions Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Medium] Missing ### Output section required by TEMPLATE.md.

The template calls for an ### Output table (column/type/description) directly under ## Query, documenting the 8 selected columns for the dashboard consumer. This doc jumps straight from the SQL fence to ## Notes — please add the output table before merging.

@sonzsara sonzsara Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Outputs are not needed, they are optional.

@github-actions github-actions Bot Sep 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair enough — ### Output is indeed optional per TEMPLATE.md conventions in this repo (only ## Purpose, ## Parameters, and ## Query are consistently required across existing docs). Not blocking on this. Resolving.

Generated by Analytics SQL Reviewer for #147 · auto · 30.9 AIC · ⌖ 2.38 AIC · ⊞ 14.7K


## Notes

- **Core cohort:** Only rows where `emr_chargeitem.status = 'billable'` and `total_price > 0` are included.
- **Identifier mapping:** `config_id = 21` is hardcoded for SSMM patient identifier configuration; update if this mapping changes.

*Last updated: 2026-08-13*
Loading