mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
feat: Allow the users to edit the conversation's title. (#9648)
This commit is contained in:
parent
c03d390772
commit
6d57eeb3ed
@ -529,4 +529,287 @@ describe("ConversationPanel", () => {
|
||||
|
||||
expect(screen.queryByTestId("stop-button")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show edit button in context menu", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
expect(cards).toHaveLength(3);
|
||||
|
||||
// Click ellipsis to open context menu
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
// Edit button should be visible
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
expect(editButton).toBeInTheDocument();
|
||||
expect(editButton).toHaveTextContent("BUTTON$EDIT_TITLE");
|
||||
});
|
||||
|
||||
it("should enter edit mode when edit button is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Click ellipsis to open context menu
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
// Click edit button
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Should find input field instead of title text
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
expect(titleInput).toBeInTheDocument();
|
||||
expect(titleInput.tagName).toBe("INPUT");
|
||||
expect(titleInput).toHaveValue("Conversation 1");
|
||||
expect(titleInput).toHaveFocus();
|
||||
});
|
||||
|
||||
it("should successfully update conversation title", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
// Mock the updateConversation API call
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
// Mock the toast function
|
||||
const mockToast = vi.fn();
|
||||
vi.mock("#/utils/custom-toast-handlers", () => ({
|
||||
displaySuccessToast: mockToast,
|
||||
}));
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Edit the title
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.type(titleInput, "Updated Title");
|
||||
|
||||
// Blur the input to save
|
||||
await user.tab();
|
||||
|
||||
// Verify API call was made with correct parameters
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Updated Title",
|
||||
});
|
||||
});
|
||||
|
||||
it("should save title when Enter key is pressed", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Edit the title and press Enter
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.type(titleInput, "Title Updated via Enter");
|
||||
await user.keyboard("{Enter}");
|
||||
|
||||
// Verify API call was made
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Title Updated via Enter",
|
||||
});
|
||||
});
|
||||
|
||||
it("should trim whitespace from title", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Edit the title with extra whitespace
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.type(titleInput, " Trimmed Title ");
|
||||
await user.tab();
|
||||
|
||||
// Verify API call was made with trimmed title
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Trimmed Title",
|
||||
});
|
||||
|
||||
// Verify input shows trimmed value
|
||||
expect(titleInput).toHaveValue("Trimmed Title");
|
||||
});
|
||||
|
||||
it("should revert to original title when empty", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Clear the title completely
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.tab();
|
||||
|
||||
// Verify API was not called
|
||||
expect(updateConversationSpy).not.toHaveBeenCalled();
|
||||
|
||||
// Verify input reverted to original value
|
||||
expect(titleInput).toHaveValue("Conversation 1");
|
||||
});
|
||||
|
||||
it("should handle API error when updating title", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockRejectedValue(new Error("API Error"));
|
||||
|
||||
vi.mock("#/utils/custom-toast-handlers", () => ({
|
||||
displayErrorToast: vi.fn(),
|
||||
}));
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Edit the title
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.type(titleInput, "Failed Update");
|
||||
await user.tab();
|
||||
|
||||
// Verify API call was made
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Failed Update",
|
||||
});
|
||||
|
||||
// Wait for error handling
|
||||
await waitFor(() => {
|
||||
expect(updateConversationSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("should close context menu when edit button is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Click ellipsis to open context menu
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
// Verify context menu is open
|
||||
const contextMenu = screen.getByTestId("context-menu");
|
||||
expect(contextMenu).toBeInTheDocument();
|
||||
|
||||
// Click edit button
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Verify context menu is closed
|
||||
expect(screen.queryByTestId("context-menu")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should not call API when title is unchanged", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Don't change the title, just blur
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.tab();
|
||||
|
||||
// Verify API was called with the same title (since handleConversationTitleChange will always be called)
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Conversation 1",
|
||||
});
|
||||
});
|
||||
|
||||
it("should handle special characters in title", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
||||
const updateConversationSpy = vi.spyOn(OpenHands, "updateConversation");
|
||||
updateConversationSpy.mockResolvedValue(true);
|
||||
|
||||
renderConversationPanel();
|
||||
|
||||
const cards = await screen.findAllByTestId("conversation-card");
|
||||
|
||||
// Enter edit mode
|
||||
const ellipsisButton = within(cards[0]).getByTestId("ellipsis-button");
|
||||
await user.click(ellipsisButton);
|
||||
|
||||
const editButton = screen.getByTestId("edit-button");
|
||||
await user.click(editButton);
|
||||
|
||||
// Edit the title with special characters
|
||||
const titleInput = within(cards[0]).getByTestId("conversation-card-title");
|
||||
await user.clear(titleInput);
|
||||
await user.type(titleInput, "Special @#$%^&*()_+ Characters");
|
||||
await user.tab();
|
||||
|
||||
// Verify API call was made with special characters
|
||||
expect(updateConversationSpy).toHaveBeenCalledWith("1", {
|
||||
title: "Special @#$%^&*()_+ Characters",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -477,6 +477,18 @@ class OpenHands {
|
||||
|
||||
return data.prompt;
|
||||
}
|
||||
|
||||
static async updateConversation(
|
||||
conversationId: string,
|
||||
updates: { title: string },
|
||||
): Promise<boolean> {
|
||||
const { data } = await openHands.patch<boolean>(
|
||||
`/api/conversations/${conversationId}`,
|
||||
updates,
|
||||
);
|
||||
|
||||
return data;
|
||||
}
|
||||
}
|
||||
|
||||
export default OpenHands;
|
||||
|
||||
@ -12,6 +12,8 @@ import { LoadingSpinner } from "#/components/shared/loading-spinner";
|
||||
import { ExitConversationModal } from "./exit-conversation-modal";
|
||||
import { useClickOutsideElement } from "#/hooks/use-click-outside-element";
|
||||
import { Provider } from "#/types/settings";
|
||||
import { useUpdateConversation } from "#/hooks/mutation/use-update-conversation";
|
||||
import { displaySuccessToast } from "#/utils/custom-toast-handlers";
|
||||
|
||||
interface ConversationPanelProps {
|
||||
onClose: () => void;
|
||||
@ -39,6 +41,7 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) {
|
||||
|
||||
const { mutate: deleteConversation } = useDeleteConversation();
|
||||
const { mutate: stopConversation } = useStopConversation();
|
||||
const { mutate: updateConversation } = useUpdateConversation();
|
||||
|
||||
const handleDeleteProject = (conversationId: string) => {
|
||||
setConfirmDeleteModalVisible(true);
|
||||
@ -50,6 +53,20 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) {
|
||||
setSelectedConversationId(conversationId);
|
||||
};
|
||||
|
||||
const handleConversationTitleChange = async (
|
||||
conversationId: string,
|
||||
newTitle: string,
|
||||
) => {
|
||||
updateConversation(
|
||||
{ conversationId, newTitle },
|
||||
{
|
||||
onSuccess: () => {
|
||||
displaySuccessToast(t(I18nKey.CONVERSATION$TITLE_UPDATED));
|
||||
},
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
const handleConfirmDelete = () => {
|
||||
if (selectedConversationId) {
|
||||
deleteConversation(
|
||||
@ -114,6 +131,9 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) {
|
||||
isActive={isActive}
|
||||
onDelete={() => handleDeleteProject(project.conversation_id)}
|
||||
onStop={() => handleStopConversation(project.conversation_id)}
|
||||
onChangeTitle={(title) =>
|
||||
handleConversationTitleChange(project.conversation_id, title)
|
||||
}
|
||||
title={project.title}
|
||||
selectedRepository={{
|
||||
selected_repository: project.selected_repository,
|
||||
|
||||
51
frontend/src/hooks/mutation/use-update-conversation.ts
Normal file
51
frontend/src/hooks/mutation/use-update-conversation.ts
Normal file
@ -0,0 +1,51 @@
|
||||
import { useMutation, useQueryClient } from "@tanstack/react-query";
|
||||
import OpenHands from "#/api/open-hands";
|
||||
|
||||
export const useUpdateConversation = () => {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useMutation({
|
||||
mutationFn: (variables: { conversationId: string; newTitle: string }) =>
|
||||
OpenHands.updateConversation(variables.conversationId, {
|
||||
title: variables.newTitle,
|
||||
}),
|
||||
onMutate: async (variables) => {
|
||||
await queryClient.cancelQueries({ queryKey: ["user", "conversations"] });
|
||||
const previousConversations = queryClient.getQueryData([
|
||||
"user",
|
||||
"conversations",
|
||||
]);
|
||||
|
||||
queryClient.setQueryData(
|
||||
["user", "conversations"],
|
||||
(old: { conversation_id: string; title: string }[] | undefined) =>
|
||||
old?.map((conv) =>
|
||||
conv.conversation_id === variables.conversationId
|
||||
? { ...conv, title: variables.newTitle }
|
||||
: conv,
|
||||
),
|
||||
);
|
||||
|
||||
return { previousConversations };
|
||||
},
|
||||
onError: (err, variables, context) => {
|
||||
if (context?.previousConversations) {
|
||||
queryClient.setQueryData(
|
||||
["user", "conversations"],
|
||||
context.previousConversations,
|
||||
);
|
||||
}
|
||||
},
|
||||
onSettled: (data, error, variables) => {
|
||||
// Invalidate and refetch the conversation list to show the updated title
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: ["user", "conversations"],
|
||||
});
|
||||
|
||||
// Also invalidate the specific conversation query
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: ["user", "conversation", variables.conversationId],
|
||||
});
|
||||
},
|
||||
});
|
||||
};
|
||||
@ -332,6 +332,7 @@ export enum I18nKey {
|
||||
CONVERSATION$AGO = "CONVERSATION$AGO",
|
||||
GITHUB$VSCODE_LINK_DESCRIPTION = "GITHUB$VSCODE_LINK_DESCRIPTION",
|
||||
CONVERSATION$EXIT_WARNING = "CONVERSATION$EXIT_WARNING",
|
||||
CONVERSATION$TITLE_UPDATED = "CONVERSATION$TITLE_UPDATED",
|
||||
LANDING$OR = "LANDING$OR",
|
||||
SUGGESTIONS$TEST_COVERAGE = "SUGGESTIONS$TEST_COVERAGE",
|
||||
SUGGESTIONS$AUTO_MERGE = "SUGGESTIONS$AUTO_MERGE",
|
||||
|
||||
@ -5312,6 +5312,22 @@
|
||||
"de": "Gespräch verlassen",
|
||||
"uk": "Вийти з розмови"
|
||||
},
|
||||
"CONVERSATION$TITLE_UPDATED": {
|
||||
"en": "Conversation title updated successfully",
|
||||
"ja": "会話のタイトルが正常に更新されました",
|
||||
"zh-CN": "对话标题更新成功",
|
||||
"zh-TW": "對話標題更新成功",
|
||||
"ko-KR": "대화 제목이 성공적으로 업데이트되었습니다",
|
||||
"de": "Gesprächstitel erfolgreich aktualisiert",
|
||||
"no": "Samtaletittel oppdatert",
|
||||
"it": "Titolo della conversazione aggiornato con successo",
|
||||
"pt": "Título da conversa atualizado com sucesso",
|
||||
"es": "Título de la conversación actualizado exitosamente",
|
||||
"ar": "تم تحديث عنوان المحادثة بنجاح",
|
||||
"fr": "Titre de la conversation mis à jour avec succès",
|
||||
"tr": "Konuşma başlığı başarıyla güncellendi",
|
||||
"uk": "Назву розмови успішно оновлено"
|
||||
},
|
||||
"LANDING$OR": {
|
||||
"en": "Or",
|
||||
"ja": "または",
|
||||
|
||||
@ -554,3 +554,122 @@ def _get_contextual_events(event_stream: EventStream, event_id: int) -> str:
|
||||
all_events = itertools.chain(ordered_context_before, context_after)
|
||||
stringified_events = '\n'.join(str(event) for event in all_events)
|
||||
return stringified_events
|
||||
|
||||
|
||||
class UpdateConversationRequest(BaseModel):
|
||||
"""Request model for updating conversation metadata."""
|
||||
|
||||
title: str = Field(
|
||||
..., min_length=1, max_length=200, description='New conversation title'
|
||||
)
|
||||
|
||||
model_config = ConfigDict(extra='forbid')
|
||||
|
||||
|
||||
@app.patch('/conversations/{conversation_id}')
|
||||
async def update_conversation(
|
||||
conversation_id: str,
|
||||
data: UpdateConversationRequest,
|
||||
user_id: str | None = Depends(get_user_id),
|
||||
conversation_store: ConversationStore = Depends(get_conversation_store),
|
||||
) -> bool:
|
||||
"""Update conversation metadata.
|
||||
|
||||
This endpoint allows updating conversation details like title.
|
||||
Only the conversation owner can update the conversation.
|
||||
|
||||
Args:
|
||||
conversation_id: The ID of the conversation to update
|
||||
data: The conversation update data (title, etc.)
|
||||
user_id: The authenticated user ID
|
||||
conversation_store: The conversation store dependency
|
||||
|
||||
Returns:
|
||||
bool: True if the conversation was updated successfully
|
||||
|
||||
Raises:
|
||||
HTTPException: If conversation is not found or user lacks permission
|
||||
"""
|
||||
|
||||
logger.info(
|
||||
f'Updating conversation {conversation_id} with title: {data.title}',
|
||||
extra={'session_id': conversation_id, 'user_id': user_id},
|
||||
)
|
||||
|
||||
try:
|
||||
# Get the existing conversation metadata
|
||||
metadata = await conversation_store.get_metadata(conversation_id)
|
||||
|
||||
# Validate that the user owns this conversation
|
||||
if user_id and metadata.user_id != user_id:
|
||||
logger.warning(
|
||||
f'User {user_id} attempted to update conversation {conversation_id} owned by {metadata.user_id}',
|
||||
extra={'session_id': conversation_id, 'user_id': user_id},
|
||||
)
|
||||
return JSONResponse(
|
||||
content={
|
||||
'status': 'error',
|
||||
'message': 'Permission denied: You can only update your own conversations',
|
||||
'msg_id': 'AUTHORIZATION$PERMISSION_DENIED',
|
||||
},
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
|
||||
# Update the conversation metadata
|
||||
original_title = metadata.title
|
||||
metadata.title = data.title.strip()
|
||||
metadata.last_updated_at = datetime.now(timezone.utc)
|
||||
|
||||
# Save the updated metadata
|
||||
await conversation_store.save_metadata(metadata)
|
||||
|
||||
# Emit a status update to connected clients about the title change
|
||||
try:
|
||||
status_update_dict = {
|
||||
'status_update': True,
|
||||
'type': 'info',
|
||||
'message': conversation_id,
|
||||
'conversation_title': metadata.title,
|
||||
}
|
||||
await conversation_manager.sio.emit(
|
||||
'oh_event',
|
||||
status_update_dict,
|
||||
to=f'room:{conversation_id}',
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(f'Error emitting title update event: {e}')
|
||||
# Don't fail the update if we can't emit the event
|
||||
|
||||
logger.info(
|
||||
f'Successfully updated conversation {conversation_id} title from "{original_title}" to "{metadata.title}"',
|
||||
extra={'session_id': conversation_id, 'user_id': user_id},
|
||||
)
|
||||
|
||||
return True
|
||||
|
||||
except FileNotFoundError:
|
||||
logger.warning(
|
||||
f'Conversation {conversation_id} not found for update',
|
||||
extra={'session_id': conversation_id, 'user_id': user_id},
|
||||
)
|
||||
return JSONResponse(
|
||||
content={
|
||||
'status': 'error',
|
||||
'message': 'Conversation not found',
|
||||
'msg_id': 'CONVERSATION$NOT_FOUND',
|
||||
},
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f'Error updating conversation {conversation_id}: {str(e)}',
|
||||
extra={'session_id': conversation_id, 'user_id': user_id},
|
||||
)
|
||||
return JSONResponse(
|
||||
content={
|
||||
'status': 'error',
|
||||
'message': f'Failed to update conversation: {str(e)}',
|
||||
'msg_id': 'CONVERSATION$UPDATE_ERROR',
|
||||
},
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
)
|
||||
|
||||
@ -1,13 +1,21 @@
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
from datetime import datetime, timezone
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import status
|
||||
from fastapi.responses import JSONResponse
|
||||
|
||||
from openhands.microagent.microagent import KnowledgeMicroagent, RepoMicroagent
|
||||
from openhands.microagent.types import MicroagentMetadata, MicroagentType
|
||||
from openhands.server.routes.conversation import get_microagents
|
||||
from openhands.server.routes.manage_conversations import (
|
||||
UpdateConversationRequest,
|
||||
update_conversation,
|
||||
)
|
||||
from openhands.server.session.conversation import ServerConversation
|
||||
from openhands.storage.conversation.conversation_store import ConversationStore
|
||||
from openhands.storage.data_models.conversation_metadata import ConversationMetadata
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@ -155,3 +163,459 @@ async def test_get_microagents_exception():
|
||||
content = json.loads(response.body)
|
||||
assert 'error' in content
|
||||
assert 'Test exception' in content['error']
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_success():
|
||||
"""Test successful conversation update."""
|
||||
# Mock data
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
original_title = 'Original Title'
|
||||
new_title = 'Updated Title'
|
||||
|
||||
# Create mock metadata
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=user_id,
|
||||
title=original_title,
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title=new_title)
|
||||
|
||||
# Mock the conversation manager socket
|
||||
mock_sio = AsyncMock()
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result
|
||||
assert result is True
|
||||
|
||||
# Verify metadata was fetched
|
||||
mock_conversation_store.get_metadata.assert_called_once_with(conversation_id)
|
||||
|
||||
# Verify metadata was updated and saved
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
saved_metadata = mock_conversation_store.save_metadata.call_args[0][0]
|
||||
assert saved_metadata.title == new_title.strip()
|
||||
assert saved_metadata.last_updated_at is not None
|
||||
|
||||
# Verify socket emission
|
||||
mock_sio.emit.assert_called_once()
|
||||
emit_call = mock_sio.emit.call_args
|
||||
assert emit_call[0][0] == 'oh_event'
|
||||
assert emit_call[0][1]['conversation_title'] == new_title
|
||||
assert emit_call[1]['to'] == f'room:{conversation_id}'
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_not_found():
|
||||
"""Test conversation update when conversation doesn't exist."""
|
||||
conversation_id = 'nonexistent_conversation'
|
||||
user_id = 'test_user_456'
|
||||
|
||||
# Create mock conversation store that raises FileNotFoundError
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(side_effect=FileNotFoundError())
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title='New Title')
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result is a 404 error response
|
||||
assert isinstance(result, JSONResponse)
|
||||
assert result.status_code == status.HTTP_404_NOT_FOUND
|
||||
|
||||
# Parse the JSON content
|
||||
content = json.loads(result.body)
|
||||
assert content['status'] == 'error'
|
||||
assert content['message'] == 'Conversation not found'
|
||||
assert content['msg_id'] == 'CONVERSATION$NOT_FOUND'
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_permission_denied():
|
||||
"""Test conversation update when user doesn't own the conversation."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
owner_id = 'different_user_789'
|
||||
|
||||
# Create mock metadata owned by different user
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=owner_id,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title='New Title')
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result is a 403 error response
|
||||
assert isinstance(result, JSONResponse)
|
||||
assert result.status_code == status.HTTP_403_FORBIDDEN
|
||||
|
||||
# Parse the JSON content
|
||||
content = json.loads(result.body)
|
||||
assert content['status'] == 'error'
|
||||
assert (
|
||||
content['message']
|
||||
== 'Permission denied: You can only update your own conversations'
|
||||
)
|
||||
assert content['msg_id'] == 'AUTHORIZATION$PERMISSION_DENIED'
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_permission_denied_no_user_id():
|
||||
"""Test conversation update when user_id is None and metadata has user_id."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = None
|
||||
owner_id = 'some_user_789'
|
||||
|
||||
# Create mock metadata owned by a user
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=owner_id,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title='New Title')
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result is successful (current logic allows this)
|
||||
assert result is True
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_socket_emission_error():
|
||||
"""Test conversation update when socket emission fails."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
new_title = 'Updated Title'
|
||||
|
||||
# Create mock metadata
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=user_id,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title=new_title)
|
||||
|
||||
# Mock the conversation manager socket to raise an exception
|
||||
mock_sio = AsyncMock()
|
||||
mock_sio.emit.side_effect = Exception('Socket error')
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function (should still succeed despite socket error)
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result is still successful
|
||||
assert result is True
|
||||
|
||||
# Verify metadata was still saved
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_general_exception():
|
||||
"""Test conversation update when an unexpected exception occurs."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
|
||||
# Create mock conversation store that raises a general exception
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(
|
||||
side_effect=Exception('Database error')
|
||||
)
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title='New Title')
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result is a 500 error response
|
||||
assert isinstance(result, JSONResponse)
|
||||
assert result.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
|
||||
|
||||
# Parse the JSON content
|
||||
content = json.loads(result.body)
|
||||
assert content['status'] == 'error'
|
||||
assert 'Failed to update conversation' in content['message']
|
||||
assert content['msg_id'] == 'CONVERSATION$UPDATE_ERROR'
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_title_whitespace_trimming():
|
||||
"""Test that conversation title is properly trimmed of whitespace."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
title_with_whitespace = ' Trimmed Title '
|
||||
expected_title = 'Trimmed Title'
|
||||
|
||||
# Create mock metadata
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=user_id,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request with whitespace
|
||||
update_request = UpdateConversationRequest(title=title_with_whitespace)
|
||||
|
||||
# Mock the conversation manager socket
|
||||
mock_sio = AsyncMock()
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify the result
|
||||
assert result is True
|
||||
|
||||
# Verify metadata was updated with trimmed title
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
saved_metadata = mock_conversation_store.save_metadata.call_args[0][0]
|
||||
assert saved_metadata.title == expected_title
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_user_owns_conversation():
|
||||
"""Test successful update when user owns the conversation."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
new_title = 'Updated Title'
|
||||
|
||||
# Create mock metadata owned by the same user
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=user_id, # Same user
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title=new_title)
|
||||
|
||||
# Mock the conversation manager socket
|
||||
mock_sio = AsyncMock()
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify success
|
||||
assert result is True
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_last_updated_at_set():
|
||||
"""Test that last_updated_at is properly set when updating."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = 'test_user_456'
|
||||
new_title = 'Updated Title'
|
||||
|
||||
# Create mock metadata
|
||||
original_timestamp = datetime.now(timezone.utc)
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=user_id,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=original_timestamp,
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title=new_title)
|
||||
|
||||
# Mock the conversation manager socket
|
||||
mock_sio = AsyncMock()
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify success
|
||||
assert result is True
|
||||
|
||||
# Verify last_updated_at was updated
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
saved_metadata = mock_conversation_store.save_metadata.call_args[0][0]
|
||||
assert saved_metadata.last_updated_at > original_timestamp
|
||||
|
||||
|
||||
@pytest.mark.update_conversation
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_conversation_no_user_id_no_metadata_user_id():
|
||||
"""Test successful update when both user_id and metadata.user_id are None."""
|
||||
conversation_id = 'test_conversation_123'
|
||||
user_id = None
|
||||
new_title = 'Updated Title'
|
||||
|
||||
# Create mock metadata with no user_id
|
||||
mock_metadata = ConversationMetadata(
|
||||
conversation_id=conversation_id,
|
||||
user_id=None,
|
||||
title='Original Title',
|
||||
selected_repository=None,
|
||||
last_updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Create mock conversation store
|
||||
mock_conversation_store = MagicMock(spec=ConversationStore)
|
||||
mock_conversation_store.get_metadata = AsyncMock(return_value=mock_metadata)
|
||||
mock_conversation_store.save_metadata = AsyncMock()
|
||||
|
||||
# Create update request
|
||||
update_request = UpdateConversationRequest(title=new_title)
|
||||
|
||||
# Mock the conversation manager socket
|
||||
mock_sio = AsyncMock()
|
||||
|
||||
with patch(
|
||||
'openhands.server.routes.manage_conversations.conversation_manager'
|
||||
) as mock_manager:
|
||||
mock_manager.sio = mock_sio
|
||||
|
||||
# Call the function
|
||||
result = await update_conversation(
|
||||
conversation_id=conversation_id,
|
||||
data=update_request,
|
||||
user_id=user_id,
|
||||
conversation_store=mock_conversation_store,
|
||||
)
|
||||
|
||||
# Verify success (should work when both are None)
|
||||
assert result is True
|
||||
mock_conversation_store.save_metadata.assert_called_once()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user