diff --git a/frontend/src/components/chat/ChatInterface.test.tsx b/frontend/src/components/chat/ChatInterface.test.tsx index 44dbf3d352..5e982e1e71 100644 --- a/frontend/src/components/chat/ChatInterface.test.tsx +++ b/frontend/src/components/chat/ChatInterface.test.tsx @@ -9,16 +9,21 @@ import ActionType from "#/types/ActionType"; import { addAssistantMessage } from "#/state/chatSlice"; import AgentState from "#/types/AgentState"; -const sessionSpy = vi.spyOn(Session, "send"); -vi.spyOn(Session, "isConnected").mockReturnValue(true); - // This is for the scrollview ref in Chat.tsx // TODO: Move this into test setup HTMLElement.prototype.scrollTo = vi.fn().mockImplementation(() => {}); describe("ChatInterface", () => { + const sessionSendSpy = vi.spyOn(Session, "send"); + vi.spyOn(Session, "isConnected").mockReturnValue(true); + + const userMessageEvent = { + action: ActionType.MESSAGE, + args: { content: "my message" }, + }; + afterEach(() => { - sessionSpy.mockClear(); + sessionSendSpy.mockClear(); }); it("should render empty message list and input", () => { @@ -26,21 +31,6 @@ describe("ChatInterface", () => { expect(screen.queryAllByTestId("message")).toHaveLength(0); }); - it("should render the new message the user has typed", async () => { - const user = userEvent.setup(); - renderWithProviders(, { - preloadedState: { - agent: { - curAgentState: AgentState.INIT, - }, - }, - }); - - const input = screen.getByRole("textbox"); - await user.type(input, "my message"); - expect(input).toHaveValue("my message"); - }); - it("should render user and assistant messages", () => { const { store } = renderWithProviders(, { preloadedState: { @@ -54,6 +44,7 @@ describe("ChatInterface", () => { expect(screen.getByText("Hello")).toBeInTheDocument(); act(() => { + // simulate assistant response store.dispatch(addAssistantMessage("Hello to you!")); }); @@ -61,7 +52,7 @@ describe("ChatInterface", () => { expect(screen.getByText("Hello to you!")).toBeInTheDocument(); }); - it("should send a start event to the Session", async () => { + it("should send the user message as an event to the Session when the agent state is INIT", async () => { const user = userEvent.setup(); renderWithProviders(, { preloadedState: { @@ -75,14 +66,12 @@ describe("ChatInterface", () => { await user.type(input, "my message"); await user.keyboard("{Enter}"); - const event = { - action: ActionType.MESSAGE, - args: { content: "my message" }, - }; - expect(sessionSpy).toHaveBeenCalledWith(JSON.stringify(event)); + expect(sessionSendSpy).toHaveBeenCalledWith( + JSON.stringify(userMessageEvent), + ); }); - it("should send a user message event to the Session", async () => { + it("should send the user message as an event to the Session when the agent state is AWAITING_USER_INPUT", async () => { const user = userEvent.setup(); renderWithProviders(, { preloadedState: { @@ -96,14 +85,13 @@ describe("ChatInterface", () => { await user.type(input, "my message"); await user.keyboard("{Enter}"); - const event = { - action: ActionType.MESSAGE, - args: { content: "my message" }, - }; - expect(sessionSpy).toHaveBeenCalledWith(JSON.stringify(event)); + expect(sessionSendSpy).toHaveBeenCalledWith( + JSON.stringify(userMessageEvent), + ); }); - it("should disable the user input if agent is not initialized", () => { + it("should disable the user input if agent is not initialized", async () => { + const user = userEvent.setup(); renderWithProviders(, { preloadedState: { agent: { @@ -112,10 +100,16 @@ describe("ChatInterface", () => { }, }); + const input = screen.getByRole("textbox"); + await user.type(input, "my message"); + await user.keyboard("{Enter}"); const submitButton = screen.getByLabelText( "CHAT_INTERFACE$TOOLTIP_SEND_MESSAGE", ); expect(submitButton).toBeDisabled(); + expect(sessionSendSpy).not.toHaveBeenCalled(); }); + + it.todo("test scroll-related behaviour"); }); diff --git a/frontend/src/components/chat/ChatInterface.tsx b/frontend/src/components/chat/ChatInterface.tsx index 5dcdae3c8f..b08bc23ebd 100644 --- a/frontend/src/components/chat/ChatInterface.tsx +++ b/frontend/src/components/chat/ChatInterface.tsx @@ -1,4 +1,3 @@ -// frontend/src/components/chat/ChatInterface.tsx import React, { useRef } from "react"; import { useDispatch, useSelector } from "react-redux"; import { IoMdChatbubbles } from "react-icons/io"; @@ -16,11 +15,7 @@ import { sendChatMessage } from "#/services/chatService"; import { addUserMessage, addAssistantMessage } from "#/state/chatSlice"; import { I18nKey } from "#/i18n/declaration"; import { useScrollToBottom } from "#/hooks/useScrollToBottom"; -import { Feedback } from "#/services/feedbackService"; import FeedbackModal from "../modals/feedback/FeedbackModal"; -import { removeApiKey } from "#/utils/utils"; -import Session from "#/services/session"; -import { getToken } from "#/services/auth"; interface ScrollButtonProps { onClick: () => void; @@ -55,15 +50,9 @@ function ChatInterface() { const { messages } = useSelector((state: RootState) => state.chat); const { curAgentState } = useSelector((state: RootState) => state.agent); - const feedbackVersion = "1.0"; - const [feedback, setFeedback] = React.useState({ - email: "", - feedback: "positive", - permissions: "private", - trajectory: [], - token: "", - version: feedbackVersion, - }); + const [feedbackPolarity, setFeedbackPolarity] = React.useState< + "positive" | "negative" + >("positive"); const [feedbackShared, setFeedbackShared] = React.useState(0); const { @@ -73,13 +62,8 @@ function ChatInterface() { } = useDisclosure(); const shareFeedback = async (polarity: "positive" | "negative") => { - setFeedback((prev) => ({ - ...prev, - feedback: polarity, - trajectory: removeApiKey(Session._history), - token: getToken(), - })); onFeedbackModalOpen(); + setFeedbackPolarity(polarity); }; const handleSendMessage = (content: string) => { @@ -87,14 +71,6 @@ function ChatInterface() { sendChatMessage(content); }; - const handleEmailChange = (key: string) => { - setFeedback({ ...feedback, email: key } as Feedback); - }; - - const handlePermissionsChange = (permissions: "public" | "private") => { - setFeedback({ ...feedback, permissions } as Feedback); - }; - const { t } = useTranslation(); const handleSendContinueMsg = () => { handleSendMessage(t(I18nKey.CHAT_INTERFACE$INPUT_CONTINUE_MESSAGE)); @@ -176,9 +152,7 @@ function ChatInterface() { onSendMessage={handleSendMessage} /> setFeedbackShared(messages.length)} diff --git a/frontend/src/components/modals/base-modal/BaseModal.tsx b/frontend/src/components/modals/base-modal/BaseModal.tsx index 63f2d08646..33015eca76 100644 --- a/frontend/src/components/modals/base-modal/BaseModal.tsx +++ b/frontend/src/components/modals/base-modal/BaseModal.tsx @@ -17,6 +17,7 @@ interface BaseModalProps { subtitle?: string; actions?: Action[]; children?: React.ReactNode; + testID?: string; } function BaseModal({ @@ -27,9 +28,11 @@ function BaseModal({ subtitle = undefined, actions = [], children = null, + testID, }: BaseModalProps) { return ( void; - onPermissionsChange: (permissions: "public" | "private") => void; -} - -function FeedbackForm({ - feedback, - onEmailChange, - onPermissionsChange, -}: FeedbackFormProps) { - const { t } = useTranslation(); - - const isEmailValid = (email: string) => { - // Regular expression to validate email format - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - return emailRegex.test(email); - }; - - return ( - <> - { - onEmailChange(e.target.value); - }} - /> - - {isEmailValid(feedback.email) ? null : ( -

Invalid email format

- )} - - ); -} - -export default FeedbackForm; diff --git a/frontend/src/components/modals/feedback/FeedbackModal.test.tsx b/frontend/src/components/modals/feedback/FeedbackModal.test.tsx new file mode 100644 index 0000000000..a69c240277 --- /dev/null +++ b/frontend/src/components/modals/feedback/FeedbackModal.test.tsx @@ -0,0 +1,194 @@ +import { render, screen, within } from "@testing-library/react"; +import { Mock, describe } from "vitest"; +import React from "react"; +import userEvent from "@testing-library/user-event"; +import toast from "react-hot-toast"; +import FeedbackModal from "./FeedbackModal"; +import { sendFeedback } from "#/services/feedbackService"; + +describe("FeedbackModal", () => { + Storage.prototype.setItem = vi.fn(); + Storage.prototype.getItem = vi.fn(); + + vi.mock("#/services/feedbackService", () => ({ + sendFeedback: vi.fn(), + })); + + vi.mock("#/services/auth", () => ({ + getToken: vi.fn().mockReturnValue("some-token"), + })); + // mock Session class + vi.mock("#/services/session", () => ({ + default: { + _history: [ + { args: { LLM_API_KEY: "DANGER-key-should-not-be-here" } }, + { content: "Hello" }, + ], + }, + })); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("should render the feedback model when open", () => { + const { rerender } = render( + , + ); + expect(screen.queryByTestId("feedback-modal")).not.toBeInTheDocument(); + + rerender( + , + ); + expect(screen.getByTestId("feedback-modal")).toBeInTheDocument(); + }); + + it("should display an error if the email is invalid when submitting", async () => { + const user = userEvent.setup(); + render( + , + ); + + const submitButton = screen.getByRole("button", { + name: "FEEDBACK$SHARE_LABEL", + }); + + await user.click(submitButton); + + expect(screen.getByTestId("invalid-email-message")).toBeInTheDocument(); + expect(sendFeedback).not.toHaveBeenCalled(); + }); + + it("should call sendFeedback with the correct data when the share button is clicked", async () => { + const user = userEvent.setup(); + render( + , + ); + + const submitButton = screen.getByRole("button", { + name: "FEEDBACK$SHARE_LABEL", + }); + + const email = "example@example.com"; + const emailInput = screen.getByTestId("email-input"); + await user.type(emailInput, email); + + // select public + const permissionsGroup = screen.getByTestId("permissions-group"); + const publicOption = within(permissionsGroup).getByRole("radio", { + name: "FEEDBACK$PUBLIC_LABEL", + }); + expect(publicOption).not.toBeChecked(); + await user.click(publicOption); + expect(publicOption).toBeChecked(); + + await user.click(submitButton); + + expect( + screen.queryByTestId("invalid-email-message"), + ).not.toBeInTheDocument(); + + expect(sendFeedback).toHaveBeenCalledWith({ + email, + permissions: "public", + feedback: "negative", + trajectory: [{ args: {} }, { content: "Hello" }], // api key should be removed + token: "some-token", + version: "1.0", + }); + }); + + it("should store the users email in local state for later use", async () => { + const email = "example@example.com"; + + const user = userEvent.setup(); + const { rerender } = render( + , + ); + + expect(localStorage.getItem).toHaveBeenCalledWith("feedback-email"); + const emailInput = screen.getByTestId("email-input"); + expect(emailInput).toHaveValue(""); + + await user.type(emailInput, email); + expect(emailInput).toHaveValue(email); + + const submitButton = screen.getByRole("button", { + name: "FEEDBACK$SHARE_LABEL", + }); + await user.click(submitButton); + + expect(localStorage.setItem).toHaveBeenCalledWith("feedback-email", email); + + rerender( + , + ); + + const emailInputAfterClose = screen.getByTestId("email-input"); + expect(emailInputAfterClose).toHaveValue(email); + }); + + // TODO: figure out how to properly mock toast + it.skip("should display a success toast when the feedback is shared successfully", async () => { + (sendFeedback as Mock).mockResolvedValue({ + statusCode: 200, + body: { + message: "Feedback shared", + feedback_id: "some-id", + password: "some-password", + }, + }); + + const user = userEvent.setup(); + render( + , + ); + + const submitButton = screen.getByRole("button", { + name: "FEEDBACK$SHARE_LABEL", + }); + + const email = "example@example.com"; + const emailInput = screen.getByTestId("email-input"); + await user.type(emailInput, email); + + await user.click(submitButton); + + expect(toast).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/modals/feedback/FeedbackModal.tsx b/frontend/src/components/modals/feedback/FeedbackModal.tsx index 10c5759509..6b462ae4a5 100644 --- a/frontend/src/components/modals/feedback/FeedbackModal.tsx +++ b/frontend/src/components/modals/feedback/FeedbackModal.tsx @@ -1,57 +1,127 @@ import React from "react"; import { useTranslation } from "react-i18next"; +import { Input, Radio, RadioGroup } from "@nextui-org/react"; +import hotToast from "react-hot-toast"; import { I18nKey } from "#/i18n/declaration"; import BaseModal from "../base-modal/BaseModal"; import { Feedback, sendFeedback } from "#/services/feedbackService"; -import FeedbackForm from "./FeedbackForm"; import toast from "#/utils/toast"; +import { getToken } from "#/services/auth"; +import Session from "#/services/session"; +import { removeApiKey } from "#/utils/utils"; + +const isEmailValid = (email: string) => { + // Regular expression to validate email format + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + return emailRegex.test(email); +}; const VIEWER_PAGE = "https://www.all-hands.dev/share-opendevin"; +const FEEDBACK_VERSION = "1.0"; interface FeedbackModalProps { - feedback: Feedback; - handleEmailChange: (key: string) => void; - handlePermissionsChange: (permissions: "public" | "private") => void; + polarity: "positive" | "negative"; isOpen: boolean; onOpenChange: (isOpen: boolean) => void; onSendFeedback: () => void; } function FeedbackModal({ - feedback, - handleEmailChange, - handlePermissionsChange, + polarity, isOpen, onOpenChange, onSendFeedback, }: FeedbackModalProps) { const { t } = useTranslation(); - const handleSendFeedback = () => { + const [email, setEmail] = React.useState(""); + const [permissions, setPermissions] = React.useState<"public" | "private">( + "private", + ); + + React.useEffect(() => { + // check if email is stored in local storage + const storedEmail = localStorage.getItem("feedback-email"); + if (storedEmail) setEmail(storedEmail); + }, []); + + const handleEmailChange = (newEmail: string) => { + setEmail(newEmail); + }; + + const copiedToClipboardToast = () => { + hotToast("Password copied to clipboard", { + icon: "📋", + position: "bottom-right", + }); + }; + + const onPressToast = (password: string) => { + navigator.clipboard.writeText(password); + copiedToClipboardToast(); + }; + + const shareFeedbackToast = ( + message: string, + link: string, + password: string, + ) => { + hotToast( +
+ {message} + onPressToast(password)} + href={link} + target="_blank" + rel="noreferrer" + > + Go to shared feedback + + onPressToast(password)} className="cursor-pointer"> + Password: {password} (copy) + +
, + { duration: 5000 }, + ); + }; + + const handleSendFeedback = async () => { onSendFeedback(); - sendFeedback(feedback) - .then((response) => { - if (response.statusCode === 200) { - const { message, feedback_id: feedbackId, password } = response.body; - const toastMessage = `${message}\nFeedback link: ${VIEWER_PAGE}?share_id=${feedbackId}\nPassword: ${password}`; - toast.info(toastMessage); - } else { - toast.error( - "share-error", - `Failed to share, please contact the developers: ${response.body.message}`, - ); - } - }) - .catch((error) => { + const feedback: Feedback = { + version: FEEDBACK_VERSION, + feedback: polarity, + email, + permissions, + token: getToken(), + trajectory: removeApiKey(Session._history), + }; + + try { + const response = await sendFeedback(feedback); + localStorage.setItem("feedback-email", email); // store email in local storage + if (response.statusCode === 200) { + const { message, feedback_id: feedbackId, password } = response.body; + const link = `${VIEWER_PAGE}?share_id=${feedbackId}&password=${password}`; + shareFeedbackToast(message, link, password); + } else { toast.error( "share-error", - `Failed to share, please contact the developers: ${error}`, + `Failed to share, please contact the developers: ${response.body.message}`, ); - }); + } + } catch (error) { + toast.error( + "share-error", + `Failed to share, please contact the developers: ${error}`, + ); + } }; return (

{t(I18nKey.FEEDBACK$MODAL_CONTENT)}

- { + handleEmailChange(e.target.value); + }} /> + {!isEmailValid(email) && ( +

+ Invalid email format +

+ )} + setPermissions(value as "public" | "private")} + > + {t(I18nKey.FEEDBACK$PRIVATE_LABEL)} + {t(I18nKey.FEEDBACK$PUBLIC_LABEL)} + ); } diff --git a/frontend/src/components/modals/settings/SettingsModal.test.tsx b/frontend/src/components/modals/settings/SettingsModal.test.tsx index 45dbad06f2..8462942ac6 100644 --- a/frontend/src/components/modals/settings/SettingsModal.test.tsx +++ b/frontend/src/components/modals/settings/SettingsModal.test.tsx @@ -1,4 +1,4 @@ -import { screen, act, waitFor } from "@testing-library/react"; +import { act, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import i18next from "i18next"; import React from "react"; @@ -70,19 +70,15 @@ describe("SettingsModal", () => { }); it("should close the modal when the close button is clicked", async () => { + const user = userEvent.setup(); const onOpenChange = vi.fn(); - await act(async () => - renderWithProviders(), - ); + renderWithProviders(); const cancelButton = screen.getByRole("button", { name: /MODAL_CLOSE_BUTTON_LABEL/i, // i18n key }); - await act(async () => { - await userEvent.click(cancelButton); - }); - + await user.click(cancelButton); expect(onOpenChange).toHaveBeenCalledWith(false); }); @@ -113,11 +109,10 @@ describe("SettingsModal", () => { }; it("should save the settings", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); - await act(async () => - renderWithProviders( - , - ), + renderWithProviders( + , ); // Use the helper function to assert models were fetched @@ -126,19 +121,11 @@ describe("SettingsModal", () => { const saveButton = screen.getByRole("button", { name: /save/i }); const modelInput = screen.getByRole("combobox", { name: "model" }); - await act(async () => { - await userEvent.click(modelInput); - }); - + await user.click(modelInput); const model3 = screen.getByText("model3"); - await act(async () => { - await userEvent.click(model3); - }); - - await act(async () => { - await userEvent.click(saveButton); - }); + await user.click(model3); + await user.click(saveButton); expect(saveSettings).toHaveBeenCalledWith({ ...initialSettings, @@ -147,6 +134,7 @@ describe("SettingsModal", () => { }); it("should reinitialize agent", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); await act(async () => renderWithProviders( @@ -157,24 +145,17 @@ describe("SettingsModal", () => { const saveButton = screen.getByRole("button", { name: /save/i }); const modelInput = screen.getByRole("combobox", { name: "model" }); - await act(async () => { - await userEvent.click(modelInput); - }); - + await user.click(modelInput); const model3 = screen.getByText("model3"); - await act(async () => { - await userEvent.click(model3); - }); - - await act(async () => { - await userEvent.click(saveButton); - }); + await user.click(model3); + await user.click(saveButton); expect(startNewSessionSpy).toHaveBeenCalled(); }); it("should display a toast for every change", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); await act(async () => renderWithProviders( @@ -185,24 +166,17 @@ describe("SettingsModal", () => { const saveButton = screen.getByRole("button", { name: /save/i }); const modelInput = screen.getByRole("combobox", { name: "model" }); - await act(async () => { - await userEvent.click(modelInput); - }); - + await user.click(modelInput); const model3 = screen.getByText("model3"); - await act(async () => { - await userEvent.click(model3); - }); - - await act(async () => { - await userEvent.click(saveButton); - }); + await user.click(model3); + await user.click(saveButton); expect(toastSpy).toHaveBeenCalledTimes(3); }); it("should change the language", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); await act(async () => renderWithProviders( @@ -213,24 +187,17 @@ describe("SettingsModal", () => { const saveButton = screen.getByRole("button", { name: /save/i }); const languageInput = screen.getByRole("combobox", { name: "language" }); - await act(async () => { - await userEvent.click(languageInput); - }); - + await user.click(languageInput); const spanish = screen.getByText("Español"); - await act(async () => { - await userEvent.click(spanish); - }); - - await act(async () => { - await userEvent.click(saveButton); - }); + await user.click(spanish); + await user.click(saveButton); expect(i18nSpy).toHaveBeenCalledWith("es"); }); it("should close the modal", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); await act(async () => renderWithProviders( @@ -245,25 +212,18 @@ describe("SettingsModal", () => { const saveButton = screen.getByRole("button", { name: /save/i }); const modelInput = screen.getByRole("combobox", { name: "model" }); - await act(async () => { - await userEvent.click(modelInput); - }); - + await user.click(modelInput); const model3 = screen.getByText("model3"); - await act(async () => { - await userEvent.click(model3); - }); - - await act(async () => { - await userEvent.click(saveButton); - }); + await user.click(model3); + await user.click(saveButton); expect(onOpenChangeMock).toHaveBeenCalledWith(false); }); }); it("should reset settings to defaults when the 'reset to defaults' button is clicked", async () => { + const user = userEvent.setup(); const onOpenChangeMock = vi.fn(); await act(async () => renderWithProviders( @@ -276,18 +236,12 @@ describe("SettingsModal", () => { }); const agentInput = screen.getByRole("combobox", { name: "agent" }); - await act(async () => { - await userEvent.click(agentInput); - }); + await user.click(agentInput); const agent3 = screen.getByText("agent3"); - await act(async () => { - await userEvent.click(agent3); - }); + await user.click(agent3); expect(agentInput).toHaveValue("agent3"); - await act(async () => { - await userEvent.click(resetButton); - }); + await user.click(resetButton); expect(getDefaultSettings).toHaveBeenCalled(); expect(agentInput).toHaveValue("CodeActAgent"); // Agent value is reset to default from getDefaultSettings() diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index 487b4535ed..4ec0d05db7 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -732,5 +732,17 @@ "zh-CN": "计划未创建", "zh-TW": "未創建任何計劃。", "de": "Kein Plan erstellt." + }, + "FEEDBACK$PUBLIC_LABEL": { + "en": "Public", + "zh-CN": "公开", + "zh-TW": "公開。", + "de": "Öffentlich" + }, + "FEEDBACK$PRIVATE_LABEL": { + "en": "Private", + "zh-CN": "私有", + "zh-TW": "私有。", + "de": "Privat" } }