fix(turn): log addParticipants + updateParticipant (logging gaps)
addParticipants + updateParticipant returned log:null → invisible in
combat log. Contract test caught them. Now both return
log:{ message, undo:{ participants }}.
App.js handlers wired (handleAddAllCharacters, handleUpdateParticipant,
handleInlineInitiative) — call logAction on logged mutations.
Logging contract now complete: every mutating combat op produces a log
entry. No gaps. turn.logging.test.js proves it.
238 tests green.
This commit is contained in:
@@ -166,23 +166,38 @@ describe('Logging undo payloads', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Logging: gaps (currently unlogged mutations)', () => {
|
||||
// Document current behavior. addParticipants + updateParticipant return
|
||||
// null log → invisible in combat log. May be intended (bulk/no-op) or bug.
|
||||
describe('Logging: addParticipants + updateParticipant', () => {
|
||||
let e;
|
||||
beforeEach(() => {
|
||||
e = enc([p('a', 10), p('b', 7)]);
|
||||
});
|
||||
|
||||
test('addParticipants returns null log (bulk gap?)', () => {
|
||||
test('addParticipants logs', () => {
|
||||
const r = addParticipants(e, [p('c', 3)]);
|
||||
// Currently null. Document so decision is explicit.
|
||||
expect(r.log).toBeNull();
|
||||
expectLogged(r);
|
||||
});
|
||||
|
||||
test('updateParticipant returns null log (edit gap?)', () => {
|
||||
test('updateParticipant (same slot) logs', () => {
|
||||
const r = updateParticipant(e, 'b', { name: 'B' });
|
||||
// Currently null. Document so decision is explicit.
|
||||
expect(r.log).toBeNull();
|
||||
expectLogged(r);
|
||||
});
|
||||
|
||||
test('updateParticipant (init change) logs', () => {
|
||||
const r = updateParticipant(e, 'b', { initiative: 5 });
|
||||
expectLogged(r);
|
||||
});
|
||||
|
||||
test('addParticipants undo restores prior list', () => {
|
||||
const orig = e.participants.map(p => p.id);
|
||||
const r = addParticipants(e, [p('c', 3)]);
|
||||
const restored = { ...e, ...r.log.undo };
|
||||
expect(restored.participants.map(p => p.id)).toEqual(orig);
|
||||
});
|
||||
|
||||
test('updateParticipant undo restores prior participant', () => {
|
||||
const orig = e.participants.map(p => p.id);
|
||||
const r = updateParticipant(e, 'b', { name: 'B' });
|
||||
const restored = { ...e, ...r.log.undo };
|
||||
expect(restored.participants.map(p => p.id)).toEqual(orig);
|
||||
});
|
||||
});
|
||||
|
||||
+25
-3
@@ -333,8 +333,16 @@ function addParticipant(encounter, participant) {
|
||||
|
||||
// ADD_PARTICIPANTS — bulk add (e.g. "add all campaign characters").
|
||||
function addParticipants(encounter, newParticipants) {
|
||||
const undo = { participants: encounter.participants || [] };
|
||||
const updatedParticipants = [...(encounter.participants || []), ...newParticipants];
|
||||
return { patch: { participants: updatedParticipants }, log: null };
|
||||
const names = newParticipants.map(p => p.name).join(', ');
|
||||
return {
|
||||
patch: { participants: updatedParticipants },
|
||||
log: {
|
||||
message: `Added ${newParticipants.length} participant${newParticipants.length === 1 ? '' : 's'}: ${names}`,
|
||||
undo,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// UPDATE_PARTICIPANT — edit modal save (name/initiative/hp/isNpc).
|
||||
@@ -346,14 +354,28 @@ function updateParticipant(encounter, participantId, updatedData) {
|
||||
// SLOT only if initiative changed. Other edits (name/hp/notes/isNpc) = no
|
||||
// position change (design doc). Re-slots on unrelated edits shuffle the
|
||||
// list mid-round → rotation dupes.
|
||||
const undo = { participants: encounter.participants || [] };
|
||||
if (!('initiative' in updatedData) || updatedData.initiative === target.initiative) {
|
||||
const sameSlot = existing.map(p => p.id === participantId ? merged : p);
|
||||
return { patch: { participants: sameSlot, ...syncTurnOrder(sameSlot) }, log: null };
|
||||
const fields = Object.keys(updatedData).join(', ');
|
||||
return {
|
||||
patch: { participants: sameSlot, ...syncTurnOrder(sameSlot) },
|
||||
log: {
|
||||
message: `${target.name} updated (${fields})`,
|
||||
undo,
|
||||
},
|
||||
};
|
||||
}
|
||||
const without = existing.filter(p => p.id !== participantId);
|
||||
const idx = slotIndexForInit(without, merged.initiative);
|
||||
without.splice(idx, 0, merged);
|
||||
return { patch: { participants: without, ...syncTurnOrder(without) }, log: null };
|
||||
return {
|
||||
patch: { participants: without, ...syncTurnOrder(without) },
|
||||
log: {
|
||||
message: `${target.name} updated (initiative: ${merged.initiative})`,
|
||||
undo,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// REMOVE_PARTICIPANT — verbatim from ParticipantManager.confirmDeleteParticipant
|
||||
|
||||
Reference in New Issue
Block a user