Remove hidden commands from feedback (#4597)

Co-authored-by: Xingyao Wang <[email protected]> Co-authored-by: Xingyao Wang <[email protected]> Co-authored-by: Graham Neubig <[email protected]>
All-Hands-AI · Oct 31, 2024 · e17f7b2 · e17f7b2
1 parent ce6939f
commit e17f7b2
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 255 deletions.
diff --git a/frontend/__tests__/components/feedback-form.test.tsx b/frontend/__tests__/components/feedback-form.test.tsx
@@ -5,15 +5,14 @@ import { FeedbackForm } from "#/components/feedback-form";
 
 describe("FeedbackForm", () => {
   const user = userEvent.setup();
-  const onSubmitMock = vi.fn();
   const onCloseMock = vi.fn();
 
   afterEach(() => {
     vi.clearAllMocks();
   });
 
   it("should render correctly", () => {
-    render(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
+    render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
 
     screen.getByLabelText("Email");
     screen.getByLabelText("Private");
@@ -24,7 +23,7 @@ describe("FeedbackForm", () => {
   });
 
   it("should switch between private and public permissions", async () => {
-    render(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
+    render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
     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(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
-    const email = screen.getByLabelText("Email");
-
-    await user.type(email, "[email protected]");
-    await user.click(screen.getByRole("button", { name: "Submit" }));
-
-    expect(onSubmitMock).toHaveBeenCalledWith("private", "[email protected]"); // private is the default value
-  });
-
-  it("should not call onSubmit when the email is invalid", async () => {
-    render(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
-    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(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
-    const email = screen.getByLabelText("Email");
-    const publicRadio = screen.getByLabelText("Public");
-
-    await user.type(email, "[email protected]");
-    await user.click(publicRadio);
-    await user.click(screen.getByRole("button", { name: "Submit" }));
-
-    expect(onSubmitMock).toHaveBeenCalledWith("public", "[email protected]");
-  });
-
   it("should call onClose when the close button is clicked", async () => {
-    render(<FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />);
+    render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
     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(
-      <FeedbackForm onSubmit={onSubmitMock} onClose={onCloseMock} />,
-    );
-    const submitButton = screen.getByRole("button", { name: "Submit" });
-    const cancelButton = screen.getByRole("button", { name: "Cancel" });
-
-    expect(submitButton).not.toBeDisabled();
-    expect(cancelButton).not.toBeDisabled();
-
-    rerender(
-      <FeedbackForm
-        onSubmit={onSubmitMock}
-        onClose={onCloseMock}
-        isSubmitting
-      />,
-    );
-    expect(submitButton).toBeDisabled();
-    expect(cancelButton).toBeDisabled();
-  });
 });
diff --git 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
@@ -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,27 +12,20 @@ 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";
 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<typeof clientAction>({ key: "feedback" });
   const scrollRef = React.useRef<HTMLDivElement>(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 (
     <div className="h-full flex flex-col justify-between">
       <div
@@ -130,16 +97,14 @@ export function ChatInterface() {
 
       <div className="flex flex-col gap-[6px] px-4 pb-4">
         <div className="flex justify-between relative">
-          {feedbackShared !== messages.length && messages.length > 3 && (
-            <FeedbackActions
-              onPositiveFeedback={() =>
-                onClickShareFeedbackActionButton("positive")
-              }
-              onNegativeFeedback={() =>
-                onClickShareFeedbackActionButton("negative")
-              }
-            />
-          )}
+          <FeedbackActions
+            onPositiveFeedback={() =>
+              onClickShareFeedbackActionButton("positive")
+            }
+            onNegativeFeedback={() =>
+              onClickShareFeedbackActionButton("negative")
+            }
+          />
           <div className="absolute left-1/2 transform -translate-x-1/2 bottom-0">
             {messages.length > 2 &&
               curAgentState === AgentState.AWAITING_USER_INPUT && (
@@ -163,9 +128,8 @@ export function ChatInterface() {
 
       <FeedbackModal
         isOpen={feedbackModalIsOpen}
-        isSubmitting={fetcher.state === "submitting"}
         onClose={() => setFeedbackModalIsOpen(false)}
-        onSubmit={handleSubmitFeedback}
+        polarity={feedbackPolarity}
       />
     </div>
   );

diff --git 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<HTMLFormElement>) => {
+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(
+      <div className="flex flex-col gap-1">
+        <span>{message}</span>
+        <a
+          data-testid="toast-share-url"
+          className="text-blue-500 underline"
+          onClick={() => onPressToast(password)}
+          href={link}
+          target="_blank"
+          rel="noreferrer"
+        >
+          Go to shared feedback
+        </a>
+        <span onClick={() => onPressToast(password)} className="cursor-pointer">
+          Password: {password} <span className="text-gray-500">(copy)</span>
+        </span>
+      </div>,
+      { duration: 10000 },
+    );
+  };
+
+  const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
     event?.preventDefault();
     const formData = new FormData(event.currentTarget);
+    setIsSubmitting(true);
+
+    const email = formData.get("email")?.toString() || "";
+    const permissions = (formData.get("permissions")?.toString() ||
+      "private") as "private" | "public";
 
-    const email = formData.get("email")?.toString();
-    const permissions = formData.get("permissions")?.toString() as
-      | "private"
-      | "public"
-      | undefined;
+    const feedback: Feedback = {
+      version: FEEDBACK_VERSION,
+      email,
+      polarity,
+      permissions,
+      trajectory: [],
+      token: "",
+    };
 
-    if (email) onSubmit(permissions || "private", email);
+    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 (