diff --git a/TODO.md b/TODO.md index 54e6164..2e4eaa7 100644 --- a/TODO.md +++ b/TODO.md @@ -90,7 +90,7 @@ REWORK_PLAN.md. ### bug-3 was a halucination has been removed -### BUG-4: hide-player-HP breaks display view (preexisting) +### BUG-4: hide-player-HP breaks display view (preexisting) --- PROD FIXED, TEST RED (mock bug) - **Broader than hide-HP**: ALL 5 `storage.setDoc(getPath.activeDisplay(), ...)` calls use `{merge:true}` which is IGNORED (setDoc = replace per contract). Each write clobbers other fields on activeDisplay/status doc. @@ -108,6 +108,13 @@ REWORK_PLAN.md. activeEncounterId with null (setDoc replace vs updateDoc patch). - Fix: use updateDoc (patch) not setDoc (replace); or include all existing fields when writing. +- Status update (2026-07): all 5 sites now use `{merge:true}`. Real firebase + adapter honors merge → production works. BUT jsdom test still RED because + `src/__mocks__/firebase/firestore.js` setDoc records call, IGNORES opts + (no actual merge). Mock must simulate firebase merge semantics for test + to pass. Fix = mock setDoc: if opts.merge, MOCK_DB.merge(path,data) else + replace. OR change App.js setDoc(merge) → updateDoc (cleaner, ws adapter + uses PATCH). Decide which. - Test: render App + DisplayView, toggle hide-HP, assert display still shows encounter (not paused). diff --git a/src/App.js b/src/App.js index 97cd7a8..d296f2a 100644 --- a/src/App.js +++ b/src/App.js @@ -1662,7 +1662,7 @@ function InitiativeControls({ campaignId, encounter, encounterPath }) { const handleToggleHidePlayerHp = async () => { if (!db) return; try { - await storage.setDoc(getPath.activeDisplay(), { hidePlayerHp: !hidePlayerHp }, { merge: true }); + await storage.updateDoc(getPath.activeDisplay(), { hidePlayerHp: !hidePlayerHp }); } catch (err) { console.error("Error toggling hidePlayerHp:", err); } @@ -1695,10 +1695,10 @@ function InitiativeControls({ campaignId, encounter, encounterPath }) { turnOrderIds: sortedParticipants.map(p => p.id) }); - await storage.setDoc(getPath.activeDisplay(), { + await storage.updateDoc(getPath.activeDisplay(), { activeCampaignId: campaignId, activeEncounterId: encounter.id - }, { merge: true }); + }); logAction(`Combat started: "${encounter.name}" — ${sortedParticipants[0].name}'s turn (Round 1)`, { encounterName: encounter.name }, { encounterPath, @@ -1826,10 +1826,10 @@ function InitiativeControls({ campaignId, encounter, encounterPath }) { turnOrderIds: [] }); - await storage.setDoc(getPath.activeDisplay(), { + await storage.updateDoc(getPath.activeDisplay(), { activeCampaignId: null, activeEncounterId: null - }, { merge: true }); + }); logAction(`Combat ended: "${encounter.name}"`, { encounterName: encounter.name }, { encounterPath, @@ -2044,15 +2044,15 @@ function EncounterManager({ campaignId, initialActiveEncounterId, campaignCharac const currentActiveEncounter = activeDisplayInfo?.activeEncounterId; if (currentActiveCampaign === campaignId && currentActiveEncounter === encounterId) { - await storage.setDoc(getPath.activeDisplay(), { + await storage.updateDoc(getPath.activeDisplay(), { activeCampaignId: null, activeEncounterId: null, - }, { merge: true }); + }); } else { - await storage.setDoc(getPath.activeDisplay(), { + await storage.updateDoc(getPath.activeDisplay(), { activeCampaignId: campaignId, activeEncounterId: encounterId, - }, { merge: true }); + }); } } catch (err) { console.error("Error toggling Player Display:", err); diff --git a/src/tests/Combat.characterization.test.js b/src/tests/Combat.characterization.test.js index 9a4a852..618762d 100644 --- a/src/tests/Combat.characterization.test.js +++ b/src/tests/Combat.characterization.test.js @@ -41,7 +41,7 @@ describe('Combat -> Firebase', () => { test('startEncounter: also sets activeDisplay to this encounter', async () => { await setupWithMonsters(); await startCombatViaUI(); - const adCalls = findCallActiveDisplay('setDoc'); + const adCalls = findCallActiveDisplay('updateDoc'); const last = adCalls[adCalls.length - 1]; expect(last.data.activeCampaignId).toBeTruthy(); expect(last.data.activeEncounterId).toBeTruthy(); @@ -111,26 +111,26 @@ describe('Combat -> Firebase', () => { fireEvent.click(screen.getByRole('button', { name: /End Combat/i })); fireEvent.click(await screen.findByRole('button', { name: /Confirm/i })); await waitFor(() => { - const adCalls = findCallActiveDisplay('setDoc'); + const adCalls = findCallActiveDisplay('updateDoc'); const last = adCalls[adCalls.length - 1]; return last && last.data.activeCampaignId === null; }); - const adCalls = findCallActiveDisplay('setDoc'); + const adCalls = findCallActiveDisplay('updateDoc'); const last = adCalls[adCalls.length - 1]; expect(last.data).toMatchObject({ activeCampaignId: null, activeEncounterId: null }); }); - test('toggleHidePlayerHp: setDoc merge on activeDisplay/status', async () => { + test('toggleHidePlayerHp: updateDoc patch on activeDisplay/status', async () => { await setupWithMonsters(); await startCombatViaUI(); const switchBtn = screen.getByRole('switch'); fireEvent.click(switchBtn); await waitFor(() => { - const adCalls = findCallActiveDisplay('setDoc'); + const adCalls = findCallActiveDisplay('updateDoc'); const last = adCalls[adCalls.length - 1]; return last && 'hidePlayerHp' in last.data; }); - const adCalls = findCallActiveDisplay('setDoc'); + const adCalls = findCallActiveDisplay('updateDoc'); const last = adCalls[adCalls.length - 1]; expect(last.data).toHaveProperty('hidePlayerHp'); }); diff --git a/src/tests/Encounter.characterization.test.js b/src/tests/Encounter.characterization.test.js index 4a9e959..acaab44 100644 --- a/src/tests/Encounter.characterization.test.js +++ b/src/tests/Encounter.characterization.test.js @@ -42,7 +42,7 @@ describe('Encounter -> Firebase', () => { expect(call.path).toMatch(/campaigns\/[^/]+\/encounters\//); }); - test('togglePlayerDisplay: setDoc merge on activeDisplay/status', async () => { + test('togglePlayerDisplay: updateDoc patch on activeDisplay/status', async () => { await setupCampaignAndEncounter('Camp D', 'Enc D'); await selectEncounterByName('Enc D'); @@ -50,33 +50,33 @@ describe('Encounter -> Firebase', () => { const eyeBtn = await screen.findByTitle('Activate for Player Display'); fireEvent.click(eyeBtn); - await waitFor(() => findCall('setDoc', 'activeDisplay/status')); - const call = findCall('setDoc', 'activeDisplay/status'); - // activeDisplay/status setDoc is called with merge option in App + await waitFor(() => findCall('updateDoc', 'activeDisplay/status')); + const call = findCall('updateDoc', 'activeDisplay/status'); + // BUG-4 fix: updateDoc patch, not setDoc replace (was clobbering fields) expect(call.data).toMatchObject({ activeCampaignId: expect.any(String), activeEncounterId: expect.any(String), }); }); - test('togglePlayerDisplay off: setDoc nulls active ids', async () => { + test('togglePlayerDisplay off: updateDoc nulls active ids', async () => { await setupCampaignAndEncounter('Camp O', 'Enc O'); await selectEncounterByName('Enc O'); // turn ON const onBtn = await screen.findByTitle('Activate for Player Display'); fireEvent.click(onBtn); - await waitFor(() => findCall('setDoc', 'activeDisplay/status')); + await waitFor(() => findCall('updateDoc', 'activeDisplay/status')); // turn OFF const offBtn = await screen.findByTitle('Deactivate for Player Display'); fireEvent.click(offBtn); await waitFor(() => { - const calls = findCalls('setDoc', 'activeDisplay/status'); + const calls = findCalls('updateDoc', 'activeDisplay/status'); const last = calls[calls.length - 1]; return last.data.activeCampaignId === null; }); - const calls = findCalls('setDoc', 'activeDisplay/status'); + const calls = findCalls('updateDoc', 'activeDisplay/status'); const last = calls[calls.length - 1]; expect(last.data).toMatchObject({ activeCampaignId: null, activeEncounterId: null }); }); @@ -103,7 +103,7 @@ describe('Encounter -> Firebase', () => { // activate display first const onBtn = await screen.findByTitle('Activate for Player Display'); fireEvent.click(onBtn); - await waitFor(() => findCall('setDoc', 'activeDisplay/status')); + await waitFor(() => findCall('updateDoc', 'activeDisplay/status')); // delete the active encounter const trashBtn = screen.getAllByTitle('Delete Encounter')[0]; diff --git a/src/tests/HideHpToggle.test.js b/src/tests/HideHpToggle.test.js index 0e44614..4dee45e 100644 --- a/src/tests/HideHpToggle.test.js +++ b/src/tests/HideHpToggle.test.js @@ -50,14 +50,14 @@ describe('BUG-4: hide-player-HP toggle preserves activeDisplay', () => { await waitFor(() => { const writes = getAdapterCalls().filter( - c => c.fn === 'setDoc' && c.path.includes('activeDisplay/status') + c => c.fn === 'updateDoc' && c.path.includes('activeDisplay/status') ); expect(writes.length).toBeGreaterThan(0); const last = writes[writes.length - 1]; - // data written must include activeCampaignId AND activeEncounterId - // BUG: writes only {hidePlayerHp:true}, clobbering them. - expect(last.data.activeCampaignId).toBe('c1'); - expect(last.data.activeEncounterId).toBe('e1'); + // patch must NOT clobber activeCampaignId/activeEncounterId. + // BUG: setDoc replace writes only {hidePlayerHp:true} → clobbers. + // Fix: updateDoc patch — other fields untouched. + expect(last.patch.hidePlayerHp).toBe(true); }, { timeout: 3000 }); }); });