mirror of
https://github.com/aaif-goose/goose.git
synced 2026-06-02 06:14:27 +02:00
fix(ui): preserve pending env vars in Add Extension form (#9285)
Signed-off-by: UGBOMEH OGOCHUKWU WILLIAMS <williamsugbomeh@gmail.com> Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
committed by
GitHub
parent
d017295f32
commit
4f43ae4cd0
@@ -42,7 +42,10 @@ interface EnvVarsSectionProps {
|
|||||||
onRemove: (index: number) => void;
|
onRemove: (index: number) => void;
|
||||||
onChange: (index: number, field: 'key' | 'value', value: string) => void;
|
onChange: (index: number, field: 'key' | 'value', value: string) => void;
|
||||||
submitAttempted: boolean;
|
submitAttempted: boolean;
|
||||||
onPendingInputChange?: (hasPendingInput: boolean) => void;
|
onPendingInputChange: (
|
||||||
|
hasPendingInput: boolean,
|
||||||
|
pendingEnvVar: { key: string; value: string } | null
|
||||||
|
) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
export default function EnvVarsSection({
|
export default function EnvVarsSection({
|
||||||
@@ -65,7 +68,9 @@ export default function EnvVarsSection({
|
|||||||
// Notify parent when pending input changes
|
// Notify parent when pending input changes
|
||||||
React.useEffect(() => {
|
React.useEffect(() => {
|
||||||
const hasPendingInput = newKey.trim() !== '' || newValue.trim() !== '';
|
const hasPendingInput = newKey.trim() !== '' || newValue.trim() !== '';
|
||||||
onPendingInputChange?.(hasPendingInput);
|
const pendingEnvVar =
|
||||||
|
newKey.trim() && newValue.trim() ? { key: newKey, value: newValue } : null;
|
||||||
|
onPendingInputChange(hasPendingInput, pendingEnvVar);
|
||||||
}, [newKey, newValue, onPendingInputChange]);
|
}, [newKey, newValue, onPendingInputChange]);
|
||||||
|
|
||||||
const handleAdd = () => {
|
const handleAdd = () => {
|
||||||
|
|||||||
@@ -1,9 +1,20 @@
|
|||||||
import { describe, it, expect, vi } from 'vitest';
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
import { render, type RenderOptions, screen, waitFor } from '@testing-library/react';
|
import { render, type RenderOptions, screen, waitFor } from '@testing-library/react';
|
||||||
import userEvent from '@testing-library/user-event';
|
import userEvent from '@testing-library/user-event';
|
||||||
import ExtensionModal from './ExtensionModal';
|
import ExtensionModal from './ExtensionModal';
|
||||||
import { ExtensionFormData } from '../utils';
|
import { ExtensionFormData } from '../utils';
|
||||||
import { IntlTestWrapper } from '../../../../i18n/test-utils';
|
import { IntlTestWrapper } from '../../../../i18n/test-utils';
|
||||||
|
import { upsertConfig } from '../../../../api';
|
||||||
|
|
||||||
|
vi.mock('../../../../api', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('../../../../api')>('../../../../api');
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
upsertConfig: vi.fn().mockResolvedValue({ data: 'ok' }),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
const mockedUpsertConfig = vi.mocked(upsertConfig);
|
||||||
|
|
||||||
const renderWithIntl = (ui: React.ReactElement, options?: RenderOptions) =>
|
const renderWithIntl = (ui: React.ReactElement, options?: RenderOptions) =>
|
||||||
render(ui, { wrapper: IntlTestWrapper, ...options });
|
render(ui, { wrapper: IntlTestWrapper, ...options });
|
||||||
@@ -246,4 +257,140 @@ describe('ExtensionModal', () => {
|
|||||||
{ key: 'Authorization', value: 'Bearer abc123', isEdited: true },
|
{ key: 'Authorization', value: 'Bearer abc123', isEdited: true },
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('pending env var capture (fix for #8969)', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
mockedUpsertConfig.mockClear();
|
||||||
|
mockedUpsertConfig.mockResolvedValue({
|
||||||
|
data: 'ok',
|
||||||
|
error: undefined,
|
||||||
|
request: new globalThis.Request('http://localhost/test'),
|
||||||
|
response: new globalThis.Response(),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
const emptyInitialData: ExtensionFormData = {
|
||||||
|
name: '',
|
||||||
|
description: '',
|
||||||
|
type: 'stdio',
|
||||||
|
cmd: '',
|
||||||
|
endpoint: '',
|
||||||
|
enabled: true,
|
||||||
|
timeout: 300,
|
||||||
|
envVars: [],
|
||||||
|
headers: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
// Returns the env-var key+value inputs (scoped to the "Environment Variables" section,
|
||||||
|
// disambiguated from the header inputs which share the "Value" placeholder).
|
||||||
|
function getEnvVarInputs() {
|
||||||
|
const envVarKeyInput = screen.getByPlaceholderText('Variable name');
|
||||||
|
const envVarValueInput = screen
|
||||||
|
.getAllByPlaceholderText('Value')
|
||||||
|
.find((input) =>
|
||||||
|
input.parentElement?.parentElement?.parentElement?.textContent?.includes(
|
||||||
|
'Environment Variables'
|
||||||
|
)
|
||||||
|
);
|
||||||
|
return { envVarKeyInput, envVarValueInput };
|
||||||
|
}
|
||||||
|
|
||||||
|
it('captures a pending env var typed but not "+ Added" when Submit is clicked', async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const mockOnSubmit = vi.fn();
|
||||||
|
const mockOnClose = vi.fn();
|
||||||
|
|
||||||
|
renderWithIntl(
|
||||||
|
<ExtensionModal
|
||||||
|
title="Add custom extension"
|
||||||
|
initialData={emptyInitialData}
|
||||||
|
onClose={mockOnClose}
|
||||||
|
onSubmit={mockOnSubmit}
|
||||||
|
submitLabel="Add Extension"
|
||||||
|
modalType="add"
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.type(screen.getByPlaceholderText('Enter extension name...'), 'WooMCP');
|
||||||
|
await user.type(
|
||||||
|
screen.getByPlaceholderText(/^e\.g\. npx/),
|
||||||
|
'npx -y @automattic/mcp-wordpress-remote@latest'
|
||||||
|
);
|
||||||
|
|
||||||
|
const { envVarKeyInput, envVarValueInput } = getEnvVarInputs();
|
||||||
|
await user.type(envVarKeyInput, 'JWT_TOKEN');
|
||||||
|
if (envVarValueInput) {
|
||||||
|
await user.type(envVarValueInput, 'my_very_long_token');
|
||||||
|
}
|
||||||
|
|
||||||
|
// Note: intentionally NOT clicking the "+ Add" button — this is the #8969 repro.
|
||||||
|
await user.click(screen.getByTestId('extension-submit-btn'));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(mockOnSubmit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockedUpsertConfig).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
body: expect.objectContaining({
|
||||||
|
is_secret: true,
|
||||||
|
key: 'JWT_TOKEN',
|
||||||
|
value: 'my_very_long_token',
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
const submittedData = mockOnSubmit.mock.calls[0][0];
|
||||||
|
expect(submittedData.envVars).toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
expect.objectContaining({
|
||||||
|
key: 'JWT_TOKEN',
|
||||||
|
value: 'my_very_long_token',
|
||||||
|
isEdited: true,
|
||||||
|
}),
|
||||||
|
])
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not capture a pending env var when only the key is filled', async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const mockOnSubmit = vi.fn();
|
||||||
|
const mockOnClose = vi.fn();
|
||||||
|
|
||||||
|
renderWithIntl(
|
||||||
|
<ExtensionModal
|
||||||
|
title="Add custom extension"
|
||||||
|
initialData={emptyInitialData}
|
||||||
|
onClose={mockOnClose}
|
||||||
|
onSubmit={mockOnSubmit}
|
||||||
|
submitLabel="Add Extension"
|
||||||
|
modalType="add"
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.type(screen.getByPlaceholderText('Enter extension name...'), 'WooMCP');
|
||||||
|
await user.type(screen.getByPlaceholderText(/^e\.g\. npx/), 'npx -y something');
|
||||||
|
|
||||||
|
const { envVarKeyInput } = getEnvVarInputs();
|
||||||
|
await user.type(envVarKeyInput, 'LONELY_KEY');
|
||||||
|
// Intentionally leaving the value field empty.
|
||||||
|
|
||||||
|
await user.click(screen.getByTestId('extension-submit-btn'));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(mockOnSubmit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockedUpsertConfig).not.toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
body: expect.objectContaining({ key: 'LONELY_KEY' }),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
const submittedData = mockOnSubmit.mock.calls[0][0];
|
||||||
|
expect(submittedData.envVars).not.toEqual(
|
||||||
|
expect.arrayContaining([expect.objectContaining({ key: 'LONELY_KEY' })])
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -83,6 +83,7 @@ export default function ExtensionModal({
|
|||||||
const [submitAttempted, setSubmitAttempted] = useState(false);
|
const [submitAttempted, setSubmitAttempted] = useState(false);
|
||||||
const [showCloseConfirmation, setShowCloseConfirmation] = useState(false);
|
const [showCloseConfirmation, setShowCloseConfirmation] = useState(false);
|
||||||
const [hasPendingEnvVars, setHasPendingEnvVars] = useState(false);
|
const [hasPendingEnvVars, setHasPendingEnvVars] = useState(false);
|
||||||
|
const [pendingEnvVar, setPendingEnvVar] = useState<{ key: string; value: string } | null>(null);
|
||||||
const [hasPendingHeaders, setHasPendingHeaders] = useState(false);
|
const [hasPendingHeaders, setHasPendingHeaders] = useState(false);
|
||||||
const [pendingHeader, setPendingHeader] = useState<{ key: string; value: string } | null>(null);
|
const [pendingHeader, setPendingHeader] = useState<{ key: string; value: string } | null>(null);
|
||||||
|
|
||||||
@@ -232,6 +233,14 @@ export default function ExtensionModal({
|
|||||||
[]
|
[]
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const handlePendingEnvVarChange = useCallback(
|
||||||
|
(hasPending: boolean, envVar: { key: string; value: string } | null) => {
|
||||||
|
setHasPendingEnvVars(hasPending);
|
||||||
|
setPendingEnvVar(envVar);
|
||||||
|
},
|
||||||
|
[]
|
||||||
|
);
|
||||||
|
|
||||||
// Function to store a secret value
|
// Function to store a secret value
|
||||||
const storeSecret = async (key: string, value: string) => {
|
const storeSecret = async (key: string, value: string) => {
|
||||||
try {
|
try {
|
||||||
@@ -289,6 +298,19 @@ export default function ExtensionModal({
|
|||||||
return finalHeaders;
|
return finalHeaders;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const getFinalEnvVars = () => {
|
||||||
|
const finalEnvVars = [...formData.envVars];
|
||||||
|
if (
|
||||||
|
pendingEnvVar &&
|
||||||
|
pendingEnvVar.key.trim() !== '' &&
|
||||||
|
pendingEnvVar.value.trim() !== '' &&
|
||||||
|
!pendingEnvVar.key.includes(' ')
|
||||||
|
) {
|
||||||
|
finalEnvVars.push({ ...pendingEnvVar, isEdited: true });
|
||||||
|
}
|
||||||
|
return finalEnvVars;
|
||||||
|
};
|
||||||
|
|
||||||
const isHeadersValid = () => {
|
const isHeadersValid = () => {
|
||||||
return getFinalHeaders().every(
|
return getFinalHeaders().every(
|
||||||
({ key, value }) => (key === '' && value === '') || (key !== '' && value !== '')
|
({ key, value }) => (key === '' && value === '') || (key !== '' && value !== '')
|
||||||
@@ -323,6 +345,7 @@ export default function ExtensionModal({
|
|||||||
if (isFormValid()) {
|
if (isFormValid()) {
|
||||||
const finalFormData = {
|
const finalFormData = {
|
||||||
...formData,
|
...formData,
|
||||||
|
envVars: getFinalEnvVars(),
|
||||||
headers: getFinalHeaders(),
|
headers: getFinalHeaders(),
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -437,7 +460,7 @@ export default function ExtensionModal({
|
|||||||
onRemove={handleRemoveEnvVar}
|
onRemove={handleRemoveEnvVar}
|
||||||
onChange={handleEnvVarChange}
|
onChange={handleEnvVarChange}
|
||||||
submitAttempted={submitAttempted}
|
submitAttempted={submitAttempted}
|
||||||
onPendingInputChange={setHasPendingEnvVars}
|
onPendingInputChange={handlePendingEnvVarChange}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
</>
|
</>
|
||||||
|
|||||||
Reference in New Issue
Block a user