From dbd0c75792bbfe75c381a562279e9842de9204a4 Mon Sep 17 00:00:00 2001 From: david raistrick <1108844+keen99@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:05:00 -0400 Subject: [PATCH] fix(BUG-12): campaign selection follows activeDisplay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selection effect had `!selectedCampaignId` guard — once any campaign selected, new activeDisplay.activeCampaignId writes ignored. Replay tool writes activeDisplay to new campaign each run; UI stayed on old selection => displayed wrong campaign data. Removed guard. Selection now syncs when activeCampaignId differs from current selection. Manual deselect (null) does not force-select (RED test locks this). Also fixed test helper bug: createCampaignViaUI/createEncounterViaUI returned FIRST setDoc match (idA for all creates). Now filters by name + .pop() for latest. This masked the real bug for several debug cycles. Tests: 2 new (SelectionFollowsActiveDisplay), both green. No regressions in full FE suite (App, Combat, DisplayView, Encounter, HideHpToggle, Logs, Participant, storage all pass). Combat.scenario = pre-existing BUG-11 crash, not regression. --- src/App.js | 5 +- .../SelectionFollowsActiveDisplay.test.js | 49 +++++++++++++++++++ src/tests/testHelpers.js | 10 ++-- 3 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 src/tests/SelectionFollowsActiveDisplay.test.js diff --git a/src/App.js b/src/App.js index 9b2e697..8305b07 100644 --- a/src/App.js +++ b/src/App.js @@ -2151,11 +2151,10 @@ function AdminView({ userId }) { if ( initialActiveInfo && initialActiveInfo.activeCampaignId && - campaignsWithDetails.length > 0 && - !selectedCampaignId + campaignsWithDetails.length > 0 ) { const campaignExists = campaignsWithDetails.some(c => c.id === initialActiveInfo.activeCampaignId); - if (campaignExists) { + if (campaignExists && selectedCampaignId !== initialActiveInfo.activeCampaignId) { setSelectedCampaignId(initialActiveInfo.activeCampaignId); } } diff --git a/src/tests/SelectionFollowsActiveDisplay.test.js b/src/tests/SelectionFollowsActiveDisplay.test.js new file mode 100644 index 0000000..9586697 --- /dev/null +++ b/src/tests/SelectionFollowsActiveDisplay.test.js @@ -0,0 +1,49 @@ +// RED test: campaign selection must follow activeDisplay.activeCampaignId changes. +// Bug: once selected, new activeDisplay writes ignored (guard `!selectedCampaignId`). +// Scenario: replay tool writes activeDisplay to new campaign -> UI must switch. +import React from 'react'; +import { screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { MOCK_DB } from '../__mocks__/firebase/_mock-db'; +import { renderApp, createCampaignViaUI, selectCampaignByName } from './testHelpers'; + +const PUBLIC_DATA = 'artifacts/ttrpg-initiative-tracker-default/public/data'; + +describe('Selection follows activeDisplay (BUG-12)', () => { + test('new activeCampaignId switches selection', async () => { + await renderApp(); + const idA = await createCampaignViaUI('Campaign A'); + const idB = await createCampaignViaUI('Campaign B'); + + // seed activeDisplay so useFirestoreDocument has a value to emit + const activePath = Object.keys(MOCK_DB._state.docs).find(p => p.includes('/activeDisplay/status')) + || `${PUBLIC_DATA}/activeDisplay/status`; + MOCK_DB.set(activePath, { activeCampaignId: null, activeEncounterId: null }); + + // manually select A first + await selectCampaignByName('Campaign A'); + expect(screen.getByText(/Managing:/i).textContent).toMatch(/Campaign A/); + + // simulate replay/another-DM setting activeDisplay to B + MOCK_DB.merge(activePath, { activeCampaignId: idB }); + + // selection should now follow -> Managing: Campaign B + await waitFor(() => { + expect(screen.getByText(/Managing:/i).textContent).toMatch(/Campaign B/); + }); + }); + + test('activeDisplay cleared (null) does not force-select', async () => { + await renderApp(); + const idA = await createCampaignViaUI('Persist A'); + const activePath = Object.keys(MOCK_DB._state.docs).find(p => p.includes('/activeDisplay/status')) + || `${PUBLIC_DATA}/activeDisplay/status`; + MOCK_DB.set(activePath, { activeCampaignId: null, activeEncounterId: null }); + await selectCampaignByName('Persist A'); + + MOCK_DB.merge(activePath, { activeCampaignId: null }); + + // should stay on A (manual selection persists) + expect(screen.getByText(/Managing:/i).textContent).toMatch(/Persist A/); + }); +}); diff --git a/src/tests/testHelpers.js b/src/tests/testHelpers.js index 4373afa..3282aa2 100644 --- a/src/tests/testHelpers.js +++ b/src/tests/testHelpers.js @@ -33,10 +33,10 @@ export async function createCampaignViaUI(name = 'Test Campaign') { await waitFor(() => screen.getByLabelText(/Campaign Name/i)); fireEvent.change(screen.getByLabelText(/Campaign Name/i), { target: { value: name } }); fireEvent.click(screen.getByRole('button', { name: /^Create$/i })); - // wait for setDoc recorded + // wait for setDoc recorded with this name (latest match) const { getCalls } = require('../__mocks__/firebase/_mock-db'); - await waitFor(() => getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/campaigns/'))); - const call = getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/campaigns/')); + await waitFor(() => getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/campaigns/') && c.data.name === name)); + const call = getCalls().filter(c => c.fn === 'setDoc' && c.path.includes('/campaigns/') && c.data.name === name).pop(); return call.path.split('/').pop(); // campaign id } @@ -54,8 +54,8 @@ export async function createEncounterViaUI(name = 'Test Encounter') { fireEvent.change(screen.getByLabelText(/Encounter Name/i), { target: { value: name } }); fireEvent.click(screen.getByRole('button', { name: /^Create$/i })); const { getCalls } = require('../__mocks__/firebase/_mock-db'); - await waitFor(() => getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/encounters/'))); - const call = getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/encounters/')); + await waitFor(() => getCalls().find(c => c.fn === 'setDoc' && c.path.includes('/encounters/') && c.data.name === name)); + const call = getCalls().filter(c => c.fn === 'setDoc' && c.path.includes('/encounters/') && c.data.name === name).pop(); return call.path.split('/').pop(); }