diff --git a/frontend/__tests__/components/feedback-form.test.tsx b/frontend/__tests__/components/feedback-form.test.tsx index b41fa7e5c7..28684401e2 100644 --- a/frontend/__tests__/components/feedback-form.test.tsx +++ b/frontend/__tests__/components/feedback-form.test.tsx @@ -5,7 +5,6 @@ import { FeedbackForm } from "#/components/feedback-form"; describe("FeedbackForm", () => { const user = userEvent.setup(); - const onSubmitMock = vi.fn(); const onCloseMock = vi.fn(); afterEach(() => { @@ -13,7 +12,7 @@ describe("FeedbackForm", () => { }); it("should render correctly", () => { - render(); + render(); screen.getByLabelText("Email"); screen.getByLabelText("Private"); @@ -24,7 +23,7 @@ describe("FeedbackForm", () => { }); it("should switch between private and public permissions", async () => { - render(); + render(); const privateRadio = screen.getByLabelText("Private"); const publicRadio = screen.getByLabelText("Public"); @@ -40,69 +39,11 @@ describe("FeedbackForm", () => { expect(publicRadio).not.toBeChecked(); }); - it("should call onSubmit when the form is submitted", async () => { - render(); - const email = screen.getByLabelText("Email"); - - await user.type(email, "test@test.test"); - await user.click(screen.getByRole("button", { name: "Submit" })); - - expect(onSubmitMock).toHaveBeenCalledWith("private", "test@test.test"); // private is the default value - }); - - it("should not call onSubmit when the email is invalid", async () => { - render(); - const email = screen.getByLabelText("Email"); - const submitButton = screen.getByRole("button", { name: "Submit" }); - - await user.click(submitButton); - - expect(onSubmitMock).not.toHaveBeenCalled(); - - await user.type(email, "test"); - await user.click(submitButton); - - expect(onSubmitMock).not.toHaveBeenCalled(); - }); - - it("should submit public permissions when the public radio is checked", async () => { - render(); - const email = screen.getByLabelText("Email"); - const publicRadio = screen.getByLabelText("Public"); - - await user.type(email, "test@test.test"); - await user.click(publicRadio); - await user.click(screen.getByRole("button", { name: "Submit" })); - - expect(onSubmitMock).toHaveBeenCalledWith("public", "test@test.test"); - }); - it("should call onClose when the close button is clicked", async () => { - render(); + render(); await user.click(screen.getByRole("button", { name: "Cancel" })); - expect(onSubmitMock).not.toHaveBeenCalled(); expect(onCloseMock).toHaveBeenCalled(); }); - it("should disable the buttons if isSubmitting is true", () => { - const { rerender } = render( - , - ); - const submitButton = screen.getByRole("button", { name: "Submit" }); - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - - expect(submitButton).not.toBeDisabled(); - expect(cancelButton).not.toBeDisabled(); - - rerender( - , - ); - expect(submitButton).toBeDisabled(); - expect(cancelButton).toBeDisabled(); - }); }); diff --git a/frontend/src/api/open-hands.types.ts b/frontend/src/api/open-hands.types.ts index ba0e8642bc..9da1a339b4 100644 --- a/frontend/src/api/open-hands.types.ts +++ b/frontend/src/api/open-hands.types.ts @@ -31,7 +31,7 @@ export interface Feedback { version: string; email: string; token: string; - feedback: "positive" | "negative"; + polarity: "positive" | "negative"; permissions: "public" | "private"; trajectory: unknown[]; } diff --git a/frontend/src/components/chat-interface.tsx b/frontend/src/components/chat-interface.tsx index 10786c1392..25a5707369 100644 --- a/frontend/src/components/chat-interface.tsx +++ b/frontend/src/components/chat-interface.tsx @@ -1,6 +1,5 @@ import { useDispatch, useSelector } from "react-redux"; import React from "react"; -import { useFetcher } from "@remix-run/react"; import { useSocket } from "#/context/socket"; import { convertImageToBase64 } from "#/utils/convert-image-to-base-64"; import { ChatMessage } from "./chat-message"; @@ -13,10 +12,6 @@ import { RootState } from "#/store"; import AgentState from "#/types/AgentState"; import { generateAgentStateChangeEvent } from "#/services/agentStateService"; import { FeedbackModal } from "./feedback-modal"; -import { Feedback } from "#/api/open-hands.types"; -import { getToken } from "#/services/auth"; -import { removeApiKey, removeUnwantedKeys } from "#/utils/utils"; -import { clientAction } from "#/routes/submit-feedback"; import { useScrollToBottom } from "#/hooks/useScrollToBottom"; import TypingIndicator from "./chat/TypingIndicator"; import ConfirmationButtons from "./chat/ConfirmationButtons"; @@ -24,16 +19,13 @@ import { ErrorMessage } from "./error-message"; import { ContinueButton } from "./continue-button"; import { ScrollToBottomButton } from "./scroll-to-bottom-button"; -const FEEDBACK_VERSION = "1.0"; - const isErrorMessage = ( message: Message | ErrorMessage, ): message is ErrorMessage => "error" in message; export function ChatInterface() { - const { send, events } = useSocket(); + const { send } = useSocket(); const dispatch = useDispatch(); - const fetcher = useFetcher({ key: "feedback" }); const scrollRef = React.useRef(null); const { scrollDomToBottom, onChatBodyScroll, hitBottom } = useScrollToBottom(scrollRef); @@ -44,7 +36,6 @@ export function ChatInterface() { const [feedbackPolarity, setFeedbackPolarity] = React.useState< "positive" | "negative" >("positive"); - const [feedbackShared, setFeedbackShared] = React.useState(0); const [feedbackModalIsOpen, setFeedbackModalIsOpen] = React.useState(false); const handleSendMessage = async (content: string, files: File[]) => { @@ -71,30 +62,6 @@ export function ChatInterface() { setFeedbackPolarity(polarity); }; - const handleSubmitFeedback = ( - permissions: "private" | "public", - email: string, - ) => { - const feedback: Feedback = { - version: FEEDBACK_VERSION, - feedback: feedbackPolarity, - email, - permissions, - token: getToken(), - trajectory: removeApiKey(removeUnwantedKeys(events)), - }; - - const formData = new FormData(); - formData.append("feedback", JSON.stringify(feedback)); - - fetcher.submit(formData, { - action: "/submit-feedback", - method: "POST", - }); - - setFeedbackShared(messages.length); - }; - return (
- {feedbackShared !== messages.length && messages.length > 3 && ( - - onClickShareFeedbackActionButton("positive") - } - onNegativeFeedback={() => - onClickShareFeedbackActionButton("negative") - } - /> - )} + + onClickShareFeedbackActionButton("positive") + } + onNegativeFeedback={() => + onClickShareFeedbackActionButton("negative") + } + />
{messages.length > 2 && curAgentState === AgentState.AWAITING_USER_INPUT && ( @@ -163,9 +128,8 @@ export function ChatInterface() { setFeedbackModalIsOpen(false)} - onSubmit={handleSubmitFeedback} + polarity={feedbackPolarity} />
); diff --git a/frontend/src/components/feedback-form.tsx b/frontend/src/components/feedback-form.tsx index 7ff03a4430..4e1ddde635 100644 --- a/frontend/src/components/feedback-form.tsx +++ b/frontend/src/components/feedback-form.tsx @@ -1,27 +1,87 @@ +import React from "react"; +import hotToast from "react-hot-toast"; import ModalButton from "./buttons/ModalButton"; +import { request } from "#/services/api"; +import { Feedback } from "#/api/open-hands.types"; + +const FEEDBACK_VERSION = "1.0"; +const VIEWER_PAGE = "https://www.all-hands.dev/share"; interface FeedbackFormProps { - onSubmit: (permissions: "private" | "public", email: string) => void; onClose: () => void; - isSubmitting?: boolean; + polarity: "positive" | "negative"; } -export function FeedbackForm({ - onSubmit, - onClose, - isSubmitting, -}: FeedbackFormProps) { - const handleSubmit = (event: React.FormEvent) => { +export function FeedbackForm({ onClose, polarity }: FeedbackFormProps) { + const [isSubmitting, setIsSubmitting] = React.useState(false); + + 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: 10000 }, + ); + }; + + const handleSubmit = async (event: React.FormEvent) => { event?.preventDefault(); const formData = new FormData(event.currentTarget); + setIsSubmitting(true); - const email = formData.get("email")?.toString(); - const permissions = formData.get("permissions")?.toString() as - | "private" - | "public" - | undefined; + const email = formData.get("email")?.toString() || ""; + const permissions = (formData.get("permissions")?.toString() || + "private") as "private" | "public"; - if (email) onSubmit(permissions || "private", email); + const feedback: Feedback = { + version: FEEDBACK_VERSION, + email, + polarity, + permissions, + trajectory: [], + token: "", + }; + + const response = await request("/api/submit-feedback", { + method: "POST", + body: JSON.stringify(feedback), + headers: { + "Content-Type": "application/json", + }, + }); + const { message, feedback_id, password } = response.body; // eslint-disable-line + const link = `${VIEWER_PAGE}?share_id=${feedback_id}`; + shareFeedbackToast(message, link, password); + setIsSubmitting(false); }; return ( diff --git a/frontend/src/components/feedback-modal.tsx b/frontend/src/components/feedback-modal.tsx index f9cf05f078..96663135e8 100644 --- a/frontend/src/components/feedback-modal.tsx +++ b/frontend/src/components/feedback-modal.tsx @@ -1,6 +1,4 @@ import React from "react"; -import hotToast, { toast } from "react-hot-toast"; -import { useFetcher } from "@remix-run/react"; import { FeedbackForm } from "./feedback-form"; import { BaseModalTitle, @@ -8,82 +6,18 @@ import { } from "./modals/confirmation-modals/BaseModal"; import { ModalBackdrop } from "./modals/modal-backdrop"; import ModalBody from "./modals/ModalBody"; -import { clientAction } from "#/routes/submit-feedback"; interface FeedbackModalProps { - onSubmit: (permissions: "private" | "public", email: string) => void; onClose: () => void; isOpen: boolean; - isSubmitting?: boolean; + polarity: "positive" | "negative"; } export function FeedbackModal({ - onSubmit, onClose, isOpen, - isSubmitting, + polarity, }: FeedbackModalProps) { - const fetcher = useFetcher({ key: "feedback" }); - const isInitialRender = React.useRef(true); - - 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: 10000 }, - ); - }; - - React.useEffect(() => { - if (isInitialRender.current) { - isInitialRender.current = false; - return; - } - - // Handle feedback submission - if (fetcher.state === "idle" && fetcher.data) { - if (!fetcher.data.success) { - toast.error("Error submitting feedback"); - } else if (fetcher.data.data) { - const { data } = fetcher.data; - const { message, link, password } = data; - shareFeedbackToast(message, link, password); - } - - onClose(); - } - }, [fetcher.state, fetcher.data?.success]); - if (!isOpen) return null; return ( @@ -91,11 +25,7 @@ export function FeedbackModal({ - + ); diff --git a/frontend/src/routes/submit-feedback.ts b/frontend/src/routes/submit-feedback.ts deleted file mode 100644 index 19281eea7a..0000000000 --- a/frontend/src/routes/submit-feedback.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { ClientActionFunctionArgs, json } from "@remix-run/react"; -import { Feedback } from "#/api/open-hands.types"; -import OpenHands from "#/api/open-hands"; - -const VIEWER_PAGE = "https://www.all-hands.dev/share"; - -const isFeedback = (feedback: unknown): feedback is Feedback => { - if (typeof feedback !== "object" || feedback === null) { - return false; - } - - return ( - "version" in feedback && - "email" in feedback && - "token" in feedback && - "feedback" in feedback && - "permissions" in feedback && - "trajectory" in feedback - ); -}; - -export const clientAction = async ({ request }: ClientActionFunctionArgs) => { - const formData = await request.formData(); - const feedback = formData.get("feedback")?.toString(); - const token = localStorage.getItem("token"); - - if (token && feedback) { - const parsed = JSON.parse(feedback); - if (isFeedback(parsed)) { - try { - const response = await OpenHands.sendFeedback(token, parsed); - if (response.statusCode === 200) { - const { message, feedback_id: feedbackId, password } = response.body; - const link = `${VIEWER_PAGE}?share_id=${feedbackId}`; - return json({ - success: true, - data: { message, link, password }, - }); - } - } catch (error) { - return json({ success: false, data: null }); - } - } - } - - return json({ success: false, data: null }); -}; diff --git a/openhands/events/stream.py b/openhands/events/stream.py index 1e4c3b9d53..aafbcc2fc8 100644 --- a/openhands/events/stream.py +++ b/openhands/events/stream.py @@ -71,7 +71,15 @@ class EventStream: end_id=None, reverse=False, filter_out_type: tuple[type[Event], ...] | None = None, + filter_hidden=False, ) -> Iterable[Event]: + def should_filter(event: Event): + if filter_hidden and hasattr(event, 'hidden') and event.hidden: + return True + if filter_out_type is not None and isinstance(event, filter_out_type): + return True + return False + if reverse: if end_id is None: end_id = self._cur_id - 1 @@ -79,9 +87,7 @@ class EventStream: while event_id >= start_id: try: event = self.get_event(event_id) - if filter_out_type is None or not isinstance( - event, filter_out_type - ): + if not should_filter(event): yield event except FileNotFoundError: logger.debug(f'No event found for ID {event_id}') @@ -93,9 +99,7 @@ class EventStream: break try: event = self.get_event(event_id) - if filter_out_type is None or not isinstance( - event, filter_out_type - ): + if not should_filter(event): yield event except FileNotFoundError: break diff --git a/openhands/server/data_models/feedback.py b/openhands/server/data_models/feedback.py index cbdd874480..59f32008b5 100644 --- a/openhands/server/data_models/feedback.py +++ b/openhands/server/data_models/feedback.py @@ -1,5 +1,5 @@ import json -from typing import Any, Literal +from typing import Any, Literal, Optional import requests from pydantic import BaseModel @@ -10,10 +10,12 @@ from openhands.core.logger import openhands_logger as logger class FeedbackDataModel(BaseModel): version: str email: str - token: str - feedback: Literal['positive', 'negative'] + polarity: Literal['positive', 'negative'] + feedback: Literal[ + 'positive', 'negative' + ] # TODO: remove this, its here for backward compatibility permissions: Literal['public', 'private'] - trajectory: list[dict[str, Any]] + trajectory: Optional[list[dict[str, Any]]] FEEDBACK_URL = 'https://share-od-trajectory-3u9bw9tx.uc.gateway.dev/share_od_trajectory' @@ -21,6 +23,7 @@ FEEDBACK_URL = 'https://share-od-trajectory-3u9bw9tx.uc.gateway.dev/share_od_tra def store_feedback(feedback: FeedbackDataModel) -> dict[str, str]: # Start logging + feedback.feedback = feedback.polarity display_feedback = feedback.model_dump() if 'trajectory' in display_feedback: display_feedback['trajectory'] = ( diff --git a/openhands/server/listen.py b/openhands/server/listen.py index fc740e8029..cc74d5ba73 100644 --- a/openhands/server/listen.py +++ b/openhands/server/listen.py @@ -634,14 +634,14 @@ async def upload_file(request: Request, files: list[UploadFile]): @app.post('/api/submit-feedback') -async def submit_feedback(request: Request, feedback: FeedbackDataModel): +async def submit_feedback(request: Request): """Submit user feedback. This function stores the provided feedback data. To submit feedback: ```sh - curl -X POST -F "email=test@example.com" -F "token=abc" -F "feedback=positive" -F "permissions=private" -F "trajectory={}" http://localhost:3000/api/submit-feedback + curl -X POST -d '{"email": "test@example.com"}' -H "Authorization:" ``` Args: @@ -656,6 +656,19 @@ async def submit_feedback(request: Request, feedback: FeedbackDataModel): """ # Assuming the storage service is already configured in the backend # and there is a function to handle the storage. + body = await request.json() + events = request.state.conversation.event_stream.get_events(filter_hidden=True) + trajectory = [] + for event in events: + trajectory.append(event_to_dict(event)) + feedback = FeedbackDataModel( + email=body.get('email', ''), + version=body.get('version', ''), + permissions=body.get('permissions', 'private'), + polarity=body.get('polarity', ''), + feedback=body.get('polarity', ''), + trajectory=trajectory, + ) try: feedback_data = store_feedback(feedback) return JSONResponse(status_code=200, content=feedback_data)