Add month-on-month purchase value difference analysis documentation for a specific supplier

[sonzsara]

No description provided.

[github-actions]

Analytics SQL Review — Month-on-month purchase value difference analysis (supplier-scoped, SSMM)

Ticket: ENG-974 could not be fetched — JIRA returned 404/401 for this ticket ID at both the scoped and unscoped endpoints, so I can't confirm the exact requirement text or check whether the ticket's comments refined the ask. Reviewing against the stated title/purpose instead; treat the fidelity finding below as provisional until the ticket is reachable.

Does it deliver a "month-on-month difference" analysis? No — this is the biggest issue (see inline comment). The query is a flat per-delivery itemized list, not a monthly aggregation with a period-over-period delta. Either the title/doc need to describe what this actually is (a supplier delivery drill-down), or the SQL needs real MoM logic (date_trunc + LAG()/self-join).

Are the numbers trustworthy as delivered (for what the query does do — a completed-delivery itemization)?

• Facility/tenant scope: fine — supplier-scoped by design (SSMM suffix), documented in Notes.
• Status filtering: correct pattern (completed/completed on both delivery and order); entered_in_error and other terminal states are implicitly excluded since status = 'completed' is an equality filter, so no missing-exclusion bug here.
• Join fan-out: none observed — invoice/order/product/user/org/location are all single-valued per delivery row, so SUM/COUNT isn't at risk (there's no aggregation in this query at all, which is itself part of the fidelity gap above).
• deleted = FALSE is missing on every table (High, inline) — low-signal on its own but cheap and matches repo convention (see sibling internalsupplydeliverypurchase_ssmm.md).
• Pricing: value is computed from the current emr_product.purchase_price, not emr_supplydelivery.total_purchase_price (a likely point-in-time snapshot) — flagged inline as High, since it directly undermines a month-on-month comparison if prices change over time.
• Minor: quoted numeric literal '20697' for a bigint column (Low, cosmetic, matches Postgres implicit cast but diverges from sibling file's style).

Docs: Notes section correctly documents the hardcoded supplier_id; Parameters table matches the two commented-out {{start_date}}/{{end_date}} filters; Last updated is current. Hygiene is fine — correct domain folder, correct _ssmm suffix, one file per PR.

Verdict: Not yet safe to publish as a "month-on-month difference" dashboard tile under its current title/purpose — as written it's an itemized delivery list, and if consumed as MoM it will read as more information than it is. Top fixes: (1) resolve the title/purpose vs. actual grain mismatch, (2) confirm whether historical or current pricing was intended, (3) add deleted = FALSE for consistency.

Generated by Analytics SQL Reviewer for #159 · auto · 73.3 AIC · ⌖ 3.48 AIC · ⊞ 14.7K

[github-actions]

[Critical] Query does not deliver a month-on-month difference analysis.

The title and PR title promise a "Month on Month Purchase Value Difference Analysis", but the SQL is a flat, row-per-delivery itemized list ordered by value DESC — there is no GROUP BY on month, no time-bucketing, and no computed delta comparing one period to the previous one (e.g. LAG() or a self-join). The ## Purpose text ("Lists completed supply deliveries ... at product level") actually matches the SQL, but not the filename/title at all.

ENG-974 could not be fetched to confirm the exact ask (JIRA returned 404/401 for this ticket), but going purely off the stated title: if a real MoM diff was intended, this query doesn't answer it — a dashboard consumer would see a raw delivery list, not a period-over-period comparison. Either the title/purpose need to be rewritten to describe what this query actually is (a supplier delivery drill-down), or the SQL needs monthly aggregation plus a delta calculation.

Suggested direction if a real MoM diff was intended:

WITH monthly AS (
  SELECT date_trunc('month', esd.created_date) AS month,
         epk.id AS product_id,
         epk.name AS product_name,
         SUM(ep.purchase_price * esd.supplied_item_quantity) AS total_value
  FROM emr_supplydelivery esd
  JOIN emr_deliveryorder edo ON esd.order_id = edo.id
  JOIN emr_product ep ON esd.supplied_item_id = ep.id
  JOIN emr_productknowledge epk ON epk.id = ep.product_knowledge_id
  WHERE esd.status = 'completed' AND edo.status = 'completed'
    AND edo.supplier_id = 20697
    AND esd.deleted = FALSE AND edo.deleted = FALSE
  GROUP BY 1, 2, 3
)
SELECT *, total_value - LAG(total_value) OVER (PARTITION BY product_id ORDER BY month) AS value_diff
FROM monthly ORDER BY month, product_name;

[github-actions]

[High] Missing deleted = FALSE on every joined table.

None of emr_supplydelivery, emr_deliveryorder, emr_product, emr_productknowledge, emr_organization, or emr_facilitylocation filter deleted = FALSE here. It's a weak signal on its own (health/inventory records are rarely hard/soft-deleted), but it's cheap, indexed, and the sibling query in this same folder (internalsupplydeliverypurchase_ssmm.md) does include it (fl.deleted = FALSE). Worth adding at least for esd, edo, and fl for consistency with repo convention.

WHERE esd.status = 'completed'
  AND esd.deleted = FALSE
  AND edo.deleted = FALSE
  AND edo.origin_id IS NULL
  AND edo.supplier_id = '20697'
  AND edo.status = 'completed'
  AND ep.purchase_price IS NOT NULL

[github-actions]

[High] Value is computed from the product's current price, not the price at time of delivery.

emr_supplydelivery has its own total_purchase_price column (a numeric snapshot presumably captured at delivery time), but this query instead multiplies the live emr_product.purchase_price by supplied_item_quantity. If the supplier's price changes after a delivery is recorded, every historical row's value/unit_price silently changes too — which directly undermines a month-on-month comparison (the whole point is to compare against what was actually paid in each period, not what the current price list says). Worth confirming with the author whether esd.total_purchase_price should be used instead (falling back to the computed value only where it's NULL).

COALESCE(esd.total_purchase_price, ep.purchase_price * esd.supplied_item_quantity) AS value

[github-actions]

[Low] supplier_id compared against a quoted string.

edo.supplier_id is a bigint FK, but the literal is quoted ('20697'). Postgres will implicitly cast this so it won't break the query or the index usage, but it's inconsistent with the sibling query in this folder (internalsupplydeliverypurchase_ssmm.md), which uses the unquoted delivery_order.supplier_id = 20697. Minor, but worth matching repo convention.

AND edo.supplier_id = 20697