diff --git a/frontend/src/pages/xray/routing/RoutingTab.tsx b/frontend/src/pages/xray/routing/RoutingTab.tsx index 95cc1d165..1b54a6213 100644 --- a/frontend/src/pages/xray/routing/RoutingTab.tsx +++ b/frontend/src/pages/xray/routing/RoutingTab.tsx @@ -21,7 +21,7 @@ import RuleFormModal from './RuleFormModal'; import type { RoutingRule } from './RuleFormModal'; import RuleCardList from './RuleCardList'; import { useRoutingColumns } from './useRoutingColumns'; -import { arrJoin, originalRuleIndex, isApiRule } from './helpers'; +import { arrJoin, originalRuleIndex } from './helpers'; import type { RuleRow } from './types'; import type { XraySettingsValue, SetTemplate } from '@/hooks/useXraySetting'; import type { RuleObject } from '@/schemas/routing'; @@ -209,20 +209,8 @@ export default function RoutingTab({ if (!tt.routing) tt.routing = { rules: [] }; if (!Array.isArray(tt.routing.rules)) tt.routing.rules = []; const typed = rule as unknown as RuleObject; - if (editingIndex == null) { - // Rules match top-to-bottom, first hit wins, so a brand-new rule - // appended at the end is silently shadowed by any earlier - // broader/catch-all rule that also happens to match its traffic — - // a real trap: the rule looks saved and enabled, but never actually - // fires. Insert it as early as possible instead (right after the - // pinned api rule, if present) so a newly created rule takes effect - // by default; the admin can still drag it lower with moveDown if - // that's genuinely what they want. - const insertAt = isApiRule(tt.routing.rules[0] as RuleObject) ? 1 : 0; - tt.routing.rules.splice(insertAt, 0, typed); - } else { - tt.routing.rules[editingIndex] = typed; - } + if (editingIndex == null) tt.routing.rules.push(typed); + else tt.routing.rules[editingIndex] = typed; }); setRuleModalOpen(false); } diff --git a/frontend/src/test/routing-new-rule-insert-position.test.tsx b/frontend/src/test/routing-new-rule-insert-position.test.tsx deleted file mode 100644 index 609dc5813..000000000 --- a/frontend/src/test/routing-new-rule-insert-position.test.tsx +++ /dev/null @@ -1,62 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; -import { fireEvent, screen } from '@testing-library/react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; - -import RoutingTab from '@/pages/xray/routing/RoutingTab'; -import type { XraySettingsValue } from '@/hooks/useXraySetting'; - -import { renderWithProviders } from './test-utils'; - -function settingsWithApiAndBlockRules(): XraySettingsValue { - return { - routing: { - rules: [ - { type: 'field', inboundTag: ['api'], outboundTag: 'api', enabled: true }, - { type: 'field', ip: ['ext:geoip_RU.dat:ru'], outboundTag: 'blocked', enabled: true }, - { type: 'field', protocol: ['bittorrent'], outboundTag: 'blocked', enabled: true }, - ], - }, - } as unknown as XraySettingsValue; -} - -// Rules match top-to-bottom, first hit wins. A brand-new rule used to be -// appended at the end, where a pre-existing broader/catch-all rule (e.g. a -// block rule with no inboundTag restriction, like the ones here) silently -// shadows it forever -- the rule looks saved and enabled but never actually -// fires. See RoutingTab.tsx onRuleConfirm. -describe('RoutingTab new-rule insert position', () => { - it('inserts a newly created rule right after the pinned api rule, not at the end', () => { - const setTemplateSettings = vi.fn(); - const initial = settingsWithApiAndBlockRules(); - - const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); - renderWithProviders( - - - , - ); - - fireEvent.click(screen.getByRole('tab', { name: /Routing Rules/ })); - fireEvent.click(screen.getByRole('button', { name: /Routing Rules/ })); - fireEvent.click(screen.getByRole('button', { name: 'Create' })); - - expect(setTemplateSettings).toHaveBeenCalledTimes(1); - const updater = setTemplateSettings.mock.calls[0][0] as (prev: XraySettingsValue) => XraySettingsValue; - const next = updater(initial); - const rules = (next.routing as { rules: Array<{ inboundTag?: string[]; outboundTag?: string }> }).rules; - - expect(rules.length).toBe(4); - expect(rules[0].outboundTag).toBe('api'); - // The two pre-existing block rules must have been pushed down, not the - // new rule appended after them. - expect(rules[1].outboundTag).not.toBe('blocked'); - expect(rules[2].outboundTag).toBe('blocked'); - expect(rules[3].outboundTag).toBe('blocked'); - }); -});