mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(ui): show each destination's own scope in the Edit scope dialog
Form.useForm() owns a store that outlives the dialog, and initialValues seeds it only on first mount. Reopening Edit scope for a second destination therefore left the previous one's scope in the fields while only the title changed, and Save sends the whole access object, so pressing it wrote that stale scope: a team-scoped destination silently became global and began receiving every tenant's traces. Key the modal on the destination so the component, and the form store with it, is rebuilt per destination. destroyOnHidden alone does not help (destroyOnClose was renamed in antd 5.25 and this is 5.29, but neither remounts the store the parent holds), and reseeding from an effect does not either, because the portal's children mount after the effect runs.
This commit is contained in:
parent
3e063ee252
commit
6e8a45f6da
3 changed files with 54 additions and 3 deletions
|
|
@ -33,8 +33,6 @@ const EditLoggingCredentialModal: React.FC<EditLoggingCredentialModalProps> = ({
|
|||
onClose,
|
||||
onSaved,
|
||||
}) => {
|
||||
// destroyOnClose remounts the Form each open, so initialValues re-seeds from the
|
||||
// current destination -- no effect syncing prop into state.
|
||||
const [form] = Form.useForm<AccessForm>();
|
||||
|
||||
const handleSave = async () => {
|
||||
|
|
@ -65,7 +63,7 @@ const EditLoggingCredentialModal: React.FC<EditLoggingCredentialModalProps> = ({
|
|||
onCancel={onClose}
|
||||
onOk={handleSave}
|
||||
okText="Save"
|
||||
destroyOnClose
|
||||
destroyOnHidden
|
||||
>
|
||||
<Form<AccessForm> form={form} layout="vertical" preserve={false} initialValues={{ access: access ?? {} }}>
|
||||
<Form.Item name="access" noStyle>
|
||||
|
|
|
|||
|
|
@ -258,6 +258,50 @@ describe("Settings", () => {
|
|||
expect(getByText("well-formed")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Regression: the Edit scope dialog is driven by a Form store that Form.useForm() owns.
|
||||
// initialValues only seeds that store on first mount, so reopening the dialog for a second
|
||||
// destination left the first one's scope in the fields while only the title changed. Save
|
||||
// sends the whole access object, so pressing it wrote the stale scope -- silently turning a
|
||||
// team-scoped destination global and leaking every tenant's traces to it. The parent keys
|
||||
// the modal per destination so the store is rebuilt each time.
|
||||
it("should show each destination's own scope when Edit scope is reopened for another one", async () => {
|
||||
const user = userEvent.setup();
|
||||
credentialsFixture = {
|
||||
credentials: [
|
||||
{
|
||||
credential_name: "global-dest",
|
||||
credential_info: { credential_type: "logging", description: "generic", access: { global: true } },
|
||||
},
|
||||
{
|
||||
credential_name: "team-dest",
|
||||
credential_info: { credential_type: "logging", description: "generic", access: { teams: ["team-1"] } },
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const { findByText } = renderSettings(defaultProps);
|
||||
await findByText("Active Logging Callbacks");
|
||||
|
||||
const openEditScope = async (name: string) => {
|
||||
await user.click(await screen.findByTestId(`callback-actions-${name}-success`));
|
||||
await user.click(await screen.findByTestId("destination-action-edit-access"));
|
||||
return await screen.findByText(`Edit scope — ${name}`);
|
||||
};
|
||||
const globalSwitch = () => document.querySelector(".ant-modal .ant-switch");
|
||||
const closeDialog = async () => {
|
||||
await user.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
await waitFor(() => expect(document.querySelector(".ant-modal-title")).not.toBeInTheDocument());
|
||||
};
|
||||
|
||||
await openEditScope("global-dest");
|
||||
expect(globalSwitch()).toHaveAttribute("aria-checked", "true");
|
||||
await closeDialog();
|
||||
|
||||
await openEditScope("team-dest");
|
||||
// Before the fix this read "true", carried over from global-dest.
|
||||
expect(globalSwitch()).toHaveAttribute("aria-checked", "false");
|
||||
});
|
||||
|
||||
it("should hold the callbacks table in loading state until the fetch settles", async () => {
|
||||
let resolveCallbacks: (value: {
|
||||
callbacks: never[];
|
||||
|
|
|
|||
|
|
@ -590,6 +590,15 @@ const Settings: React.FC<SettingsPageProps> = ({ accessToken, userRole, userID,
|
|||
/>
|
||||
{accessToken && (
|
||||
<EditLoggingCredentialModal
|
||||
// Remount per destination. Form.useForm() owns a store that outlives the
|
||||
// dialog and initialValues only seeds it on first mount, so reopening for
|
||||
// another credential kept the previous one's scope in the fields while the
|
||||
// title updated -- and Save sends the whole access object, so that stale
|
||||
// scope was written, silently turning a team destination global.
|
||||
// Neither destroyOnHidden nor resetting the fields from an effect fixes
|
||||
// this (the portal's children mount after the effect runs); remounting the
|
||||
// component, and with it the form store, is what works. Verified by A/B.
|
||||
key={editAccessFor?.name ?? "none"}
|
||||
accessToken={accessToken}
|
||||
credentialName={editAccessFor?.name ?? null}
|
||||
access={editAccessFor?.access}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue