From 84a8b78acdc239d525c0dc1f428860f5cba01413 Mon Sep 17 00:00:00 2001
From: david raistrick <1108844+keen99@users.noreply.github.com>
Date: Mon, 29 Jun 2026 13:13:46 -0400
Subject: [PATCH] M2: refactor DisplayView to storage adapter (red test first)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DisplayView missed in original M2 refactor — raw onSnapshot(doc(db,path))
survived. In ws/memory mode db is a stub sentinel, so raw SDK calls crash
('Expected first argument to collection() to be a CollectionReference...').
Reported by human testing player display after start combat.

TDD:
1. RED: DisplayView.characterization.test.js asserts adapter.subscribeDoc
   called for campaign + encounter + activeDisplay paths. Adapter recorder
   (getAdapterCalls/resetAdapterCalls in firebase.js) instruments subscribe
   calls — catches raw-SDK bypass that firebase mock alone cannot (mock db
   satisfies raw onSnapshot, hiding the bug).
2. Fix: 2 raw onSnapshot sites in DisplayView -> storage.subscribeDoc.
3. GREEN: 2 new tests pass, 116 total green.

Audit confirmed DisplayView was the ONLY remaining raw SDK site in App.js.
---
 src/App.js                               | 32 ++++---------
 src/DisplayView.characterization.test.js | 60 ++++++++++++++++++++++++
 src/storage/firebase.js                  |  9 ++++
 3 files changed, 79 insertions(+), 22 deletions(-)
 create mode 100644 src/DisplayView.characterization.test.js

