feat(financial-entities): harden list-input validation for emails and phrases #3852
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| } | ||
|
Comment on lines
+30
to
+51
Contributor
There was a problem hiding this comment. The substring overlap logic and error messages are reversed. If the existing phrase is 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 broader than "${item}" — remove the more specific phrase first`;
}
if (candidate.includes(existing)) {
return `"${value.trim()}" is already covered by "${item}"`;
}
}
return null;
}
Collaborator
Author
There was a problem hiding this comment. Same reasoning as the Generated by Claude Code |
||
|
|
||
| /** 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; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the test expectations to match the corrected phrase conflict validation logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tied to the
phraseConflictError/cleanPhrasesthreads above: the behavior is intentionally keep-the-more-specific-phrase (the shorter substring is the weaker phrase under the whole-word matcher), so these expectations are correct as written. Leaving as-is.Generated by Claude Code