Disable send button while approval is pending (#1368)
Fixes #912 This PR implements disabling send button while approval is pending and addresses issue #912 <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Disable the chat send button while a proposal is awaiting approval, and re-enable it after approve or reject. Prevents accidental messages during pending changes. Addresses issue #912. - **New Features** - Track pending changes with isChangesPending based on the last assistant message’s approvalState. - Disable the send button when a proposal is pending (in addition to the existing empty input check). - Re-enable after approve/reject by refreshing the proposal and messages. - Added Playwright e2e tests for both approve and reject flows. <!-- End of auto-generated description by cubic. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Disable the chat send button when a code proposal is pending approval and re-enable after approve/reject; add e2e coverage and update MCP flow. > > - **Frontend** > - `ChatInput.tsx`: Read `messages` from `chatMessagesAtom` and derive `disableSendButton` when the last assistant message (matching `proposal.messageId`) has no `approvalState` and `proposal.type === "code-proposal"`. > - Apply `disableSendButton` to the send button’s `disabled` condition (in addition to empty input/attachments). > - Ensure proposal/messages refresh after approve/reject. > - **Tests** > - Add Playwright tests `e2e-tests/chat_input.spec.ts` to verify send button disabled during pending proposal and re-enabled after approve or reject. > - Update `e2e-tests/mcp.spec.ts` to click "Approve" after granting consent. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b9b47bd6f547449cc5cf1d39a00e4e7fb5de1bcd. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Will Chen <willchen90@gmail.com>
This commit is contained in:
committed by
GitHub
parent
d96e95c1da
commit
582793eba0
50
e2e-tests/chat_input.spec.ts
Normal file
50
e2e-tests/chat_input.spec.ts
Normal file
@@ -0,0 +1,50 @@
|
|||||||
|
import { test } from "./helpers/test_helper";
|
||||||
|
import { expect } from "@playwright/test";
|
||||||
|
|
||||||
|
test("send button disabled during pending proposal", async ({ po }) => {
|
||||||
|
await po.setUp();
|
||||||
|
|
||||||
|
// Send a prompt that generates a proposal
|
||||||
|
await po.sendPrompt("Create a simple React component");
|
||||||
|
|
||||||
|
// Wait for proposal buttons to appear (ensuring proposal is rendered)
|
||||||
|
await expect(po.page.getByTestId("approve-proposal-button")).toBeVisible();
|
||||||
|
|
||||||
|
// Type something in the input to ensure it's not disabled due to empty input
|
||||||
|
await po.getChatInput().fill("test message");
|
||||||
|
|
||||||
|
// Check send button is disabled due to pending changes
|
||||||
|
const sendButton = po.page.getByRole("button", { name: "Send message" });
|
||||||
|
await expect(sendButton).toBeDisabled();
|
||||||
|
|
||||||
|
// Approve the proposal
|
||||||
|
await po.approveProposal();
|
||||||
|
|
||||||
|
// Check send button is enabled again
|
||||||
|
await expect(sendButton).toBeEnabled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("send button disabled during pending proposal - reject", async ({
|
||||||
|
po,
|
||||||
|
}) => {
|
||||||
|
await po.setUp();
|
||||||
|
|
||||||
|
// Send a prompt that generates a proposal
|
||||||
|
await po.sendPrompt("Create a simple React component");
|
||||||
|
|
||||||
|
// Wait for proposal buttons to appear (ensuring proposal is rendered)
|
||||||
|
await expect(po.page.getByTestId("reject-proposal-button")).toBeVisible();
|
||||||
|
|
||||||
|
// Type something in the input to ensure it's not disabled due to empty input
|
||||||
|
await po.getChatInput().fill("test message");
|
||||||
|
|
||||||
|
// Check send button is disabled due to pending changes
|
||||||
|
const sendButton = po.page.getByRole("button", { name: "Send message" });
|
||||||
|
await expect(sendButton).toBeDisabled();
|
||||||
|
|
||||||
|
// Reject the proposal
|
||||||
|
await po.rejectProposal();
|
||||||
|
|
||||||
|
// Check send button is enabled again
|
||||||
|
await expect(sendButton).toBeEnabled();
|
||||||
|
});
|
||||||
@@ -43,6 +43,7 @@ test("mcp - call calculator", async ({ po }) => {
|
|||||||
// Make sure the tool call doesn't execute until consent is given
|
// Make sure the tool call doesn't execute until consent is given
|
||||||
await po.snapshotMessages();
|
await po.snapshotMessages();
|
||||||
await alwaysAllowButton.click();
|
await alwaysAllowButton.click();
|
||||||
|
await po.page.getByRole("button", { name: "Approve" }).click();
|
||||||
|
|
||||||
await po.sendPrompt("[dump]");
|
await po.sendPrompt("[dump]");
|
||||||
await po.snapshotServerDump("all-messages");
|
await po.snapshotServerDump("all-messages");
|
||||||
|
|||||||
@@ -79,7 +79,7 @@ export function ChatInput({ chatId }: { chatId?: number }) {
|
|||||||
const [showError, setShowError] = useState(true);
|
const [showError, setShowError] = useState(true);
|
||||||
const [isApproving, setIsApproving] = useState(false); // State for approving
|
const [isApproving, setIsApproving] = useState(false); // State for approving
|
||||||
const [isRejecting, setIsRejecting] = useState(false); // State for rejecting
|
const [isRejecting, setIsRejecting] = useState(false); // State for rejecting
|
||||||
const [, setMessages] = useAtom<Message[]>(chatMessagesAtom);
|
const [messages, setMessages] = useAtom<Message[]>(chatMessagesAtom);
|
||||||
const setIsPreviewOpen = useSetAtom(isPreviewOpenAtom);
|
const setIsPreviewOpen = useSetAtom(isPreviewOpenAtom);
|
||||||
const [showTokenBar, setShowTokenBar] = useAtom(showTokenBarAtom);
|
const [showTokenBar, setShowTokenBar] = useAtom(showTokenBarAtom);
|
||||||
const [selectedComponent, setSelectedComponent] = useAtom(
|
const [selectedComponent, setSelectedComponent] = useAtom(
|
||||||
@@ -108,6 +108,14 @@ export function ChatInput({ chatId }: { chatId?: number }) {
|
|||||||
} = useProposal(chatId);
|
} = useProposal(chatId);
|
||||||
const { proposal, messageId } = proposalResult ?? {};
|
const { proposal, messageId } = proposalResult ?? {};
|
||||||
|
|
||||||
|
const lastMessage = messages.at(-1);
|
||||||
|
const disableSendButton =
|
||||||
|
lastMessage?.role === "assistant" &&
|
||||||
|
!lastMessage.approvalState &&
|
||||||
|
!!proposal &&
|
||||||
|
proposal.type === "code-proposal" &&
|
||||||
|
messageId === lastMessage.id;
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (error) {
|
if (error) {
|
||||||
setShowError(true);
|
setShowError(true);
|
||||||
@@ -214,7 +222,6 @@ export function ChatInput({ chatId }: { chatId?: number }) {
|
|||||||
setError((err as Error)?.message || "An error occurred while rejecting");
|
setError((err as Error)?.message || "An error occurred while rejecting");
|
||||||
} finally {
|
} finally {
|
||||||
setIsRejecting(false);
|
setIsRejecting(false);
|
||||||
|
|
||||||
// Keep same as handleApprove
|
// Keep same as handleApprove
|
||||||
refreshProposal();
|
refreshProposal();
|
||||||
fetchChatMessages();
|
fetchChatMessages();
|
||||||
@@ -307,7 +314,10 @@ export function ChatInput({ chatId }: { chatId?: number }) {
|
|||||||
) : (
|
) : (
|
||||||
<button
|
<button
|
||||||
onClick={handleSubmit}
|
onClick={handleSubmit}
|
||||||
disabled={!inputValue.trim() && attachments.length === 0}
|
disabled={
|
||||||
|
(!inputValue.trim() && attachments.length === 0) ||
|
||||||
|
disableSendButton
|
||||||
|
}
|
||||||
className="px-2 py-2 mt-1 mr-1 hover:bg-(--background-darkest) text-(--sidebar-accent-fg) rounded-lg disabled:opacity-50"
|
className="px-2 py-2 mt-1 mr-1 hover:bg-(--background-darkest) text-(--sidebar-accent-fg) rounded-lg disabled:opacity-50"
|
||||||
title="Send message"
|
title="Send message"
|
||||||
>
|
>
|
||||||
|
|||||||
Reference in New Issue
Block a user