From 120046828c40cf5368d16c2cdfd363ebf0b287cf Mon Sep 17 00:00:00 2001 From: Katherine Date: Tue, 28 Jul 2026 19:28:35 +0800 Subject: [PATCH] fix(plugin-environment-variables): show v2 submit errors (#10181) * fix(plugin-environment-variables): show v2 submit errors * fix(plugin-environment-variables): exclude secrets from value filters --- .../__tests__/EnvironmentPage.test.tsx | 114 ++++++++++++++++++ .../src/client-v2/pages/EnvironmentPage.tsx | 56 +++++++-- .../src/server/__tests__/plugin.test.ts | 51 ++++++++ .../src/server/plugin.ts | 32 ++++- 4 files changed, 239 insertions(+), 14 deletions(-) create mode 100644 packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/__tests__/EnvironmentPage.test.tsx create mode 100644 packages/plugins/@nocobase/plugin-environment-variables/src/server/__tests__/plugin.test.ts diff --git a/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/__tests__/EnvironmentPage.test.tsx b/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/__tests__/EnvironmentPage.test.tsx new file mode 100644 index 00000000000..fc876d5dafa --- /dev/null +++ b/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/__tests__/EnvironmentPage.test.tsx @@ -0,0 +1,114 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { App } from 'antd'; +import React from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { BulkImportForm, VariableForm } from '../pages/EnvironmentPage'; + +const state = vi.hoisted(() => ({ + close: vi.fn(), + notificationError: vi.fn(), + request: vi.fn(), + toErrMessages: vi.fn(), +})); + +vi.mock('@nocobase/flow-engine', async (importOriginal) => { + const actual = await importOriginal(); + const view = { + close: state.close, + Footer: ({ children }: { children: React.ReactNode }) =>
{children}
, + }; + const context = { + api: { + request: state.request, + toErrMessages: state.toErrMessages, + }, + }; + + return { + ...actual, + useFlowContext: () => context, + useFlowEngine: () => ({ context: { t: (key: string) => key } }), + useFlowView: () => view, + }; +}); + +describe('plugin-environment-variables client-v2 forms', () => { + beforeEach(() => { + state.close.mockReset(); + state.notificationError.mockReset(); + state.request.mockReset(); + state.toErrMessages.mockReset(); + vi.spyOn(App, 'useApp').mockReturnValue({ + notification: { error: state.notificationError }, + } as ReturnType); + }); + + it('shows the API error and keeps the drawer open when creating a duplicate variable', async () => { + const duplicateError = new Error('Request failed with status code 400'); + state.request.mockRejectedValue(duplicateError); + state.toErrMessages.mockReturnValue([{ message: 'Name already exists' }]); + const onSubmitted = vi.fn(); + + render(); + + fireEvent.change(screen.getByLabelText('Name :'), { target: { value: 'DUPLICATE_NAME' } }); + fireEvent.change(screen.getByLabelText('Value :'), { target: { value: 'value' } }); + fireEvent.click(screen.getByRole('button', { name: 'Submit' })); + + await waitFor(() => { + expect(state.notificationError).toHaveBeenCalledWith({ message: 'Name already exists' }); + }); + expect(state.toErrMessages).toHaveBeenCalledWith(duplicateError); + expect(onSubmitted).not.toHaveBeenCalled(); + expect(state.close).not.toHaveBeenCalled(); + }); + + it('refreshes the list and closes the drawer after creating a variable successfully', async () => { + state.request.mockResolvedValue({ data: { data: {} } }); + const onSubmitted = vi.fn(); + + render(); + + fireEvent.change(screen.getByLabelText('Name :'), { target: { value: 'NEW_NAME' } }); + fireEvent.change(screen.getByLabelText('Value :'), { target: { value: 'value' } }); + fireEvent.click(screen.getByRole('button', { name: 'Submit' })); + + await waitFor(() => { + expect(state.request).toHaveBeenCalledWith({ + url: 'environmentVariables:create', + method: 'post', + data: { name: 'NEW_NAME', type: 'default', value: 'value' }, + }); + }); + expect(onSubmitted).toHaveBeenCalledTimes(1); + expect(state.close).toHaveBeenCalledTimes(1); + expect(state.notificationError).not.toHaveBeenCalled(); + }); + + it('shows the API error and preserves bulk import input when a duplicate variable is included', async () => { + state.request.mockRejectedValue(new Error('Request failed with status code 400')); + state.toErrMessages.mockReturnValue([{ message: 'Name already exists' }]); + const onSubmitted = vi.fn(); + + render(); + + fireEvent.change(screen.getByLabelText('Plain text :'), { target: { value: 'DUPLICATE_NAME=value' } }); + fireEvent.click(screen.getByRole('button', { name: 'Submit' })); + + await waitFor(() => { + expect(state.notificationError).toHaveBeenCalledWith({ message: 'Name already exists' }); + }); + expect(screen.getByLabelText('Plain text :')).toHaveValue('DUPLICATE_NAME=value'); + expect(onSubmitted).not.toHaveBeenCalled(); + expect(state.close).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/pages/EnvironmentPage.tsx b/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/pages/EnvironmentPage.tsx index 05d92883773..f8e3b0858c1 100644 --- a/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/pages/EnvironmentPage.tsx +++ b/packages/plugins/@nocobase/plugin-environment-variables/src/client-v2/pages/EnvironmentPage.tsx @@ -40,6 +40,20 @@ type FilterGroupValue = { items: any[]; }; +function getFirstErrorMessage(errors: unknown): string | undefined { + if (!Array.isArray(errors) || !errors.length) { + return; + } + + const firstError = errors[0]; + if (typeof firstError === 'string') { + return firstError; + } + if (typeof firstError === 'object' && firstError !== null && 'message' in firstError) { + return typeof firstError.message === 'string' ? firstError.message : undefined; + } +} + const drawerTitleClassName = css` display: inline-flex; align-items: center; @@ -307,7 +321,7 @@ const EnvironmentPage: React.FC = observer(() => { ); }); -function VariableForm(props: { +export function VariableForm(props: { mode: 'create' | 'edit'; initialValues?: EnvVariable; onSubmitted: () => void; @@ -317,6 +331,7 @@ function VariableForm(props: { const t = useT(); const ctx = useFlowContext(); const view = useFlowView(); + const { notification } = App.useApp(); const [form] = Form.useForm(); const [submitting, setSubmitting] = useState(false); @@ -324,22 +339,29 @@ function VariableForm(props: { const values = await form.validateFields(); setSubmitting(true); try { - if (mode === 'create') { - await ctx.api.request({ url: 'environmentVariables:create', method: 'post', data: values }); - } else { - await ctx.api.request({ - url: `environmentVariables:update`, - method: 'post', - params: { filterByTk: initialValues?.name }, - data: values, + try { + if (mode === 'create') { + await ctx.api.request({ url: 'environmentVariables:create', method: 'post', data: values }); + } else { + await ctx.api.request({ + url: `environmentVariables:update`, + method: 'post', + params: { filterByTk: initialValues?.name }, + data: values, + }); + } + } catch (error) { + notification.error({ + message: getFirstErrorMessage(ctx.api.toErrMessages(error)) || t('Operation failed'), }); + return; } onSubmitted(); await view.close(); } finally { setSubmitting(false); } - }, [ctx.api, form, initialValues?.name, mode, onSubmitted, view]); + }, [ctx.api, form, initialValues?.name, mode, notification, onSubmitted, t, view]); return (
@@ -410,11 +432,12 @@ function VariableForm(props: { ); } -function BulkImportForm(props: { onSubmitted: () => void }) { +export function BulkImportForm(props: { onSubmitted: () => void }) { const { onSubmitted } = props; const t = useT(); const ctx = useFlowContext(); const view = useFlowView(); + const { notification } = App.useApp(); const [form] = Form.useForm<{ variables?: string; secret?: string }>(); const [submitting, setSubmitting] = useState(false); @@ -430,13 +453,20 @@ function BulkImportForm(props: { onSubmitted: () => void }) { } setSubmitting(true); try { - await ctx.api.request({ url: 'environmentVariables:create', method: 'post', data: items }); + try { + await ctx.api.request({ url: 'environmentVariables:create', method: 'post', data: items }); + } catch (error) { + notification.error({ + message: getFirstErrorMessage(ctx.api.toErrMessages(error)) || t('Operation failed'), + }); + return; + } onSubmitted(); await view.close(); } finally { setSubmitting(false); } - }, [ctx.api, form, onSubmitted, view]); + }, [ctx.api, form, notification, onSubmitted, t, view]); return (
diff --git a/packages/plugins/@nocobase/plugin-environment-variables/src/server/__tests__/plugin.test.ts b/packages/plugins/@nocobase/plugin-environment-variables/src/server/__tests__/plugin.test.ts new file mode 100644 index 00000000000..16e0403f17a --- /dev/null +++ b/packages/plugins/@nocobase/plugin-environment-variables/src/server/__tests__/plugin.test.ts @@ -0,0 +1,51 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import type { Filter } from '@nocobase/database'; +import { describe, expect, it } from 'vitest'; +import { restrictValueFilterToPlainText } from '../plugin'; + +describe('restrictValueFilterToPlainText', () => { + it.each(['$includes', '$notIncludes', '$eq', '$ne'] as const)( + 'restricts the Value %s operator to plain text variables', + (operator) => { + const filter = { + value: { [operator]: 'test' }, + } as Filter; + + expect(restrictValueFilterToPlainText(filter)).toEqual({ + $and: [filter, { type: { $eq: 'default' } }], + }); + }, + ); + + it('restricts nested Value filters to plain text variables', () => { + const filter: Filter = { + $or: [ + { name: { $includes: 'API' } }, + { + $and: [{ type: { $eq: 'secret' } }, { value: { $eq: 'test' } }], + }, + ], + }; + + expect(restrictValueFilterToPlainText(filter)).toEqual({ + $and: [filter, { type: { $eq: 'default' } }], + }); + }); + + it.each([ + undefined, + { name: { $includes: 'API' } } as Filter, + { type: { $eq: 'secret' } } as Filter, + { $or: [{ name: { $eq: 'API_SECRET' } }, { type: { $eq: 'secret' } }] } as Filter, + ])('keeps filters without Value conditions unchanged', (filter) => { + expect(restrictValueFilterToPlainText(filter)).toBe(filter); + }); +}); diff --git a/packages/plugins/@nocobase/plugin-environment-variables/src/server/plugin.ts b/packages/plugins/@nocobase/plugin-environment-variables/src/server/plugin.ts index 2ba414ebc89..d907e942e11 100644 --- a/packages/plugins/@nocobase/plugin-environment-variables/src/server/plugin.ts +++ b/packages/plugins/@nocobase/plugin-environment-variables/src/server/plugin.ts @@ -7,8 +7,37 @@ * For more information, please refer to: https://www.nocobase.com/agreement. */ +import type { Filter } from '@nocobase/database'; import { Plugin } from '@nocobase/server'; +const FILTER_LOGIC_OPERATORS = ['$and', '$or'] as const; + +function containsValueFilter(filter: unknown): boolean { + if (!filter || typeof filter !== 'object' || Array.isArray(filter)) { + return false; + } + + const condition = filter as Record; + if (Object.prototype.hasOwnProperty.call(condition, 'value')) { + return true; + } + + return FILTER_LOGIC_OPERATORS.some((operator) => { + const items = condition[operator]; + return Array.isArray(items) && items.some(containsValueFilter); + }); +} + +export function restrictValueFilterToPlainText(filter?: Filter): Filter | undefined { + if (!filter || !containsValueFilter(filter)) { + return filter; + } + + return { + $and: [filter, { type: { $eq: 'default' } }], + }; +} + export class PluginEnvironmentVariablesServer extends Plugin { updated = false; @@ -158,9 +187,10 @@ export class PluginEnvironmentVariablesServer extends Plugin { }); this.app.resourceManager.registerActionHandler('environmentVariables:list', async (ctx, next) => { const repository = this.db.getRepository('environmentVariables'); + const filter = restrictValueFilterToPlainText(ctx.action.params.filter as Filter | undefined); const items = await repository.find({ sort: 'name', - filter: ctx.action.params.filter, + filter, }); for (const model of items) { if (model.type === 'secret') {