mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
fix(frontend): prevent duplicate sub-conversation creation on page refresh (planning agent) (#12645)
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import {
|
||||
clearConversationLocalStorage,
|
||||
getConversationState,
|
||||
setConversationState,
|
||||
LOCAL_STORAGE_KEYS,
|
||||
} from "#/utils/conversation-local-storage";
|
||||
|
||||
@@ -35,4 +37,125 @@ describe("conversation localStorage utilities", () => {
|
||||
}).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("getConversationState", () => {
|
||||
it("returns default state with subConversationTaskId as null when no state exists", () => {
|
||||
const conversationId = "conv-123";
|
||||
const state = getConversationState(conversationId);
|
||||
|
||||
expect(state.subConversationTaskId).toBeNull();
|
||||
expect(state.selectedTab).toBe("editor");
|
||||
expect(state.rightPanelShown).toBe(true);
|
||||
expect(state.unpinnedTabs).toEqual([]);
|
||||
});
|
||||
|
||||
it("retrieves subConversationTaskId from localStorage when it exists", () => {
|
||||
const conversationId = "conv-123";
|
||||
const taskId = "task-uuid-123";
|
||||
const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`;
|
||||
|
||||
localStorage.setItem(
|
||||
consolidatedKey,
|
||||
JSON.stringify({
|
||||
selectedTab: "editor",
|
||||
rightPanelShown: true,
|
||||
unpinnedTabs: [],
|
||||
subConversationTaskId: taskId,
|
||||
}),
|
||||
);
|
||||
|
||||
const state = getConversationState(conversationId);
|
||||
|
||||
expect(state.subConversationTaskId).toBe(taskId);
|
||||
});
|
||||
|
||||
it("merges stored state with defaults when partial state exists", () => {
|
||||
const conversationId = "conv-123";
|
||||
const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`;
|
||||
|
||||
localStorage.setItem(
|
||||
consolidatedKey,
|
||||
JSON.stringify({
|
||||
subConversationTaskId: "task-123",
|
||||
}),
|
||||
);
|
||||
|
||||
const state = getConversationState(conversationId);
|
||||
|
||||
expect(state.subConversationTaskId).toBe("task-123");
|
||||
expect(state.selectedTab).toBe("editor");
|
||||
expect(state.rightPanelShown).toBe(true);
|
||||
expect(state.unpinnedTabs).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setConversationState", () => {
|
||||
it("persists subConversationTaskId to localStorage", () => {
|
||||
const conversationId = "conv-123";
|
||||
const taskId = "task-uuid-456";
|
||||
const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`;
|
||||
|
||||
setConversationState(conversationId, {
|
||||
subConversationTaskId: taskId,
|
||||
});
|
||||
|
||||
const stored = localStorage.getItem(consolidatedKey);
|
||||
expect(stored).not.toBeNull();
|
||||
|
||||
const parsed = JSON.parse(stored!);
|
||||
expect(parsed.subConversationTaskId).toBe(taskId);
|
||||
});
|
||||
|
||||
it("merges subConversationTaskId with existing state", () => {
|
||||
const conversationId = "conv-123";
|
||||
const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`;
|
||||
|
||||
// Set initial state
|
||||
localStorage.setItem(
|
||||
consolidatedKey,
|
||||
JSON.stringify({
|
||||
selectedTab: "changes",
|
||||
rightPanelShown: false,
|
||||
unpinnedTabs: ["tab-1"],
|
||||
subConversationTaskId: "old-task-id",
|
||||
}),
|
||||
);
|
||||
|
||||
// Update only subConversationTaskId
|
||||
setConversationState(conversationId, {
|
||||
subConversationTaskId: "new-task-id",
|
||||
});
|
||||
|
||||
const stored = localStorage.getItem(consolidatedKey);
|
||||
const parsed = JSON.parse(stored!);
|
||||
|
||||
expect(parsed.subConversationTaskId).toBe("new-task-id");
|
||||
expect(parsed.selectedTab).toBe("changes");
|
||||
expect(parsed.rightPanelShown).toBe(false);
|
||||
expect(parsed.unpinnedTabs).toEqual(["tab-1"]);
|
||||
});
|
||||
|
||||
it("clears subConversationTaskId when set to null", () => {
|
||||
const conversationId = "conv-123";
|
||||
const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`;
|
||||
|
||||
// Set initial state with task ID
|
||||
localStorage.setItem(
|
||||
consolidatedKey,
|
||||
JSON.stringify({
|
||||
subConversationTaskId: "task-123",
|
||||
}),
|
||||
);
|
||||
|
||||
// Clear the task ID
|
||||
setConversationState(conversationId, {
|
||||
subConversationTaskId: null,
|
||||
});
|
||||
|
||||
const stored = localStorage.getItem(consolidatedKey);
|
||||
const parsed = JSON.parse(stored!);
|
||||
|
||||
expect(parsed.subConversationTaskId).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
import { describe, it, expect, afterEach, vi, beforeEach } from "vitest";
|
||||
import React from "react";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { useSubConversationTaskPolling } from "#/hooks/query/use-sub-conversation-task-polling";
|
||||
import V1ConversationService from "#/api/conversation-service/v1-conversation-service.api";
|
||||
import { setConversationState } from "#/utils/conversation-local-storage";
|
||||
import { useConversationStore } from "#/stores/conversation-store";
|
||||
import type { V1AppConversationStartTask } from "#/api/conversation-service/v1-conversation-service.types";
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock("#/api/conversation-service/v1-conversation-service.api");
|
||||
vi.mock("#/utils/conversation-local-storage");
|
||||
vi.mock("#/stores/conversation-store");
|
||||
|
||||
const mockSetSubConversationTaskId = vi.fn();
|
||||
const mockInvalidateQueries = vi.fn();
|
||||
|
||||
// Helper function to create properly typed mock return values
|
||||
function asMockReturnValue<T>(value: Partial<T>): T {
|
||||
return value as T;
|
||||
}
|
||||
|
||||
function makeTask(
|
||||
status: V1AppConversationStartTask["status"],
|
||||
appConversationId: string | null = null,
|
||||
): V1AppConversationStartTask {
|
||||
return {
|
||||
id: "task-123",
|
||||
created_by_user_id: "user-123",
|
||||
status,
|
||||
detail: null,
|
||||
app_conversation_id: appConversationId,
|
||||
sandbox_id: null,
|
||||
agent_server_url: null,
|
||||
request: {
|
||||
agent_type: "plan",
|
||||
parent_conversation_id: "parent-conv-123",
|
||||
},
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
|
||||
function createQueryClient() {
|
||||
return new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: {
|
||||
retry: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function wrapper({ children }: { children: React.ReactNode }) {
|
||||
const queryClient = createQueryClient();
|
||||
queryClient.invalidateQueries = mockInvalidateQueries;
|
||||
return (
|
||||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
|
||||
);
|
||||
}
|
||||
|
||||
describe("useSubConversationTaskPolling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(useConversationStore).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useConversationStore>>({
|
||||
setSubConversationTaskId: mockSetSubConversationTaskId,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("when task status is READY", () => {
|
||||
it("clears subConversationTaskId from localStorage and store when task completes successfully", async () => {
|
||||
const parentConversationId = "parent-conv-123";
|
||||
const taskId = "task-123";
|
||||
const appConversationId = "sub-conv-456";
|
||||
|
||||
const readyTask = makeTask("READY", appConversationId);
|
||||
vi.mocked(V1ConversationService.getStartTask).mockResolvedValue(
|
||||
readyTask,
|
||||
);
|
||||
|
||||
renderHook(
|
||||
() => useSubConversationTaskPolling(taskId, parentConversationId),
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setConversationState).toHaveBeenCalledWith(
|
||||
parentConversationId,
|
||||
{ subConversationTaskId: null },
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockSetSubConversationTaskId).toHaveBeenCalledWith(null);
|
||||
expect(mockInvalidateQueries).toHaveBeenCalledWith({
|
||||
queryKey: ["user", "conversation", parentConversationId],
|
||||
});
|
||||
});
|
||||
|
||||
it("invalidates parent conversation cache when task is READY", async () => {
|
||||
const parentConversationId = "parent-conv-123";
|
||||
const taskId = "task-123";
|
||||
const appConversationId = "sub-conv-456";
|
||||
|
||||
const readyTask = makeTask("READY", appConversationId);
|
||||
vi.mocked(V1ConversationService.getStartTask).mockResolvedValue(
|
||||
readyTask,
|
||||
);
|
||||
|
||||
renderHook(
|
||||
() => useSubConversationTaskPolling(taskId, parentConversationId),
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockInvalidateQueries).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(mockInvalidateQueries).toHaveBeenCalledWith({
|
||||
queryKey: ["user", "conversation", parentConversationId],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("when task status is ERROR", () => {
|
||||
it("clears subConversationTaskId from localStorage and store on error", async () => {
|
||||
const parentConversationId = "parent-conv-123";
|
||||
const taskId = "task-123";
|
||||
|
||||
const errorTask = makeTask("ERROR", null);
|
||||
vi.mocked(V1ConversationService.getStartTask).mockResolvedValue(
|
||||
errorTask,
|
||||
);
|
||||
|
||||
renderHook(
|
||||
() => useSubConversationTaskPolling(taskId, parentConversationId),
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setConversationState).toHaveBeenCalledWith(
|
||||
parentConversationId,
|
||||
{ subConversationTaskId: null },
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockSetSubConversationTaskId).toHaveBeenCalledWith(null);
|
||||
expect(mockInvalidateQueries).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("when task is still in progress", () => {
|
||||
it("does not clear subConversationTaskId when task status is WORKING", async () => {
|
||||
const parentConversationId = "parent-conv-123";
|
||||
const taskId = "task-123";
|
||||
|
||||
const workingTask = makeTask("WORKING", null);
|
||||
vi.mocked(V1ConversationService.getStartTask).mockResolvedValue(
|
||||
workingTask,
|
||||
);
|
||||
|
||||
renderHook(
|
||||
() => useSubConversationTaskPolling(taskId, parentConversationId),
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
// Wait a bit to ensure useEffect has run
|
||||
await waitFor(() => {
|
||||
expect(V1ConversationService.getStartTask).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Should not clear anything while task is in progress
|
||||
expect(setConversationState).not.toHaveBeenCalled();
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not clear subConversationTaskId when task status is WAITING_FOR_SANDBOX", async () => {
|
||||
const parentConversationId = "parent-conv-123";
|
||||
const taskId = "task-123";
|
||||
|
||||
const waitingTask = makeTask("WAITING_FOR_SANDBOX", null);
|
||||
vi.mocked(V1ConversationService.getStartTask).mockResolvedValue(
|
||||
waitingTask,
|
||||
);
|
||||
|
||||
renderHook(
|
||||
() => useSubConversationTaskPolling(taskId, parentConversationId),
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(V1ConversationService.getStartTask).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(setConversationState).not.toHaveBeenCalled();
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("when parentConversationId is null", () => {
|
||||
it("does not clear subConversationTaskId or invalidate queries", () => {
|
||||
const taskId = "task-123";
|
||||
|
||||
renderHook(() => useSubConversationTaskPolling(taskId, null), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
// Query is disabled when parentConversationId is null, so getStartTask won't be called
|
||||
expect(V1ConversationService.getStartTask).not.toHaveBeenCalled();
|
||||
expect(setConversationState).not.toHaveBeenCalled();
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
expect(mockInvalidateQueries).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
330
frontend/__tests__/hooks/use-handle-plan-click.test.tsx
Normal file
330
frontend/__tests__/hooks/use-handle-plan-click.test.tsx
Normal file
@@ -0,0 +1,330 @@
|
||||
import { describe, it, expect, afterEach, vi, beforeEach } from "vitest";
|
||||
import { renderHook, waitFor, act } from "@testing-library/react";
|
||||
import { useHandlePlanClick } from "#/hooks/use-handle-plan-click";
|
||||
import { useConversationStore } from "#/stores/conversation-store";
|
||||
import { useActiveConversation } from "#/hooks/query/use-active-conversation";
|
||||
import { useCreateConversation } from "#/hooks/mutation/use-create-conversation";
|
||||
import {
|
||||
getConversationState,
|
||||
setConversationState,
|
||||
} from "#/utils/conversation-local-storage";
|
||||
import { displaySuccessToast } from "#/utils/custom-toast-handlers";
|
||||
import type { Conversation } from "#/api/open-hands.types";
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock("#/stores/conversation-store");
|
||||
vi.mock("#/hooks/query/use-active-conversation");
|
||||
vi.mock("#/hooks/mutation/use-create-conversation");
|
||||
vi.mock("#/utils/conversation-local-storage");
|
||||
vi.mock("#/utils/custom-toast-handlers");
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({
|
||||
t: (key: string) => key,
|
||||
}),
|
||||
}));
|
||||
|
||||
const mockSetConversationMode = vi.fn();
|
||||
const mockSetSubConversationTaskId = vi.fn();
|
||||
const mockCreateConversation = vi.fn();
|
||||
|
||||
// Helper function to create properly typed mock return values
|
||||
function asMockReturnValue<T>(value: Partial<T>): T {
|
||||
return value as T;
|
||||
}
|
||||
|
||||
function makeConversation(overrides?: Partial<Conversation>): Conversation {
|
||||
return {
|
||||
conversation_id: "conv-123",
|
||||
title: "Test Conversation",
|
||||
selected_repository: null,
|
||||
selected_branch: null,
|
||||
git_provider: null,
|
||||
last_updated_at: new Date().toISOString(),
|
||||
created_at: new Date().toISOString(),
|
||||
status: "RUNNING",
|
||||
runtime_status: null,
|
||||
url: null,
|
||||
session_api_key: null,
|
||||
conversation_version: "V1",
|
||||
sub_conversation_ids: [],
|
||||
...overrides,
|
||||
} as Conversation;
|
||||
}
|
||||
|
||||
describe("useHandlePlanClick", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
vi.mocked(useConversationStore).mockReturnValue({
|
||||
setConversationMode: mockSetConversationMode,
|
||||
setSubConversationTaskId: mockSetSubConversationTaskId,
|
||||
subConversationTaskId: null,
|
||||
});
|
||||
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: makeConversation(),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mocked(useCreateConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useCreateConversation>>({
|
||||
mutate: mockCreateConversation,
|
||||
isPending: false,
|
||||
isSuccess: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mocked(getConversationState).mockReturnValue({
|
||||
selectedTab: "editor",
|
||||
rightPanelShown: true,
|
||||
unpinnedTabs: [],
|
||||
subConversationTaskId: null,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("localStorage restoration", () => {
|
||||
it("restores subConversationTaskId from localStorage when conversation loads", () => {
|
||||
const conversationId = "conv-123";
|
||||
const storedTaskId = "task-456";
|
||||
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: makeConversation({ conversation_id: conversationId }),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mocked(getConversationState).mockReturnValue({
|
||||
selectedTab: "editor",
|
||||
rightPanelShown: true,
|
||||
unpinnedTabs: [],
|
||||
subConversationTaskId: storedTaskId,
|
||||
});
|
||||
|
||||
renderHook(() => useHandlePlanClick());
|
||||
|
||||
expect(getConversationState).toHaveBeenCalledWith(conversationId);
|
||||
expect(mockSetSubConversationTaskId).toHaveBeenCalledWith(storedTaskId);
|
||||
});
|
||||
|
||||
it("does not restore subConversationTaskId if it already exists in store", () => {
|
||||
const conversationId = "conv-123";
|
||||
const storedTaskId = "task-456";
|
||||
const existingTaskId = "task-789";
|
||||
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: makeConversation({ conversation_id: conversationId }),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mocked(useConversationStore).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useConversationStore>>({
|
||||
setConversationMode: mockSetConversationMode,
|
||||
setSubConversationTaskId: mockSetSubConversationTaskId,
|
||||
subConversationTaskId: existingTaskId,
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mocked(getConversationState).mockReturnValue({
|
||||
selectedTab: "editor",
|
||||
rightPanelShown: true,
|
||||
unpinnedTabs: [],
|
||||
subConversationTaskId: storedTaskId,
|
||||
});
|
||||
|
||||
renderHook(() => useHandlePlanClick());
|
||||
|
||||
expect(getConversationState).toHaveBeenCalledWith(conversationId);
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not restore subConversationTaskId when conversation is not loaded", () => {
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
renderHook(() => useHandlePlanClick());
|
||||
|
||||
expect(getConversationState).not.toHaveBeenCalled();
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("plan creation prevention", () => {
|
||||
it("prevents plan creation when subConversationTaskId exists in store", () => {
|
||||
const taskId = "task-123";
|
||||
|
||||
vi.mocked(useConversationStore).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useConversationStore>>({
|
||||
setConversationMode: mockSetConversationMode,
|
||||
setSubConversationTaskId: mockSetSubConversationTaskId,
|
||||
subConversationTaskId: taskId,
|
||||
}),
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useHandlePlanClick());
|
||||
|
||||
act(() => {
|
||||
result.current.handlePlanClick();
|
||||
});
|
||||
|
||||
expect(mockSetConversationMode).toHaveBeenCalledWith("plan");
|
||||
expect(mockCreateConversation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("prevents plan creation when conversation has existing sub_conversation_ids", () => {
|
||||
vi.mocked(useActiveConversation).mockReturnValue({
|
||||
data: makeConversation({
|
||||
sub_conversation_ids: ["sub-conv-1"],
|
||||
}),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
} as Partial<ReturnType<typeof useActiveConversation>> as ReturnType<
|
||||
typeof useActiveConversation
|
||||
>);
|
||||
|
||||
const { result } = renderHook(() => useHandlePlanClick());
|
||||
|
||||
act(() => {
|
||||
result.current.handlePlanClick();
|
||||
});
|
||||
|
||||
expect(mockSetConversationMode).toHaveBeenCalledWith("plan");
|
||||
expect(mockCreateConversation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("prevents plan creation when conversation_id is missing", () => {
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useHandlePlanClick());
|
||||
|
||||
act(() => {
|
||||
result.current.handlePlanClick();
|
||||
});
|
||||
|
||||
expect(mockSetConversationMode).toHaveBeenCalledWith("plan");
|
||||
expect(mockCreateConversation).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("plan creation and persistence", () => {
|
||||
it("creates plan conversation and persists subConversationTaskId to localStorage", () => {
|
||||
const conversationId = "conv-123";
|
||||
const taskId = "task-789";
|
||||
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: makeConversation({ conversation_id: conversationId }),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useHandlePlanClick());
|
||||
|
||||
act(() => {
|
||||
result.current.handlePlanClick();
|
||||
});
|
||||
|
||||
expect(mockSetConversationMode).toHaveBeenCalledWith("plan");
|
||||
expect(mockCreateConversation).toHaveBeenCalledWith(
|
||||
{
|
||||
parentConversationId: conversationId,
|
||||
agentType: "plan",
|
||||
},
|
||||
expect.objectContaining({
|
||||
onSuccess: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
|
||||
// Simulate successful conversation creation
|
||||
const onSuccessCallback = mockCreateConversation.mock.calls[0][1]
|
||||
.onSuccess as (data: { v1_task_id?: string }) => void;
|
||||
|
||||
act(() => {
|
||||
onSuccessCallback({ v1_task_id: taskId });
|
||||
});
|
||||
|
||||
expect(mockSetSubConversationTaskId).toHaveBeenCalledWith(taskId);
|
||||
expect(setConversationState).toHaveBeenCalledWith(conversationId, {
|
||||
subConversationTaskId: taskId,
|
||||
});
|
||||
expect(displaySuccessToast).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not persist subConversationTaskId when v1_task_id is missing", () => {
|
||||
const conversationId = "conv-123";
|
||||
|
||||
vi.mocked(useActiveConversation).mockReturnValue(
|
||||
asMockReturnValue<ReturnType<typeof useActiveConversation>>({
|
||||
data: makeConversation({ conversation_id: conversationId }),
|
||||
isLoading: false,
|
||||
isPending: false,
|
||||
isError: false,
|
||||
error: null,
|
||||
refetch: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useHandlePlanClick());
|
||||
|
||||
act(() => {
|
||||
result.current.handlePlanClick();
|
||||
});
|
||||
|
||||
const onSuccessCallback = mockCreateConversation.mock.calls[0][1]
|
||||
.onSuccess as (data: { v1_task_id?: string }) => void;
|
||||
|
||||
act(() => {
|
||||
onSuccessCallback({});
|
||||
});
|
||||
|
||||
expect(mockSetSubConversationTaskId).not.toHaveBeenCalled();
|
||||
expect(setConversationState).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,8 @@
|
||||
import { useEffect } from "react";
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import V1ConversationService from "#/api/conversation-service/v1-conversation-service.api";
|
||||
import { setConversationState } from "#/utils/conversation-local-storage";
|
||||
import { useConversationStore } from "#/stores/conversation-store";
|
||||
|
||||
/**
|
||||
* Hook that polls V1 sub-conversation start tasks and invalidates parent conversation cache when ready.
|
||||
@@ -22,6 +24,7 @@ export const useSubConversationTaskPolling = (
|
||||
parentConversationId: string | null,
|
||||
) => {
|
||||
const queryClient = useQueryClient();
|
||||
const { setSubConversationTaskId } = useConversationStore();
|
||||
|
||||
// Poll the task if we have both taskId and parentConversationId
|
||||
const taskQuery = useQuery({
|
||||
@@ -46,20 +49,34 @@ export const useSubConversationTaskPolling = (
|
||||
retry: false,
|
||||
});
|
||||
|
||||
// Invalidate parent conversation cache when task is ready
|
||||
// Invalidate parent conversation cache when task is ready and clear localStorage
|
||||
useEffect(() => {
|
||||
const task = taskQuery.data;
|
||||
if (
|
||||
task?.status === "READY" &&
|
||||
task.app_conversation_id &&
|
||||
parentConversationId
|
||||
) {
|
||||
if (!parentConversationId) return;
|
||||
|
||||
if (task?.status === "READY" && task.app_conversation_id) {
|
||||
// Invalidate the parent conversation to refetch with updated sub_conversation_ids
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: ["user", "conversation", parentConversationId],
|
||||
});
|
||||
// Clear the task ID from localStorage and store since task completed successfully
|
||||
setConversationState(parentConversationId, {
|
||||
subConversationTaskId: null,
|
||||
});
|
||||
setSubConversationTaskId(null);
|
||||
} else if (task?.status === "ERROR") {
|
||||
// Clear the task ID from localStorage and store on error so user can retry
|
||||
setConversationState(parentConversationId, {
|
||||
subConversationTaskId: null,
|
||||
});
|
||||
setSubConversationTaskId(null);
|
||||
}
|
||||
}, [taskQuery.data, parentConversationId, queryClient]);
|
||||
}, [
|
||||
taskQuery.data,
|
||||
parentConversationId,
|
||||
queryClient,
|
||||
setSubConversationTaskId,
|
||||
]);
|
||||
|
||||
return {
|
||||
task: taskQuery.data,
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
import { useCallback } from "react";
|
||||
import { useCallback, useEffect } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { I18nKey } from "#/i18n/declaration";
|
||||
import { useConversationStore } from "#/stores/conversation-store";
|
||||
import { useActiveConversation } from "#/hooks/query/use-active-conversation";
|
||||
import { useCreateConversation } from "#/hooks/mutation/use-create-conversation";
|
||||
import { displaySuccessToast } from "#/utils/custom-toast-handlers";
|
||||
import {
|
||||
getConversationState,
|
||||
setConversationState,
|
||||
} from "#/utils/conversation-local-storage";
|
||||
|
||||
/**
|
||||
* Custom hook that encapsulates the logic for handling plan creation.
|
||||
@@ -15,12 +19,30 @@ import { displaySuccessToast } from "#/utils/custom-toast-handlers";
|
||||
*/
|
||||
export const useHandlePlanClick = () => {
|
||||
const { t } = useTranslation();
|
||||
const { setConversationMode, setSubConversationTaskId } =
|
||||
useConversationStore();
|
||||
const {
|
||||
setConversationMode,
|
||||
setSubConversationTaskId,
|
||||
subConversationTaskId,
|
||||
} = useConversationStore();
|
||||
const { data: conversation } = useActiveConversation();
|
||||
const { mutate: createConversation, isPending: isCreatingConversation } =
|
||||
useCreateConversation();
|
||||
|
||||
// Restore subConversationTaskId from localStorage on conversation load
|
||||
// This handles the case where page was refreshed while sub-conversation creation was in progress
|
||||
useEffect(() => {
|
||||
if (!conversation?.conversation_id) return;
|
||||
|
||||
const storedState = getConversationState(conversation.conversation_id);
|
||||
if (storedState.subConversationTaskId && !subConversationTaskId) {
|
||||
setSubConversationTaskId(storedState.subConversationTaskId);
|
||||
}
|
||||
}, [
|
||||
conversation?.conversation_id,
|
||||
subConversationTaskId,
|
||||
setSubConversationTaskId,
|
||||
]);
|
||||
|
||||
const handlePlanClick = useCallback(
|
||||
(event?: React.MouseEvent<HTMLButtonElement> | KeyboardEvent) => {
|
||||
event?.preventDefault();
|
||||
@@ -29,13 +51,14 @@ export const useHandlePlanClick = () => {
|
||||
// Set conversation mode to "plan" immediately
|
||||
setConversationMode("plan");
|
||||
|
||||
// Check if sub_conversation_ids is not empty
|
||||
// Check if sub_conversation_ids is not empty or if a sub-conversation creation is already in progress
|
||||
if (
|
||||
(conversation?.sub_conversation_ids &&
|
||||
conversation.sub_conversation_ids.length > 0) ||
|
||||
!conversation?.conversation_id
|
||||
!conversation?.conversation_id ||
|
||||
subConversationTaskId
|
||||
) {
|
||||
// Do nothing if both conditions are true
|
||||
// Do nothing if any condition is true
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -53,6 +76,10 @@ export const useHandlePlanClick = () => {
|
||||
// Track the task ID to poll for sub-conversation creation
|
||||
if (data.v1_task_id) {
|
||||
setSubConversationTaskId(data.v1_task_id);
|
||||
// Persist to localStorage so it survives page refresh
|
||||
setConversationState(conversation.conversation_id, {
|
||||
subConversationTaskId: data.v1_task_id,
|
||||
});
|
||||
}
|
||||
},
|
||||
},
|
||||
@@ -63,6 +90,7 @@ export const useHandlePlanClick = () => {
|
||||
createConversation,
|
||||
setConversationMode,
|
||||
setSubConversationTaskId,
|
||||
subConversationTaskId,
|
||||
t,
|
||||
],
|
||||
);
|
||||
|
||||
@@ -12,12 +12,14 @@ export interface ConversationState {
|
||||
selectedTab: ConversationTab | null;
|
||||
rightPanelShown: boolean;
|
||||
unpinnedTabs: string[];
|
||||
subConversationTaskId: string | null;
|
||||
}
|
||||
|
||||
const DEFAULT_CONVERSATION_STATE: ConversationState = {
|
||||
selectedTab: "editor",
|
||||
rightPanelShown: true,
|
||||
unpinnedTabs: [],
|
||||
subConversationTaskId: null,
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user