mirror of
https://github.com/OFFIS-ESC/constellation-analyzer
synced 2026-01-27 07:43:41 +00:00
refactor: move snapshot creation to historyStore (architectural fix)
Fixes architectural issue where snapshot logic was duplicated between hook and store layers. Problem Identified: - useDocumentHistory (hook) created snapshots with duplicate logic - timelineStore (store) created snapshots with duplicate logic - timelineStore couldn't call useDocumentHistory (hooks can't be used in stores) - Snapshot creation logic scattered across codebase Root Cause: Timeline operations happen in the store layer but history was managed via a hook. This architectural mismatch forced duplication. Solution: - Add pushToHistory() method to historyStore - Single source of truth for snapshot creation - Both hook and store layers call historyStore.pushToHistory() - Eliminates all snapshot creation duplication Changes: - Add historyStore.pushToHistory() - high-level API with snapshot creation - Keep historyStore.pushAction() - low-level API (for future use) - Update useDocumentHistory to call historyStore.pushToHistory() - Update timelineStore pushDocumentHistory() to call historyStore.pushToHistory() - Remove createDocumentSnapshot import from useDocumentHistory - Remove createDocumentSnapshot import from timelineStore Architecture: Before: Hook (useDocumentHistory) → creates snapshot → historyStore.pushAction() Store (timelineStore) → creates snapshot → historyStore.pushAction() [Duplication at 2 call sites] After: Hook (useDocumentHistory) → historyStore.pushToHistory() → creates snapshot Store (timelineStore) → historyStore.pushToHistory() → creates snapshot [Single implementation in historyStore] Benefits: - ✅ Zero snapshot creation duplication - ✅ Proper architectural separation (store handles snapshots, not hook/store) - ✅ Single source of truth (historyStore.pushToHistory) - ✅ Timeline and graph operations use identical snapshot logic - ✅ Easy to modify snapshot behavior in future (one place) Impact: - No breaking changes - No behavior changes - Better code organization - Easier maintenance Related: Phase 2.1 architectural improvement 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
0ef81260cc
commit
6a56b94477
3 changed files with 55 additions and 32 deletions
|
|
@ -4,7 +4,6 @@ import { useWorkspaceStore } from '../stores/workspaceStore';
|
||||||
import { useHistoryStore } from '../stores/historyStore';
|
import { useHistoryStore } from '../stores/historyStore';
|
||||||
import { useGraphStore } from '../stores/graphStore';
|
import { useGraphStore } from '../stores/graphStore';
|
||||||
import { useTimelineStore } from '../stores/timelineStore';
|
import { useTimelineStore } from '../stores/timelineStore';
|
||||||
import { createDocumentSnapshot } from '../stores/workspace/documentUtils';
|
|
||||||
import type { DocumentSnapshot } from '../stores/historyStore';
|
import type { DocumentSnapshot } from '../stores/historyStore';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -59,25 +58,14 @@ export function useDocumentHistory() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ✅ Use centralized snapshot creation (single source of truth)
|
// ✅ Call historyStore's high-level pushToHistory (single source of truth)
|
||||||
const snapshot = createDocumentSnapshot(
|
historyStore.pushToHistory(
|
||||||
activeDocumentId,
|
activeDocumentId,
|
||||||
|
description,
|
||||||
activeDoc,
|
activeDoc,
|
||||||
timeline,
|
timeline,
|
||||||
graphStore
|
graphStore
|
||||||
);
|
);
|
||||||
|
|
||||||
if (!snapshot) {
|
|
||||||
console.warn('Failed to create snapshot');
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Push to history
|
|
||||||
historyStore.pushAction(activeDocumentId, {
|
|
||||||
description,
|
|
||||||
timestamp: Date.now(),
|
|
||||||
documentState: snapshot,
|
|
||||||
});
|
|
||||||
},
|
},
|
||||||
[activeDocumentId, historyStore]
|
[activeDocumentId, historyStore]
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,9 @@
|
||||||
import { create } from "zustand";
|
import { create } from "zustand";
|
||||||
import type { NodeTypeConfig, EdgeTypeConfig, LabelConfig } from "../types";
|
import type { NodeTypeConfig, EdgeTypeConfig, LabelConfig } from "../types";
|
||||||
import type { ConstellationState, StateId } from "../types/timeline";
|
import type { ConstellationState, StateId, Timeline } from "../types/timeline";
|
||||||
|
import type { Actor, Relation, Group } from "../types";
|
||||||
|
import type { ConstellationDocument } from "./persistence/types";
|
||||||
|
import { createDocumentSnapshot } from "./workspace/documentUtils";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* History Store - Per-Document Undo/Redo System
|
* History Store - Per-Document Undo/Redo System
|
||||||
|
|
@ -52,9 +55,18 @@ interface HistoryActions {
|
||||||
// Initialize history for a document
|
// Initialize history for a document
|
||||||
initializeHistory: (documentId: string) => void;
|
initializeHistory: (documentId: string) => void;
|
||||||
|
|
||||||
// Push a new action onto the document's history stack
|
// Push a new action onto the document's history stack (low-level)
|
||||||
pushAction: (documentId: string, action: HistoryAction) => void;
|
pushAction: (documentId: string, action: HistoryAction) => void;
|
||||||
|
|
||||||
|
// Push action with automatic snapshot creation (high-level - USE THIS)
|
||||||
|
pushToHistory: (
|
||||||
|
documentId: string,
|
||||||
|
description: string,
|
||||||
|
document: ConstellationDocument,
|
||||||
|
timeline: Timeline,
|
||||||
|
graphStore: { nodes: Actor[]; edges: Relation[]; groups: Group[] }
|
||||||
|
) => void;
|
||||||
|
|
||||||
// Undo the last action for a specific document
|
// Undo the last action for a specific document
|
||||||
undo: (documentId: string, currentSnapshot: DocumentSnapshot) => DocumentSnapshot | null;
|
undo: (documentId: string, currentSnapshot: DocumentSnapshot) => DocumentSnapshot | null;
|
||||||
|
|
||||||
|
|
@ -163,6 +175,34 @@ export const useHistoryStore = create<HistoryStore & HistoryActions>(
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
|
pushToHistory: (
|
||||||
|
documentId: string,
|
||||||
|
description: string,
|
||||||
|
document: ConstellationDocument,
|
||||||
|
timeline: Timeline,
|
||||||
|
graphStore: { nodes: Actor[]; edges: Relation[]; groups: Group[] }
|
||||||
|
) => {
|
||||||
|
// ✅ Use centralized snapshot creation (single source of truth)
|
||||||
|
const snapshot = createDocumentSnapshot(
|
||||||
|
documentId,
|
||||||
|
document,
|
||||||
|
timeline,
|
||||||
|
graphStore
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!snapshot) {
|
||||||
|
console.warn('Failed to create snapshot for history');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call low-level pushAction
|
||||||
|
get().pushAction(documentId, {
|
||||||
|
description,
|
||||||
|
timestamp: Date.now(),
|
||||||
|
documentState: snapshot,
|
||||||
|
});
|
||||||
|
},
|
||||||
|
|
||||||
undo: (documentId: string, currentSnapshot: DocumentSnapshot) => {
|
undo: (documentId: string, currentSnapshot: DocumentSnapshot) => {
|
||||||
const state = get();
|
const state = get();
|
||||||
const history = state.histories.get(documentId);
|
const history = state.histories.get(documentId);
|
||||||
|
|
|
||||||
|
|
@ -11,7 +11,6 @@ import { useGraphStore } from "./graphStore";
|
||||||
import { useWorkspaceStore } from "./workspaceStore";
|
import { useWorkspaceStore } from "./workspaceStore";
|
||||||
import { useToastStore } from "./toastStore";
|
import { useToastStore } from "./toastStore";
|
||||||
import { useHistoryStore } from "./historyStore";
|
import { useHistoryStore } from "./historyStore";
|
||||||
import { createDocumentSnapshot } from "./workspace/documentUtils";
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Timeline Store
|
* Timeline Store
|
||||||
|
|
@ -33,7 +32,13 @@ function generateStateId(): StateId {
|
||||||
return `state_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
|
return `state_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Helper to push document state to history
|
/**
|
||||||
|
* Helper to push document state to history from timeline operations
|
||||||
|
*
|
||||||
|
* This is a convenience wrapper that gathers the required state and calls
|
||||||
|
* historyStore.pushToHistory(). Timeline operations use this to track changes
|
||||||
|
* in the unified document history system.
|
||||||
|
*/
|
||||||
function pushDocumentHistory(documentId: string, description: string) {
|
function pushDocumentHistory(documentId: string, description: string) {
|
||||||
const historyStore = useHistoryStore.getState();
|
const historyStore = useHistoryStore.getState();
|
||||||
const timelineStore = useTimelineStore.getState();
|
const timelineStore = useTimelineStore.getState();
|
||||||
|
|
@ -48,24 +53,14 @@ function pushDocumentHistory(documentId: string, description: string) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ✅ Use centralized snapshot creation (single source of truth)
|
// ✅ Call historyStore's high-level pushToHistory (single source of truth for snapshots)
|
||||||
const snapshot = createDocumentSnapshot(
|
historyStore.pushToHistory(
|
||||||
documentId,
|
documentId,
|
||||||
|
description,
|
||||||
document,
|
document,
|
||||||
timeline,
|
timeline,
|
||||||
graphStore
|
graphStore
|
||||||
);
|
);
|
||||||
|
|
||||||
if (!snapshot) {
|
|
||||||
console.warn('Failed to create snapshot');
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
historyStore.pushAction(documentId, {
|
|
||||||
description,
|
|
||||||
timestamp: Date.now(),
|
|
||||||
documentState: snapshot,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export const useTimelineStore = create<TimelineStore & TimelineActions>(
|
export const useTimelineStore = create<TimelineStore & TimelineActions>(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue