From 00f1b8b0670234e69586eab80ab32f51bf9e55bd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 09:40:04 +0000 Subject: [PATCH 1/2] feat(financial-entities): harden list-input validation for emails and phrases Guard against meaningless/error-prone entries in the business list-inputs: - Emails (general contacts, billing emails, recognition emails, internal email links): reject duplicate addresses (case-insensitive). - Auto-match phrases: reject a phrase that duplicates or is a substring of another (e.g. "GOOGL" vs "GOOGLE"), which would silently shadow the more specific phrase during matching. Client add-handlers now block conflicting entries with an inline message, and the relevant zod schemas gained uniqueness refinements. The server normalizes suggestion-data list fields on every write (dedupe + drop substring-covered phrases), which self-heals legacy data and keeps automated merges from storing redundant entries; client billing emails are deduped in the clients resolver. Shared helpers added on both sides (list-input-validation) with unit tests. Closes #3848 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QHPnaJtxYAwhWZAUrz1hrD --- .../business/configurations-section.tsx | 22 ++++ .../business/contact-info-section.tsx | 26 ++++- .../clients/modify-client-dialog.tsx | 19 +++- .../common/modals/insert-business.tsx | 38 ++++++- .../src/helpers/list-input-validation.test.ts | 52 +++++++++ .../src/helpers/list-input-validation.ts | 69 ++++++++++++ ...ness-suggestion-data-schema.helper.test.ts | 38 ++++++- .../list-input-validation.helper.test.ts | 62 +++++++++++ .../business-suggestion-data-schema.helper.ts | 27 +++++ .../helpers/businesses.helper.ts | 11 +- .../helpers/list-input-validation.helper.ts | 101 ++++++++++++++++++ .../resolvers/businesses.resolver.ts | 5 +- .../resolvers/clients.resolvers.ts | 5 +- 13 files changed, 458 insertions(+), 17 deletions(-) create mode 100644 packages/client/src/helpers/list-input-validation.test.ts create mode 100644 packages/client/src/helpers/list-input-validation.ts create mode 100644 packages/server/src/modules/financial-entities/helpers/__tests__/list-input-validation.helper.test.ts create mode 100644 packages/server/src/modules/financial-entities/helpers/list-input-validation.helper.ts diff --git a/packages/client/src/components/business/configurations-section.tsx b/packages/client/src/components/business/configurations-section.tsx index c77b083da3..38b13939fe 100644 --- a/packages/client/src/components/business/configurations-section.tsx +++ b/packages/client/src/components/business/configurations-section.tsx @@ -44,6 +44,10 @@ import { relevantDataPicker, type MakeBoolean, } from '@/helpers/index.js'; +import { + duplicateEntryError, + phraseConflictError, +} from '@/helpers/list-input-validation.js'; import { useGetSortCodes } from '@/hooks/use-get-sort-codes.js'; import { useGetTags } from '@/hooks/use-get-tags.js'; import { useGetTaxCategories } from '@/hooks/use-get-tax-categories.js'; @@ -641,6 +645,12 @@ function AutoMatchingConfigurationSubSection({ form }: SubSectionProps) { const addPhrase = () => { if (newPhrase.trim()) { const currentPhrases = form.getValues('phrases'); + const conflict = phraseConflictError(currentPhrases, newPhrase); + if (conflict) { + form.setError('phrases', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('phrases'); form.setValue('phrases', [...currentPhrases, newPhrase.trim()], { shouldDirty: true }); setNewPhrase(''); } @@ -649,6 +659,12 @@ function AutoMatchingConfigurationSubSection({ form }: SubSectionProps) { const addEmail = () => { if (newEmail.trim()) { const currentEmails = form.getValues('emails'); + const conflict = duplicateEntryError(currentEmails, newEmail); + if (conflict) { + form.setError('emails', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('emails'); form.setValue('emails', [...currentEmails, newEmail.trim()], { shouldDirty: true }); setNewEmail(''); } @@ -775,6 +791,12 @@ function GmailConfigurationSubSection({ form }: SubSectionProps) { const addLink = () => { if (newLink.trim()) { const currentLinks = form.getValues('internalLinks'); + const conflict = duplicateEntryError(currentLinks, newLink); + if (conflict) { + form.setError('internalLinks', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('internalLinks'); form.setValue('internalLinks', [...currentLinks, newLink.trim()], { shouldDirty: true }); setNewLink(''); } diff --git a/packages/client/src/components/business/contact-info-section.tsx b/packages/client/src/components/business/contact-info-section.tsx index d2db78e118..63066e60d8 100644 --- a/packages/client/src/components/business/contact-info-section.tsx +++ b/packages/client/src/components/business/contact-info-section.tsx @@ -30,6 +30,10 @@ import { } from '@/gql/graphql.js'; import { getFragmentData, type FragmentType } from '@/gql/index.js'; import { dirtyFieldMarker, relevantDataPicker, type MakeBoolean } from '@/helpers/index.js'; +import { + duplicateEntryError, + hasNoDuplicateEntries, +} from '@/helpers/list-input-validation.js'; import { useAllCountries } from '@/hooks/use-get-countries.js'; import { useUpdateBusiness } from '@/hooks/use-update-business.js'; import { useUpdateClient } from '@/hooks/use-update-client.js'; @@ -75,8 +79,14 @@ const contactInfoSchema = z.object({ localAddress: z.string().optional(), phone: z.string().optional(), website: z.url('Invalid URL').optional().or(z.literal('')), - generalContacts: z.array(z.email()).optional(), - billingEmails: z.array(z.email()).optional(), + generalContacts: z + .array(z.email()) + .refine(hasNoDuplicateEntries, { message: 'Contacts must be unique' }) + .optional(), + billingEmails: z + .array(z.email()) + .refine(hasNoDuplicateEntries, { message: 'Billing emails must be unique' }) + .optional(), }); type ContactInfoFormValues = z.infer; @@ -184,6 +194,12 @@ export function ContactInfoSection({ data, refetchBusiness }: Props) { const addGeneralContact = (currentContacts: string[]) => { if (newContact.trim()) { + const conflict = duplicateEntryError(currentContacts, newContact); + if (conflict) { + form.setError('generalContacts', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('generalContacts'); form.setValue('generalContacts', [...currentContacts, newContact.trim()], { shouldDirty: true, }); @@ -201,6 +217,12 @@ export function ContactInfoSection({ data, refetchBusiness }: Props) { const addBillingEmail = (currentEmails: string[]) => { if (newBillingEmail.trim()) { + const conflict = duplicateEntryError(currentEmails, newBillingEmail); + if (conflict) { + form.setError('billingEmails', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('billingEmails'); form.setValue('billingEmails', [...currentEmails, newBillingEmail.trim()], { shouldDirty: true, }); diff --git a/packages/client/src/components/clients/modify-client-dialog.tsx b/packages/client/src/components/clients/modify-client-dialog.tsx index 392c64ec78..b83fb49f85 100644 --- a/packages/client/src/components/clients/modify-client-dialog.tsx +++ b/packages/client/src/components/clients/modify-client-dialog.tsx @@ -31,12 +31,19 @@ import { } from '@/components/ui/select.js'; import { DocumentType } from '@/gql/graphql.js'; import { getDocumentNameFromType } from '@/helpers/index.js'; +import { + duplicateEntryError, + hasNoDuplicateEntries, +} from '@/helpers/list-input-validation.js'; import { useInsertClient } from '@/hooks/use-insert-client.js'; import { Badge } from '../ui/badge'; const clientFormSchema = z.object({ businessId: z.uuid().optional(), - emails: z.array(z.email()).optional(), + emails: z + .array(z.email()) + .refine(hasNoDuplicateEntries, { message: 'Emails must be unique' }) + .optional(), generatedDocumentType: z.enum(Object.values(DocumentType)), }); @@ -79,8 +86,14 @@ export function ModifyClientDialog({ businessId, onDone, showTrigger = true }: P const addEmail = () => { if (newEmail.trim()) { - const currentEmails = form.getValues('emails'); - form.setValue('emails', [...(currentEmails ?? []), newEmail.trim()], { shouldDirty: true }); + const currentEmails = form.getValues('emails') ?? []; + const conflict = duplicateEntryError(currentEmails, newEmail); + if (conflict) { + form.setError('emails', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('emails'); + form.setValue('emails', [...currentEmails, newEmail.trim()], { shouldDirty: true }); setNewEmail(''); } }; diff --git a/packages/client/src/components/common/modals/insert-business.tsx b/packages/client/src/components/common/modals/insert-business.tsx index 7e4dcd7797..5196e604ba 100644 --- a/packages/client/src/components/common/modals/insert-business.tsx +++ b/packages/client/src/components/common/modals/insert-business.tsx @@ -38,6 +38,12 @@ import { Switch } from '@/components/ui/switch.js'; import { Textarea } from '@/components/ui/textarea.js'; import { DocumentType } from '@/gql/graphql.js'; import { pcn874RecordEnum } from '@/helpers/index.js'; +import { + duplicateEntryError, + hasNoDuplicateEntries, + hasNoPhraseOverlap, + phraseConflictError, +} from '@/helpers/list-input-validation.js'; import { useAllCountries } from '@/hooks/use-get-countries.js'; import { useGetSortCodes } from '@/hooks/use-get-sort-codes.js'; import { useGetTags } from '@/hooks/use-get-tags.js'; @@ -57,7 +63,9 @@ const businessFormSchema = z address: z.string().optional(), city: z.string().optional(), zipCode: z.string().optional(), - generalContacts: z.array(z.email()), + generalContacts: z + .array(z.email()) + .refine(hasNoDuplicateEntries, { message: 'Contacts must be unique' }), website: z.url().optional().or(z.literal('')), phone: z.string().optional(), taxCategory: z.string().optional(), @@ -67,8 +75,14 @@ const businessFormSchema = z defaultDescription: z.string().optional(), defaultTags: z.array(z.string()), isClient: z.boolean().default(false).optional(), - transactionPhrases: z.array(z.string()), - emailAddresses: z.array(z.email()), + transactionPhrases: z + .array(z.string()) + .refine(hasNoPhraseOverlap, { + message: 'Phrases must be unique and not overlap one another', + }), + emailAddresses: z + .array(z.email()) + .refine(hasNoDuplicateEntries, { message: 'Emails must be unique' }), }) .refine( data => { @@ -230,6 +244,12 @@ function ContactInformationSection({ form }: SectionProps) { const addGeneralContact = (currentContacts: string[]) => { if (newContact.trim()) { + const conflict = duplicateEntryError(currentContacts, newContact); + if (conflict) { + form.setError('generalContacts', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('generalContacts'); setValue('generalContacts', [...currentContacts, newContact.trim()], { shouldDirty: true, }); @@ -647,6 +667,12 @@ function AutoMatchingSection({ form }: SectionProps) { const addPhrase = () => { if (newPhrase.trim()) { const currentPhrases = getValues('transactionPhrases'); + const conflict = phraseConflictError(currentPhrases, newPhrase); + if (conflict) { + form.setError('transactionPhrases', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('transactionPhrases'); form.setValue('transactionPhrases', [...currentPhrases, newPhrase.trim()], { shouldDirty: true, }); @@ -657,6 +683,12 @@ function AutoMatchingSection({ form }: SectionProps) { const addEmail = () => { if (newEmail.trim()) { const currentEmails = getValues('emailAddresses'); + const conflict = duplicateEntryError(currentEmails, newEmail); + if (conflict) { + form.setError('emailAddresses', { type: 'manual', message: conflict }); + return; + } + form.clearErrors('emailAddresses'); setValue('emailAddresses', [...currentEmails, newEmail.trim()], { shouldDirty: true }); setNewEmail(''); } diff --git a/packages/client/src/helpers/list-input-validation.test.ts b/packages/client/src/helpers/list-input-validation.test.ts new file mode 100644 index 0000000000..0b338ab98f --- /dev/null +++ b/packages/client/src/helpers/list-input-validation.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; +import { + duplicateEntryError, + hasNoDuplicateEntries, + hasNoPhraseOverlap, + phraseConflictError, +} from './list-input-validation.js'; + +describe('duplicateEntryError', () => { + it('returns null when the value is new', () => { + expect(duplicateEntryError(['a@x.com'], 'b@x.com')).toBeNull(); + }); + + it('detects case- and whitespace-insensitive duplicates', () => { + expect(duplicateEntryError(['a@x.com'], ' A@X.com ')).toContain('already in the list'); + }); +}); + +describe('phraseConflictError', () => { + it('returns null for a distinct phrase', () => { + expect(phraseConflictError(['GOOGLE'], 'AMAZON')).toBeNull(); + }); + + it('flags an exact duplicate', () => { + expect(phraseConflictError(['GOOGLE'], 'google')).toContain('already in the list'); + }); + + it('flags a new phrase already covered by an existing broader one', () => { + expect(phraseConflictError(['GOOGL'], 'GOOGLE')).toContain('overlaps'); + }); + + it('flags a new phrase that would shadow an existing more specific one', () => { + expect(phraseConflictError(['GOOGLE'], 'GOOGL')).toContain('already covered'); + }); +}); + +describe('hasNoDuplicateEntries', () => { + it('is true for unique entries and false for case-insensitive duplicates', () => { + expect(hasNoDuplicateEntries(['a@x.com', 'b@x.com'])).toBe(true); + expect(hasNoDuplicateEntries(['a@x.com', 'A@X.com'])).toBe(false); + }); +}); + +describe('hasNoPhraseOverlap', () => { + it('is true for non-overlapping phrases', () => { + expect(hasNoPhraseOverlap(['GOOGLE', 'AMAZON'])).toBe(true); + }); + + it('is false when one phrase is a substring of another', () => { + expect(hasNoPhraseOverlap(['GOOGL', 'GOOGLE'])).toBe(false); + }); +}); diff --git a/packages/client/src/helpers/list-input-validation.ts b/packages/client/src/helpers/list-input-validation.ts new file mode 100644 index 0000000000..1be8423748 --- /dev/null +++ b/packages/client/src/helpers/list-input-validation.ts @@ -0,0 +1,69 @@ +/** + * Client-side guards for list-of-string form inputs (emails, auto-match + * phrases, …). Mirrors the server-side rules in + * `financial-entities/helpers/list-input-validation.helper.ts` so the user gets + * immediate feedback before the mutation is sent. + * + * All comparisons are done on a normalized (trimmed, lower-cased) form. + */ + +export function normalizeEntry(value: string): string { + return value.trim().toLowerCase(); +} + +/** + * Message to show when `value` would duplicate an existing entry + * (case-insensitive), or `null` when it is safe to add. + */ +export function duplicateEntryError(list: readonly string[], value: string): string | null { + const candidate = normalizeEntry(value); + return list.some(item => normalizeEntry(item) === candidate) + ? `"${value.trim()}" is already in the list` + : null; +} + +/** + * Message to show when `value` conflicts with an existing phrase — either a + * duplicate or a substring overlap (e.g. "GOOGL" vs "GOOGLE") — or `null` when + * it is safe to add. + */ +export function phraseConflictError(list: readonly string[], value: string): string | null { + const candidate = normalizeEntry(value); + if (candidate === '') { + return null; + } + for (const item of list) { + const existing = normalizeEntry(item); + if (existing === '') { + continue; + } + if (existing === candidate) { + return `"${value.trim()}" is already in the list`; + } + if (existing.includes(candidate)) { + return `"${value.trim()}" is already covered by "${item}"`; + } + if (candidate.includes(existing)) { + return `"${value.trim()}" overlaps with "${item}" — remove the broader phrase first`; + } + } + return null; +} + +/** True when every entry in the list is unique (case-insensitive). */ +export function hasNoDuplicateEntries(list: readonly string[]): boolean { + return new Set(list.map(normalizeEntry)).size === list.length; +} + +/** True when no phrase is a duplicate or substring of another. */ +export function hasNoPhraseOverlap(list: readonly string[]): boolean { + const normalized = list.map(normalizeEntry).filter(entry => entry !== ''); + for (let i = 0; i < normalized.length; i++) { + for (let j = i + 1; j < normalized.length; j++) { + if (normalized[i].includes(normalized[j]) || normalized[j].includes(normalized[i])) { + return false; + } + } + } + return true; +} diff --git a/packages/server/src/modules/financial-entities/helpers/__tests__/business-suggestion-data-schema.helper.test.ts b/packages/server/src/modules/financial-entities/helpers/__tests__/business-suggestion-data-schema.helper.test.ts index efbf25c66c..035ec2f87b 100644 --- a/packages/server/src/modules/financial-entities/helpers/__tests__/business-suggestion-data-schema.helper.test.ts +++ b/packages/server/src/modules/financial-entities/helpers/__tests__/business-suggestion-data-schema.helper.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest'; -import { suggestionDataSchema } from '../business-suggestion-data-schema.helper.js'; +import { + normalizeSuggestionListData, + suggestionDataSchema, +} from '../business-suggestion-data-schema.helper.js'; describe('suggestionDataSchema emails', () => { it('accepts concrete email addresses', () => { @@ -36,3 +39,36 @@ describe('suggestionDataSchema emails', () => { expect(result.success).toBe(false); }); }); + +describe('normalizeSuggestionListData', () => { + it('drops duplicate recognition emails (case-insensitive)', () => { + expect(normalizeSuggestionListData({ emails: ['vendor@acme.com', 'Vendor@Acme.com'] })).toEqual({ + emails: ['vendor@acme.com'], + }); + }); + + it('drops duplicate phrases (case-insensitive)', () => { + expect(normalizeSuggestionListData({ phrases: ['GOOGLE', 'google'] })).toEqual({ + phrases: ['GOOGLE'], + }); + }); + + it('drops a phrase that is a substring of another, keeping the more specific one', () => { + expect(normalizeSuggestionListData({ phrases: ['GOOGL', 'GOOGLE'] })).toEqual({ + phrases: ['GOOGLE'], + }); + }); + + it('drops duplicate internal email links', () => { + expect( + normalizeSuggestionListData({ + emailListener: { internalEmailLinks: ['https://x.com/a', 'https://x.com/a'] }, + }), + ).toEqual({ emailListener: { internalEmailLinks: ['https://x.com/a'] } }); + }); + + it('leaves distinct, non-overlapping phrases and emails untouched', () => { + const input = { phrases: ['GOOGLE', 'AMAZON'], emails: ['a@x.com', 'b@x.com'] }; + expect(normalizeSuggestionListData(input)).toEqual(input); + }); +}); diff --git a/packages/server/src/modules/financial-entities/helpers/__tests__/list-input-validation.helper.test.ts b/packages/server/src/modules/financial-entities/helpers/__tests__/list-input-validation.helper.test.ts new file mode 100644 index 0000000000..d608b0b668 --- /dev/null +++ b/packages/server/src/modules/financial-entities/helpers/__tests__/list-input-validation.helper.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import { + cleanPhrases, + dedupeList, + findDuplicateEntry, + findPhraseConflict, +} from '../list-input-validation.helper.js'; + +describe('findDuplicateEntry', () => { + it('returns null when all entries are unique', () => { + expect(findDuplicateEntry(['a@x.com', 'b@x.com'])).toBeNull(); + }); + + it('detects case-insensitive, whitespace-insensitive duplicates', () => { + expect(findDuplicateEntry(['a@x.com', ' A@X.com '])).toBe(' A@X.com '); + }); +}); + +describe('dedupeList', () => { + it('removes case-insensitive duplicates keeping the first occurrence', () => { + expect(dedupeList(['a@x.com', 'A@X.com', 'b@x.com'])).toEqual(['a@x.com', 'b@x.com']); + }); +}); + +describe('findPhraseConflict', () => { + it('returns null for distinct, non-overlapping phrases', () => { + expect(findPhraseConflict(['GOOGLE', 'AMAZON'])).toBeNull(); + }); + + it('flags exact duplicates (case-insensitive)', () => { + expect(findPhraseConflict(['GOOGLE', 'google'])).toEqual({ + type: 'duplicate', + value: 'google', + }); + }); + + it('flags a phrase that is a substring of another', () => { + expect(findPhraseConflict(['GOOGL', 'GOOGLE'])).toEqual({ + type: 'substring', + shorter: 'GOOGL', + longer: 'GOOGLE', + }); + }); + + it('detects the overlap regardless of order', () => { + expect(findPhraseConflict(['GOOGLE', 'GOOGL'])).toEqual({ + type: 'substring', + shorter: 'GOOGL', + longer: 'GOOGLE', + }); + }); +}); + +describe('cleanPhrases', () => { + it('drops the broader phrase when one contains another', () => { + expect(cleanPhrases(['GOOGL', 'GOOGLE', 'AMAZON'])).toEqual(['GOOGLE', 'AMAZON']); + }); + + it('removes case-insensitive duplicates', () => { + expect(cleanPhrases(['GOOGLE', 'google', 'AMAZON'])).toEqual(['GOOGLE', 'AMAZON']); + }); +}); diff --git a/packages/server/src/modules/financial-entities/helpers/business-suggestion-data-schema.helper.ts b/packages/server/src/modules/financial-entities/helpers/business-suggestion-data-schema.helper.ts index b7c768c39e..b9c81ebd0f 100644 --- a/packages/server/src/modules/financial-entities/helpers/business-suggestion-data-schema.helper.ts +++ b/packages/server/src/modules/financial-entities/helpers/business-suggestion-data-schema.helper.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import { isValidWildcardEmailPattern } from './email-pattern.helper.js'; +import { cleanPhrases, dedupeList } from './list-input-validation.helper.js'; // A recognition email is either a concrete address or a wildcard pattern // (e.g. `*@cloudflare.com`) for suppliers that send from a unique address per @@ -33,3 +34,29 @@ export const suggestionDataSchema = z .strict(); export type SuggestionData = z.infer; + +/** + * Defensively clean the list fields of a suggestion-data object before it is + * persisted: drop duplicate recognition emails / internal email links + * (case-insensitive) and duplicate or substring-overlapping phrases (keeping the + * more specific phrase). The UI blocks these conflicts up front with a clear + * message; this is the write-path safety net that also self-heals legacy data + * and keeps automated merges from storing redundant entries. Shape validation is + * still performed by the caller via `suggestionDataSchema`. + */ +export function normalizeSuggestionListData(data: SuggestionData): SuggestionData { + const normalized: SuggestionData = { ...data }; + if (normalized.phrases) { + normalized.phrases = cleanPhrases(normalized.phrases); + } + if (normalized.emails) { + normalized.emails = dedupeList(normalized.emails); + } + if (normalized.emailListener?.internalEmailLinks) { + normalized.emailListener = { + ...normalized.emailListener, + internalEmailLinks: dedupeList(normalized.emailListener.internalEmailLinks), + }; + } + return normalized; +} diff --git a/packages/server/src/modules/financial-entities/helpers/businesses.helper.ts b/packages/server/src/modules/financial-entities/helpers/businesses.helper.ts index 1b63d8de47..eae43ea60d 100644 --- a/packages/server/src/modules/financial-entities/helpers/businesses.helper.ts +++ b/packages/server/src/modules/financial-entities/helpers/businesses.helper.ts @@ -2,7 +2,10 @@ import type { SuggestionsEmailListenerConfigInput, UpdateBusinessInput, } from '../../../__generated__/types.js'; -import { suggestionDataSchema } from '../helpers/business-suggestion-data-schema.helper.js'; +import { + normalizeSuggestionListData, + suggestionDataSchema, +} from '../helpers/business-suggestion-data-schema.helper.js'; import type { Json, SuggestionData } from '../types.js'; function mergeEmailListenerConfig( @@ -55,11 +58,11 @@ export function updateSuggestions( const currentPhrases = currentSuggestionData.phrases ?? []; const newPhrases = newSuggestions.phrases ?? currentPhrases; - const phrases = merge ? Array.from(new Set([...currentPhrases, ...newPhrases])) : newPhrases; + const phrases = merge ? [...currentPhrases, ...newPhrases] : newPhrases; const currentEmails = currentSuggestionData.emails ?? []; const newEmails = newSuggestions.emails ?? currentEmails; - const emails = merge ? Array.from(new Set([...currentEmails, ...newEmails])) : newEmails; + const emails = merge ? [...currentEmails, ...newEmails] : newEmails; const currentEmailListener = currentSuggestionData.emailListener ?? undefined; const newEmailListener = newSuggestions.emailListener ?? undefined; @@ -89,5 +92,5 @@ export function updateSuggestions( throw new Error('Invalid business suggestions format'); } - return updatedSuggestions; + return normalizeSuggestionListData(updatedSuggestions); } diff --git a/packages/server/src/modules/financial-entities/helpers/list-input-validation.helper.ts b/packages/server/src/modules/financial-entities/helpers/list-input-validation.helper.ts new file mode 100644 index 0000000000..238ab6b7e1 --- /dev/null +++ b/packages/server/src/modules/financial-entities/helpers/list-input-validation.helper.ts @@ -0,0 +1,101 @@ +/** + * Shared validation/normalization utilities for list-of-string inputs + * (recognition emails, internal email links, auto-match phrases, …). + * + * Two problems these guard against: + * - duplicate entries (case-insensitive), which are meaningless and bloat the + * stored data; and + * - phrases that are a substring of another phrase (e.g. "GOOGL" vs "GOOGLE"), + * where the broader entry silently shadows the more specific one during + * auto-matching. + * + * Comparison is always done on a normalized (trimmed, lower-cased) form so that + * "Vendor@Acme.com" and "vendor@acme.com " are treated as the same entry. + */ + +/** Normalize an entry for case-insensitive comparison. */ +export function normalizeListEntry(value: string): string { + return value.trim().toLowerCase(); +} + +/** + * Return the first entry whose normalized form has already appeared earlier in + * the list, or `null` when every entry is unique. + */ +export function findDuplicateEntry(list: readonly string[]): string | null { + const seen = new Set(); + for (const item of list) { + const key = normalizeListEntry(item); + if (seen.has(key)) { + return item; + } + seen.add(key); + } + return null; +} + +/** Remove duplicate entries (case-insensitive), keeping the first occurrence. */ +export function dedupeList(list: readonly string[]): string[] { + const seen = new Set(); + const result: string[] = []; + for (const item of list) { + const key = normalizeListEntry(item); + if (!seen.has(key)) { + seen.add(key); + result.push(item); + } + } + return result; +} + +export type PhraseConflict = + | { type: 'duplicate'; value: string } + | { type: 'substring'; shorter: string; longer: string }; + +/** + * Detect the first conflict in a phrase list: either an exact (case-insensitive) + * duplicate, or a phrase whose normalized form is contained within another + * phrase. Empty/whitespace-only entries are ignored for the substring check. + */ +export function findPhraseConflict(list: readonly string[]): PhraseConflict | null { + const normalized = list.map(normalizeListEntry); + for (let i = 0; i < normalized.length; i++) { + for (let j = i + 1; j < normalized.length; j++) { + const a = normalized[i]; + const b = normalized[j]; + if (a === b) { + return { type: 'duplicate', value: list[j] }; + } + if (a === '' || b === '') { + continue; + } + if (b.includes(a)) { + return { type: 'substring', shorter: list[i], longer: list[j] }; + } + if (a.includes(b)) { + return { type: 'substring', shorter: list[j], longer: list[i] }; + } + } + } + return null; +} + +/** + * Normalize a phrase list by removing duplicates and any phrase whose normalized + * form is a substring of another kept phrase (the broader, less specific entry + * is dropped). Used on automated merge paths where rejecting the whole operation + * would be wrong. + */ +export function cleanPhrases(list: readonly string[]): string[] { + const deduped = dedupeList(list); + const normalized = deduped.map(normalizeListEntry); + return deduped.filter((_, i) => { + const current = normalized[i]; + if (current === '') { + return true; + } + return !normalized.some( + (other, j) => j !== i && other !== current && other.includes(current), + ); + }); +} diff --git a/packages/server/src/modules/financial-entities/resolvers/businesses.resolver.ts b/packages/server/src/modules/financial-entities/resolvers/businesses.resolver.ts index 06206c343a..fd28ece047 100644 --- a/packages/server/src/modules/financial-entities/resolvers/businesses.resolver.ts +++ b/packages/server/src/modules/financial-entities/resolvers/businesses.resolver.ts @@ -7,6 +7,7 @@ import { SortCodesProvider } from '../../sort-codes/providers/sort-codes.provide import { TagsProvider } from '../../tags/providers/tags.provider.js'; import { TransactionsProvider } from '../../transactions/providers/transactions.provider.js'; import { + normalizeSuggestionListData, SuggestionData, suggestionDataSchema, } from '../helpers/business-suggestion-data-schema.helper.js'; @@ -123,7 +124,7 @@ export const businessesResolvers: FinancialEntitiesModule.Resolvers & } const suggestions: SuggestionData | undefined = fields.suggestions - ? { + ? normalizeSuggestionListData({ tags: fields.suggestions.tags?.map(tag => tag.id), phrases: fields.suggestions.phrases?.map(phrase => phrase), description: fields.suggestions.description ?? undefined, @@ -141,7 +142,7 @@ export const businessesResolvers: FinancialEntitiesModule.Resolvers & } : undefined, priority: fields.suggestions.priority ?? undefined, - } + }) : undefined; const business = await injector.get(BusinessesProvider).insertBusinessLoader.load({ diff --git a/packages/server/src/modules/financial-entities/resolvers/clients.resolvers.ts b/packages/server/src/modules/financial-entities/resolvers/clients.resolvers.ts index 565e8c3cf9..1fbbe483cb 100644 --- a/packages/server/src/modules/financial-entities/resolvers/clients.resolvers.ts +++ b/packages/server/src/modules/financial-entities/resolvers/clients.resolvers.ts @@ -5,6 +5,7 @@ import { updateGreenInvoiceClient, } from '../../green-invoice/helpers/green-invoice-clients.helper.js'; import { validateClientIntegrations } from '../helpers/clients.helper.js'; +import { dedupeList } from '../helpers/list-input-validation.helper.js'; import { BusinessesProvider } from '../providers/businesses.provider.js'; import { ClientsProvider } from '../providers/clients.provider.js'; import type { @@ -62,7 +63,7 @@ export const clientsResolvers: FinancialEntitiesModule.Resolvers & } const adjustedFields: IUpdateClientParams = { businessId, - emails: fields.emails ? [...fields.emails] : undefined, + emails: fields.emails ? dedupeList(fields.emails) : undefined, newBusinessId: fields.newBusinessId, integrations: updatedIntegrations, }; @@ -94,7 +95,7 @@ export const clientsResolvers: FinancialEntitiesModule.Resolvers & try { const newClient: IInsertClientParams = { businessId: fields.businessId, - emails: fields.emails ? [...fields.emails] : [], + emails: fields.emails ? dedupeList(fields.emails) : [], integrations: fields.integrations ?? {}, }; const [insertClient] = await injector.get(ClientsProvider).insertClient(newClient); From 9aaa3d563c6906e548d0f0b362f2c8b24a4762b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Jul 2026 05:36:08 +0000 Subject: [PATCH 2/2] fix(client): guard list add-handlers against undefined form values Default form.getValues(...) to [] before duplicate/phrase-conflict checks so the list helpers never receive undefined. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QHPnaJtxYAwhWZAUrz1hrD --- .../src/components/business/configurations-section.tsx | 6 +++--- .../client/src/components/common/modals/insert-business.tsx | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/client/src/components/business/configurations-section.tsx b/packages/client/src/components/business/configurations-section.tsx index 38b13939fe..4fa4450348 100644 --- a/packages/client/src/components/business/configurations-section.tsx +++ b/packages/client/src/components/business/configurations-section.tsx @@ -644,7 +644,7 @@ function AutoMatchingConfigurationSubSection({ form }: SubSectionProps) { const addPhrase = () => { if (newPhrase.trim()) { - const currentPhrases = form.getValues('phrases'); + const currentPhrases = form.getValues('phrases') ?? []; const conflict = phraseConflictError(currentPhrases, newPhrase); if (conflict) { form.setError('phrases', { type: 'manual', message: conflict }); @@ -658,7 +658,7 @@ function AutoMatchingConfigurationSubSection({ form }: SubSectionProps) { const addEmail = () => { if (newEmail.trim()) { - const currentEmails = form.getValues('emails'); + const currentEmails = form.getValues('emails') ?? []; const conflict = duplicateEntryError(currentEmails, newEmail); if (conflict) { form.setError('emails', { type: 'manual', message: conflict }); @@ -790,7 +790,7 @@ function GmailConfigurationSubSection({ form }: SubSectionProps) { const addLink = () => { if (newLink.trim()) { - const currentLinks = form.getValues('internalLinks'); + const currentLinks = form.getValues('internalLinks') ?? []; const conflict = duplicateEntryError(currentLinks, newLink); if (conflict) { form.setError('internalLinks', { type: 'manual', message: conflict }); diff --git a/packages/client/src/components/common/modals/insert-business.tsx b/packages/client/src/components/common/modals/insert-business.tsx index 5196e604ba..c445d741d0 100644 --- a/packages/client/src/components/common/modals/insert-business.tsx +++ b/packages/client/src/components/common/modals/insert-business.tsx @@ -666,7 +666,7 @@ function AutoMatchingSection({ form }: SectionProps) { const addPhrase = () => { if (newPhrase.trim()) { - const currentPhrases = getValues('transactionPhrases'); + const currentPhrases = getValues('transactionPhrases') ?? []; const conflict = phraseConflictError(currentPhrases, newPhrase); if (conflict) { form.setError('transactionPhrases', { type: 'manual', message: conflict }); @@ -682,7 +682,7 @@ function AutoMatchingSection({ form }: SectionProps) { const addEmail = () => { if (newEmail.trim()) { - const currentEmails = getValues('emailAddresses'); + const currentEmails = getValues('emailAddresses') ?? []; const conflict = duplicateEntryError(currentEmails, newEmail); if (conflict) { form.setError('emailAddresses', { type: 'manual', message: conflict });