diff --git a/src/App.js b/src/App.js
index fe01c58..2c1ae77 100644
--- a/src/App.js
+++ b/src/App.js
@@ -2453,35 +2453,23 @@ function DisplayView() {
         setIsLoadingEncounter(true);
         setEncounterError(null);
 
-        const campaignDocRef = doc(db, getPath.campaign(activeCampaignId));
-        unsubscribeCampaign = onSnapshot(
-          campaignDocRef,
-          (campSnap) => {
-            if (campSnap.exists()) {
-              setCampaignBackgroundUrl(campSnap.data().playerDisplayBackgroundUrl || '');
-            } else {
-              setCampaignBackgroundUrl('');
-            }
-          },
-          (err) => console.error("Error fetching campaign background:", err)
+        unsubscribeCampaign = storage.subscribeDoc(
+          getPath.campaign(activeCampaignId),
+          (camp) => {
+            setCampaignBackgroundUrl((camp && camp.playerDisplayBackgroundUrl) || '');
+          }
         );
 
-        const encounterPath = getPath.encounter(activeCampaignId, activeEncounterId);
-        unsubscribeEncounter = onSnapshot(
-          doc(db, encounterPath),
-          (encDocSnap) => {
-            if (encDocSnap.exists()) {
-              setActiveEncounterData({ id: encDocSnap.id, ...encDocSnap.data() });
+        unsubscribeEncounter = storage.subscribeDoc(
+          getPath.encounter(activeCampaignId, activeEncounterId),
+          (enc) => {
+            if (enc) {
+              setActiveEncounterData({ id: activeEncounterId, ...enc });
             } else {
               setActiveEncounterData(null);
               setEncounterError("Active encounter data not found.");
             }
             setIsLoadingEncounter(false);
-          },
-          (err) => {
-            console.error("Error fetching active encounter details:", err);
-            setEncounterError("Error loading active encounter data.");
-            setIsLoadingEncounter(false);
           }
         );
       } else {
diff --git a/src/DisplayView.characterization.test.js b/src/DisplayView.characterization.test.js
new file mode 100644
index 0000000..633edec
--- /dev/null
+++ b/src/DisplayView.characterization.test.js
@@ -0,0 +1,60 @@
+// DisplayView.characterization.test.js
+// Lock DisplayView uses storage adapter (subscribeDoc), NOT raw SDK onSnapshot(doc(db)).
+// Blind spot caught: M2 refactor missed DisplayView; raw SDK + ws stub db = crash.
+// Test asserts adapter recorder shows subscribeDoc calls when player view boots.
+
+import React from 'react';
+import { render, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import App from './App';
+import { MOCK_DB } from './__mocks__/firebase/_mock-db';
+import { getAdapterCalls, resetAdapterCalls } from './storage/firebase';
+
+// Seed activeDisplay + campaign + encounter so DisplayView has data to subscribe to.
+function seedActiveDisplay() {
+  const campaignPath = 'artifacts/ttrpg-initiative-tracker-default/public/data/campaigns/c1';
+  const encounterPath = 'artifacts/ttrpg-initiative-tracker-default/public/data/campaigns/c1/encounters/e1';
+  const activeDisplayPath = 'artifacts/ttrpg-initiative-tracker-default/public/data/activeDisplay/status';
+
+  MOCK_DB.set(campaignPath, { name: 'Camp', playerDisplayBackgroundUrl: '' });
+  MOCK_DB.set(encounterPath, { name: 'Enc', participants: [], isStarted: true });
+  MOCK_DB.set(activeDisplayPath, { activeCampaignId: 'c1', activeEncounterId: 'e1', hidePlayerHp: false });
+}
+
+describe('DisplayView characterization', () => {
+  beforeEach(() => {
+    window.history.replaceState({}, '', '/display');
+    global.alert = jest.fn();
+    window.open = jest.fn();
+    resetAdapterCalls();
+  });
+  afterEach(() => {
+    window.history.replaceState({}, '', '/');
+  });
+
+  test('DisplayView subscribes via adapter.subscribeDoc (not raw SDK)', async () => {
+    seedActiveDisplay();
+    render(<App />);
+
+    // wait for DisplayView to mount and attempt subscriptions
+    await waitFor(() => {
+      const subs = getAdapterCalls().filter(c => c.fn === 'subscribeDoc');
+      expect(subs.length).toBeGreaterThanOrEqual(1);
+    }, { timeout: 3000 });
+
+    // must subscribe to campaign doc (for background url) and encounter doc
+    const docSubs = getAdapterCalls().filter(c => c.fn === 'subscribeDoc').map(c => c.path);
+    expect(docSubs.some(p => p.includes('/campaigns/c1'))).toBe(true);
+    expect(docSubs.some(p => p.includes('/encounters/e1'))).toBe(true);
+  });
+
+  test('DisplayView also subscribes to activeDisplay status doc via adapter', async () => {
+    seedActiveDisplay();
+    render(<App />);
+
+    await waitFor(() => {
+      const subs = getAdapterCalls().filter(c => c.fn === 'subscribeDoc' && c.path.includes('activeDisplay'));
+      expect(subs.length).toBeGreaterThanOrEqual(1);
+    }, { timeout: 3000 });
+  });
+});
diff --git a/src/storage/firebase.js b/src/storage/firebase.js
index b019a5f..eafcd96 100644
--- a/src/storage/firebase.js
+++ b/src/storage/firebase.js
@@ -15,6 +15,13 @@ import {
   onSnapshot, updateDoc, deleteDoc, query, orderBy, limit, writeBatch, serverTimestamp,
 } from 'firebase/firestore';
 
+// Adapter call recorder (instrumentation, no behavior change).
+// Tests assert adapter.subscribeDoc called (catches raw-SDK bypass like DisplayView).
+const ADAPTER_CALLS = [];
+function recordAdapterCall(entry) { ADAPTER_CALLS.push({ ...entry, ts: Date.now() }); }
+export function getAdapterCalls() { return [...ADAPTER_CALLS]; }
+export function resetAdapterCalls() { ADAPTER_CALLS.length = 0; }
+
 // Path helpers mirror App.js getPath object.
 const APP_ID = process.env.REACT_APP_TRACKER_APP_ID || 'ttrpg-initiative-tracker-default';
 const PUBLIC_DATA_PATH = `artifacts/${APP_ID}/public/data`;
@@ -112,12 +119,14 @@ export function createFirebaseStorage() {
 
     // Subscribe = onSnapshot. cb fires immediately + on change. Returns unsubscribe.
     subscribeDoc(path, cb) {
+      recordAdapterCall({ fn: 'subscribeDoc', path });
       return onSnapshot(doc(db, path), (snap) => {
         cb(snap.exists() ? { id: snap.id, ...snap.data() } : null);
       }, (err) => console.error(`subscribeDoc ${path}:`, err));
     },
 
     subscribeCollection(collectionPath, cb, queryConstraints = []) {
+      recordAdapterCall({ fn: 'subscribeCollection', path: collectionPath });
       const q = queryConstraints.length > 0
         ? query(collection(db, collectionPath), ...queryConstraints)
         : collection(db, collectionPath);