Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove hidden commands from feedback #4597

Merged
merged 21 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 3 additions & 62 deletions frontend/__tests__/components/feedback-form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");

Expand All @@ -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();
});
});
2 changes: 1 addition & 1 deletion frontend/src/api/open-hands.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface Feedback {
version: string;
email: string;
token: string;
feedback: "positive" | "negative";
polarity: "positive" | "negative";
permissions: "public" | "private";
trajectory: unknown[];
}
Expand Down
56 changes: 10 additions & 46 deletions frontend/src/components/chat-interface.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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);
Expand All @@ -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[]) => {
Expand All @@ -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
Expand Down Expand Up @@ -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 && (
Expand All @@ -163,9 +128,8 @@ export function ChatInterface() {

<FeedbackModal
isOpen={feedbackModalIsOpen}
isSubmitting={fetcher.state === "submitting"}
onClose={() => setFeedbackModalIsOpen(false)}
onSubmit={handleSubmitFeedback}
polarity={feedbackPolarity}
/>
</div>
);
Expand Down
88 changes: 74 additions & 14 deletions frontend/src/components/feedback-form.tsx
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
Loading
Loading