From 367fc0324e4f79c4924cd3c0324806a7c7138986 Mon Sep 17 00:00:00 2001
From: Thomas Joise <thomas@sims.com>
Date: Tue, 21 Jul 2026 00:33:50 +0530
Subject: [PATCH] fix(dashboard): never report "all good" when the fetch failed

The SMS low-balance alert rendered only when the request succeeded with
lowCount > 0, so a FAILED check looked exactly like "no client is low" --
the dangerous direction. It now has three distinct states: a skeleton
while the check is in flight, an ErrorState with retry when it fails, and
a quiet positive caption only for a loaded, genuinely-empty result (which
also spells out `unknownCount`, since balances the gateway could not
return are not "fine" either).

Same three-state audit on the rest of the page: the "Onboarding stalled"
tile had the error case fixed already but still showed count 0 and
"Nothing stalled." while its fetch was in flight -- it now takes a
`loading` prop (new on Card) and shows a skeleton, and its count appears
only once the list has loaded. Every other bento card is fed by the one
dashboard payload, whose loading/error/empty states are already
distinguished at page level.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 apps/hq-web/src/pages/Dashboard.tsx | 39 ++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/apps/hq-web/src/pages/Dashboard.tsx b/apps/hq-web/src/pages/Dashboard.tsx
index e39fbf9..d741859 100644
--- a/apps/hq-web/src/pages/Dashboard.tsx
+++ b/apps/hq-web/src/pages/Dashboard.tsx
@@ -70,13 +70,27 @@ export function Dashboard() {
             </div>
           </section>
 
-          {/* SMS low-balance alert (D28) — surfaced here so it's seen without opening the SMS screen. */}
-          {smsLow.data !== undefined && smsLow.data.lowCount > 0 && (
+          {/* SMS low-balance alert (D28) — surfaced here so it's seen without opening the SMS
+              screen. Three states, never collapsed into one: while the check is in flight a
+              skeleton stands in; a FAILED check says so with a retry (rendering nothing there
+              would read as "no client is low" — the dangerous direction); and only a loaded,
+              genuinely-empty result gets the positive line. `unknownCount` is spelled out in
+              that line because balances the gateway couldn't return aren't "fine" either. */}
+          {smsLow.error !== undefined ? (
+            <ErrorState message={`SMS balances could not be checked — ${smsLow.error}`} onRetry={smsLow.reload} />
+          ) : smsLow.data === undefined ? (
+            <Skeleton rows={1} />
+          ) : smsLow.data.lowCount > 0 ? (
             <Notice tone="warn">
               <Link to="/sms-balances" style={{ color: 'inherit', fontWeight: 600 }}>
                 {smsLow.data.lowCount} SMS client{smsLow.data.lowCount === 1 ? '' : 's'} low on balance — review &amp; top up →
               </Link>
             </Notice>
+          ) : (
+            <p className="dash-cap">
+              SMS balances checked — none low
+              {smsLow.data.unknownCount > 0 ? ` (${smsLow.data.unknownCount} could not be read)` : ''}.
+            </p>
           )}
 
           {/* KPIs — the app's existing stat cards (kept minimal; red only when a send failed). */}
@@ -146,11 +160,14 @@ export function Dashboard() {
             </Card>
 
             <Card
-              title="Onboarding stalled" count={stalled.data?.length ?? 0}
+              title="Onboarding stalled"
+              // Neither a fetch failure nor a still-running fetch is a genuine empty result:
+              // the error gets an ErrorState with retry, the pending fetch a skeleton, and the
+              // count/"Nothing stalled." line only appear once the list has actually loaded.
+              {...(stalled.data !== undefined ? { count: stalled.data.length } : {})}
               empty="Nothing stalled."
-              // A fetch failure is not the same as a genuine empty result — show the error
-              // (with retry) instead of quietly reporting "Nothing stalled." on failure.
-              isEmpty={stalled.error === undefined && (stalled.data ?? []).length === 0}
+              loading={stalled.data === undefined && stalled.error === undefined}
+              isEmpty={stalled.data !== undefined && stalled.data.length === 0}
             >
               {stalled.error !== undefined
                 ? <ErrorState message={stalled.error} onRetry={stalled.reload} />
@@ -182,10 +199,12 @@ export function Dashboard() {
 }
 
 /** One bento card: an uppercase-labelled header (+ count and optional link) over a
- *  scrollable list, or an empty-state line when there's nothing to show. */
+ *  scrollable list, or an empty-state line when there's nothing to show. Cards fed by
+ *  their own request pass `loading` so a fetch still in flight shows a skeleton instead
+ *  of the empty line — "nothing to show" must mean the data loaded and was empty. */
 function Card(props: {
   title: string; count?: number; link?: { to: string; label?: string }
-  empty: string; isEmpty: boolean; children: ReactNode
+  empty: string; isEmpty: boolean; loading?: boolean; children: ReactNode
 }) {
   return (
     <section className="dash-card">
@@ -194,7 +213,9 @@ function Card(props: {
         {props.count !== undefined && <span className="count">{props.count}</span>}
         {props.link !== undefined && <Link to={props.link.to}>{props.link.label ?? 'All'} →</Link>}
       </div>
-      {props.isEmpty ? <div className="dash-empty">{props.empty}</div> : <div className="dash-list">{props.children}</div>}
+      {props.loading === true ? <div className="dash-list"><Skeleton rows={2} /></div>
+        : props.isEmpty ? <div className="dash-empty">{props.empty}</div>
+        : <div className="dash-list">{props.children}</div>}
     </section>
   )
 }