feat(settings): rename learned counterparty templates (#2454)
* feat(settings): rename learned counterparty templates A user asked why a learned template under Inställningar > Mallar can be deleted but not renamed. Nothing legal or ledger-shaped blocks a rename; the one real obstacle was that the learn path keys templates by the normalized bank description, so a renamed row would stop receiving re-approvals and a duplicate would appear under the old key. Why it occurred: counterparty_name doubles as display name and as the learn/upsert key, and the only write path for it was the learner. There was no rename because every later approval would have forked the row. What was simplified instead of added: no display-label column, no new table, no migration. The rename moves the old key into counterparty_aliases, which the matcher already checks first, and the learn lookup (findTemplateByKey) now resolves name-then-alias so re-approvals and SIE re-imports land on the renamed row. Why this over the proposed shape: a separate label would have kept the key untouched but added a second name field for users to reason about; renaming the key with an alias trail gives the user exactly what they asked for with one fewer concept. Duplicate names are refused with 409 (active twin) or the invisible soft-deleted twin is removed (inactive). Fixes #2453 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgYn5GEp4N5Dxjc1S9Ljbq * fix(bookkeeping): resolve the normalized-name match tier through aliases after a rename Skeptic refutation on 819894559: the alias tier compares raw lowercased bank descriptors, so the normalized key a rename pushes into aliases ("spotify") never matched there, and the name tier only knew the new label ("musik"). A renamed template kept learning through findTemplateByKey but was never proposed again for the merchant it was learned from. nameMap now also resolves aliases, with a real counterparty_name always winning over another row's alias. Refs #2453 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgYn5GEp4N5Dxjc1S9Ljbq * fix(bookkeeping): canonical name beats a borrowed alias; unique-name race returns 409 Review findings on #2454: - The alias tier ran before the name tier, so a bank line that is exactly another template's canonical name could resolve to a row holding that string as a rename alias. Aliases claimed by a different template's counterparty_name are now skipped when building the alias map. - The PATCH twin check and the update are separate statements; a learn or a concurrent rename between them surfaced as 500. Postgres 23505 on the update now maps to the same 409 as the pre-check. Refs #2453 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgYn5GEp4N5Dxjc1S9Ljbq --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
bb28968151
commit
e996d70955
@@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { createMockRequest, parseJsonResponse, createQueuedMockSupabase } from '@/tests/helpers'
|
||||
|
||||
const { supabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
const { supabase, enqueue, reset, findCall } = createQueuedMockSupabase()
|
||||
|
||||
const requireAuthMock = vi.fn()
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
@@ -24,7 +24,7 @@ vi.mock('@/lib/bookkeeping/counterparty-templates', () => ({
|
||||
findCounterpartyTemplate: (...args: unknown[]) => findCounterpartyTemplateMock(...args),
|
||||
}))
|
||||
|
||||
import { GET, DELETE } from '../route'
|
||||
import { GET, DELETE, PATCH } from '../route'
|
||||
|
||||
describe('GET /api/settings/counterparty-templates', () => {
|
||||
beforeEach(() => {
|
||||
@@ -158,3 +158,145 @@ describe('DELETE /api/settings/counterparty-templates', () => {
|
||||
expect(body.data.success).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('PATCH /api/settings/counterparty-templates', () => {
|
||||
const TEMPLATE_ID = '11111111-1111-4111-8111-111111111111'
|
||||
const TWIN_ID = '22222222-2222-4222-8222-222222222222'
|
||||
const existing = {
|
||||
id: TEMPLATE_ID,
|
||||
company_id: 'company-1',
|
||||
counterparty_name: 'spotify ab stockholm 4471',
|
||||
counterparty_aliases: ['spotify ab stockholm 4471 kortköp'],
|
||||
is_active: true,
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase, error: null })
|
||||
requireWriteMock.mockResolvedValue({ ok: true })
|
||||
})
|
||||
|
||||
function patch(body: unknown) {
|
||||
const request = createMockRequest('/api/settings/counterparty-templates', {
|
||||
method: 'PATCH',
|
||||
body,
|
||||
})
|
||||
return PATCH(request, { params: Promise.resolve({}) })
|
||||
}
|
||||
|
||||
it('returns 401 when not authenticated', async () => {
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: null,
|
||||
supabase,
|
||||
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
|
||||
})
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 403 for a viewer without write permission', async () => {
|
||||
requireWriteMock.mockResolvedValue({
|
||||
ok: false,
|
||||
response: NextResponse.json({ error: 'Forbidden' }, { status: 403 }),
|
||||
})
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(403)
|
||||
})
|
||||
|
||||
it('returns 400 for an empty name', async () => {
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: ' ' }))
|
||||
expect(status).toBe(400)
|
||||
expect(supabase.from).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns 400 for a name that is too long', async () => {
|
||||
const { status } = await parseJsonResponse(
|
||||
await patch({ id: TEMPLATE_ID, counterparty_name: 'a'.repeat(101) }),
|
||||
)
|
||||
expect(status).toBe(400)
|
||||
})
|
||||
|
||||
it('returns 400 for a one-character name', async () => {
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'S' }))
|
||||
expect(status).toBe(400)
|
||||
expect(supabase.from).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns 404 when the template is not in the active company', async () => {
|
||||
enqueue({ data: null }) // lookup by id + company
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(404)
|
||||
expect(findCall('categorization_templates', 'update')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns 409 when another active template already has the name', async () => {
|
||||
enqueue({ data: existing }) // lookup by id
|
||||
enqueue({ data: { id: TWIN_ID, is_active: true } }) // twin lookup
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(409)
|
||||
expect(findCall('categorization_templates', 'update')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('removes a soft-deleted twin instead of refusing the rename', async () => {
|
||||
enqueue({ data: existing }) // lookup by id
|
||||
enqueue({ data: { id: TWIN_ID, is_active: false } }) // inactive twin
|
||||
enqueue({ error: null }) // delete twin
|
||||
enqueue({ data: { ...existing, counterparty_name: 'spotify' } }) // update
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(200)
|
||||
expect(findCall('categorization_templates', 'delete')).toBeDefined()
|
||||
expect(findCall('categorization_templates', 'update')).toBeDefined()
|
||||
})
|
||||
|
||||
it('renames, lowercases the key and keeps the old name as an alias', async () => {
|
||||
enqueue({ data: existing }) // lookup by id
|
||||
enqueue({ data: null }) // no twin
|
||||
enqueue({
|
||||
data: {
|
||||
...existing,
|
||||
counterparty_name: 'spotify',
|
||||
counterparty_aliases: [...existing.counterparty_aliases, existing.counterparty_name],
|
||||
},
|
||||
}) // update
|
||||
|
||||
const { status, body } = await parseJsonResponse<{
|
||||
data: { counterparty_name: string; counterparty_aliases: string[] }
|
||||
}>(await patch({ id: TEMPLATE_ID, counterparty_name: ' Spotify ' }))
|
||||
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.counterparty_name).toBe('spotify')
|
||||
const payload = findCall('categorization_templates', 'update')?.[0] as {
|
||||
counterparty_name: string
|
||||
counterparty_aliases: string[]
|
||||
}
|
||||
expect(payload.counterparty_name).toBe('spotify')
|
||||
expect(payload.counterparty_aliases).toContain('spotify ab stockholm 4471')
|
||||
expect(payload.counterparty_aliases).toContain('spotify ab stockholm 4471 kortköp')
|
||||
expect(findCall('categorization_templates', 'delete')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns 409 when the update loses the race on the unique name', async () => {
|
||||
enqueue({ data: existing }) // lookup by id
|
||||
enqueue({ data: null }) // no twin at check time
|
||||
enqueue({ data: null, error: { code: '23505', message: 'duplicate key value' } }) // update
|
||||
|
||||
const { status } = await parseJsonResponse(await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify' }))
|
||||
expect(status).toBe(409)
|
||||
})
|
||||
|
||||
it('is a no-op when the name is unchanged', async () => {
|
||||
enqueue({ data: existing }) // lookup by id
|
||||
|
||||
const { status } = await parseJsonResponse(
|
||||
await patch({ id: TEMPLATE_ID, counterparty_name: 'Spotify AB Stockholm 4471' }),
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(findCall('categorization_templates', 'update')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,8 +1,25 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { z } from 'zod'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import { findCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templates'
|
||||
import type { Transaction } from '@/types'
|
||||
import type { CategorizationTemplate, Transaction } from '@/types'
|
||||
|
||||
const RenameCounterpartyTemplateSchema = z.object({
|
||||
id: z.string().uuid(),
|
||||
counterparty_name: z.string().trim().min(1).max(100),
|
||||
})
|
||||
|
||||
/**
|
||||
* Stored names are lowercase, single-spaced keys (the matcher compares them
|
||||
* against normalized bank descriptions). A user-typed name gets the same
|
||||
* shape; display re-capitalizes through formatCounterpartyName().
|
||||
*/
|
||||
function toTemplateKey(input: string): string {
|
||||
return input.toLowerCase().replace(/\s+/g, ' ').trim()
|
||||
}
|
||||
|
||||
export const GET = withRouteContext(
|
||||
'counterparty_template.list',
|
||||
@@ -64,3 +81,107 @@ export const DELETE = withRouteContext(
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
/**
|
||||
* PATCH /api/settings/counterparty-templates
|
||||
* Rename a learned template. The old name is kept as an alias so both the
|
||||
* matcher (alias tier) and the learn path (findTemplateByKey) keep landing on
|
||||
* this row for the merchant it was learned from.
|
||||
*/
|
||||
export const PATCH = withRouteContext(
|
||||
'counterparty_template.rename',
|
||||
async (request, { supabase, companyId, log, requestId }) => {
|
||||
const validation = await validateBody(request, RenameCounterpartyTemplateSchema)
|
||||
if (!validation.success) return validation.response
|
||||
|
||||
const { id } = validation.data
|
||||
const newName = toTemplateKey(validation.data.counterparty_name)
|
||||
if (newName.length < 2) {
|
||||
return errorResponseFromCode('VALIDATION_ERROR', log, {
|
||||
requestId,
|
||||
messageSv: 'Namnet måste vara minst två tecken',
|
||||
messageEn: 'The name must be at least two characters',
|
||||
})
|
||||
}
|
||||
|
||||
const { data: existing, error: fetchError } = await supabase
|
||||
.from('categorization_templates')
|
||||
.select('*')
|
||||
.eq('id', id)
|
||||
.eq('company_id', companyId)
|
||||
.eq('is_active', true)
|
||||
.maybeSingle()
|
||||
if (fetchError) {
|
||||
return NextResponse.json({ error: getUserErrorMessage(fetchError) }, { status: 500 })
|
||||
}
|
||||
if (!existing) {
|
||||
return errorResponseFromCode('NOT_FOUND', log, {
|
||||
requestId,
|
||||
messageSv: 'Mallen hittades inte',
|
||||
messageEn: 'Template not found',
|
||||
})
|
||||
}
|
||||
const current = existing as CategorizationTemplate
|
||||
if (current.counterparty_name === newName) {
|
||||
return NextResponse.json({ data: current })
|
||||
}
|
||||
|
||||
// (company_id, counterparty_name) is UNIQUE and includes soft-deleted
|
||||
// rows. An active twin is a real conflict the user can see and resolve;
|
||||
// an inactive twin is invisible to them, so it is removed instead of
|
||||
// blocking the rename (a learned pattern, not a retention-bound record).
|
||||
const { data: twin, error: twinError } = await supabase
|
||||
.from('categorization_templates')
|
||||
.select('id, is_active')
|
||||
.eq('company_id', companyId)
|
||||
.eq('counterparty_name', newName)
|
||||
.neq('id', id)
|
||||
.maybeSingle()
|
||||
if (twinError) {
|
||||
return NextResponse.json({ error: getUserErrorMessage(twinError) }, { status: 500 })
|
||||
}
|
||||
if (twin?.is_active) {
|
||||
return errorResponseFromCode('CONFLICT', log, {
|
||||
requestId,
|
||||
messageSv: 'Det finns redan en mall med det här namnet',
|
||||
messageEn: 'A template with this name already exists',
|
||||
})
|
||||
}
|
||||
if (twin) {
|
||||
const { error: purgeError } = await supabase
|
||||
.from('categorization_templates')
|
||||
.delete()
|
||||
.eq('id', twin.id)
|
||||
.eq('company_id', companyId)
|
||||
if (purgeError) {
|
||||
return NextResponse.json({ error: getUserErrorMessage(purgeError) }, { status: 500 })
|
||||
}
|
||||
}
|
||||
|
||||
const aliases = [...(current.counterparty_aliases || [])]
|
||||
if (!aliases.includes(current.counterparty_name)) aliases.push(current.counterparty_name)
|
||||
|
||||
const { data: updated, error: updateError } = await supabase
|
||||
.from('categorization_templates')
|
||||
.update({ counterparty_name: newName, counterparty_aliases: aliases })
|
||||
.eq('id', id)
|
||||
.eq('company_id', companyId)
|
||||
.select('*')
|
||||
.single()
|
||||
if (updateError) {
|
||||
// Lost the race against another rename or the learn path: the unique
|
||||
// (company_id, counterparty_name) index rejected the new name.
|
||||
if ((updateError as { code?: string }).code === '23505') {
|
||||
return errorResponseFromCode('CONFLICT', log, {
|
||||
requestId,
|
||||
messageSv: 'Det finns redan en mall med det här namnet',
|
||||
messageEn: 'A template with this name already exists',
|
||||
})
|
||||
}
|
||||
return NextResponse.json({ error: getUserErrorMessage(updateError) }, { status: 500 })
|
||||
}
|
||||
|
||||
return NextResponse.json({ data: updated })
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
@@ -4,9 +4,10 @@ import { useTranslations } from 'next-intl'
|
||||
import { useState, useEffect, useCallback } from 'react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { EmptyState } from '@/components/ui/empty-state'
|
||||
import { Input } from '@/components/ui/input'
|
||||
import { useToast } from '@/components/ui/use-toast'
|
||||
import { SettingsGroup } from '@/components/settings/SettingsRows'
|
||||
import { Loader2, Trash2, Users, ChevronDown } from 'lucide-react'
|
||||
import { Loader2, Trash2, Users, ChevronDown, Pencil } from 'lucide-react'
|
||||
import { formatAccountWithName } from '@/lib/bookkeeping/client-account-names'
|
||||
import { formatCounterpartyName } from '@/lib/bookkeeping/counterparty-templates'
|
||||
import type { CategorizationTemplate } from '@/types'
|
||||
@@ -50,6 +51,9 @@ export function CounterpartyTemplatesPanel() {
|
||||
const [isLoading, setIsLoading] = useState(true)
|
||||
const [deletingId, setDeletingId] = useState<string | null>(null)
|
||||
const [expandedId, setExpandedId] = useState<string | null>(null)
|
||||
const [editingId, setEditingId] = useState<string | null>(null)
|
||||
const [editName, setEditName] = useState('')
|
||||
const [savingId, setSavingId] = useState<string | null>(null)
|
||||
|
||||
const fetchTemplates = useCallback(async () => {
|
||||
try {
|
||||
@@ -91,6 +95,48 @@ export function CounterpartyTemplatesPanel() {
|
||||
}
|
||||
}
|
||||
|
||||
function startRename(tt: CategorizationTemplate) {
|
||||
setEditingId(tt.id)
|
||||
setEditName(formatCounterpartyName(tt.counterparty_name))
|
||||
}
|
||||
|
||||
function cancelRename() {
|
||||
setEditingId(null)
|
||||
setEditName('')
|
||||
}
|
||||
|
||||
async function handleRename(id: string) {
|
||||
const name = editName.trim()
|
||||
if (!name) return
|
||||
setSavingId(id)
|
||||
try {
|
||||
const res = await fetch('/api/settings/counterparty-templates', {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ id, counterparty_name: name }),
|
||||
})
|
||||
if (res.status === 409) {
|
||||
toast({ title: t('toast_duplicate_name'), variant: 'destructive' })
|
||||
return
|
||||
}
|
||||
if (!res.ok) {
|
||||
toast({ title: t('toast_rename_failed'), variant: 'destructive' })
|
||||
return
|
||||
}
|
||||
const json = (await res.json()) as { data?: CategorizationTemplate }
|
||||
if (json.data) {
|
||||
const updated = json.data
|
||||
setTemplates((prev) => prev.map((tt) => (tt.id === id ? { ...tt, ...updated } : tt)))
|
||||
}
|
||||
cancelRename()
|
||||
toast({ title: t('toast_renamed') })
|
||||
} catch {
|
||||
toast({ title: t('toast_rename_failed'), variant: 'destructive' })
|
||||
} finally {
|
||||
setSavingId(null)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<SettingsGroup label={t('title')} help={t('description')}>
|
||||
{isLoading ? (
|
||||
@@ -213,8 +259,58 @@ export function CounterpartyTemplatesPanel() {
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Delete */}
|
||||
<div className="flex justify-end pt-1">
|
||||
{/* Rename + delete */}
|
||||
{editingId === tt.id ? (
|
||||
<form
|
||||
className="flex items-center gap-2 pt-1"
|
||||
onSubmit={(e) => {
|
||||
e.preventDefault()
|
||||
handleRename(tt.id)
|
||||
}}
|
||||
>
|
||||
<Input
|
||||
value={editName}
|
||||
onChange={(e) => setEditName(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === 'Escape') cancelRename()
|
||||
}}
|
||||
maxLength={100}
|
||||
autoFocus
|
||||
aria-label={t('rename_label')}
|
||||
className="h-8 max-w-xs text-sm"
|
||||
/>
|
||||
<Button
|
||||
type="submit"
|
||||
size="sm"
|
||||
disabled={savingId === tt.id || editName.trim().length < 2}
|
||||
className="h-7 text-xs"
|
||||
>
|
||||
{savingId === tt.id ? <Loader2 className="mr-1.5 h-3 w-3 animate-spin" /> : null}
|
||||
{t('rename_save')}
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
onClick={cancelRename}
|
||||
disabled={savingId === tt.id}
|
||||
className="h-7 text-xs"
|
||||
>
|
||||
{t('rename_cancel')}
|
||||
</Button>
|
||||
</form>
|
||||
) : (
|
||||
<div className="flex justify-end gap-2 pt-1">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => startRename(tt)}
|
||||
disabled={deletingId === tt.id}
|
||||
className="h-7 text-xs"
|
||||
>
|
||||
<Pencil className="mr-1.5 h-3 w-3" />
|
||||
{t('rename_button')}
|
||||
</Button>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
@@ -230,6 +326,7 @@ export function CounterpartyTemplatesPanel() {
|
||||
{t('delete_button')}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -520,6 +520,9 @@ describe('counterparty-templates', () => {
|
||||
const chain = {
|
||||
select: () => chain,
|
||||
eq: () => chain,
|
||||
contains: () => chain,
|
||||
order: () => chain,
|
||||
limit: () => chain,
|
||||
maybeSingle: async () => ({ data: null, error: null }),
|
||||
insert: async (payload: Record<string, unknown>) => {
|
||||
inserted.push(payload)
|
||||
@@ -710,15 +713,123 @@ describe('counterparty-templates', () => {
|
||||
expect(supabase.from).toHaveBeenCalledWith('categorization_templates')
|
||||
})
|
||||
|
||||
it('does DB lookup when existingTemplate is undefined', async () => {
|
||||
it('does DB lookup by name, then by alias, when existingTemplate is undefined', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null }) // select returns null
|
||||
enqueue({ data: null }) // select by name returns null
|
||||
enqueue({ data: null }) // select by alias returns null
|
||||
enqueue({ data: null }) // insert
|
||||
|
||||
await insertOrUpdateTemplate(supabase as never, 'user-1', baseParams)
|
||||
|
||||
// Two calls: select + insert
|
||||
expect(supabase.from).toHaveBeenCalledTimes(2)
|
||||
// Three calls: select by name + select by alias + insert
|
||||
expect(supabase.from).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('re-approval lands on a renamed template through its alias, not as a new row', async () => {
|
||||
// A user rename moves the bank-derived key into counterparty_aliases.
|
||||
// The learn path derives its key from the bank description, so without
|
||||
// the alias leg every later approval would insert a duplicate.
|
||||
const { supabase, enqueue, findCall, calls } = createQueuedMockSupabase()
|
||||
const renamed = makeCategorizationTemplate({
|
||||
id: 'renamed-1',
|
||||
counterparty_name: 'spotify',
|
||||
counterparty_aliases: ['spotify ab stockholm 4471 kortköp', 'spotify ab stockholm 4471'],
|
||||
debit_account: '6200',
|
||||
credit_account: '1930',
|
||||
occurrence_count: 5,
|
||||
})
|
||||
enqueue({ data: null }) // select by name: nothing under the old key
|
||||
enqueue({ data: renamed }) // select by alias: the renamed row
|
||||
enqueue({ data: null }) // update
|
||||
|
||||
await insertOrUpdateTemplate(supabase as never, 'user-1', {
|
||||
...baseParams,
|
||||
counterpartyName: 'spotify ab stockholm 4471',
|
||||
})
|
||||
|
||||
const aliasFilter = findCall('categorization_templates', 'contains')
|
||||
expect(aliasFilter).toEqual(['counterparty_aliases', ['spotify ab stockholm 4471']])
|
||||
expect(calls.some((c) => c.method === 'insert')).toBe(false)
|
||||
const payload = findCall('categorization_templates', 'update')?.[0] as { occurrence_count: number }
|
||||
expect(payload.occurrence_count).toBe(6)
|
||||
})
|
||||
})
|
||||
|
||||
// ── findCounterpartyTemplate after a rename ──────────────────
|
||||
|
||||
describe('findCounterpartyTemplate after a user rename', () => {
|
||||
it('still proposes a template renamed to a label for a new bank-line variant of the merchant', async () => {
|
||||
// Learned from "Kortköp 260612 SPOTIFY AB" (key "spotify"), then renamed
|
||||
// to "Musik" by the user: the old key sits in aliases. Next month's line
|
||||
// carries a new date, so the raw-descriptor alias tier misses; the
|
||||
// normalized-name tier must resolve through the alias instead.
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
const renamed = makeCategorizationTemplate({
|
||||
id: 'renamed-1',
|
||||
counterparty_name: 'musik',
|
||||
counterparty_aliases: ['kortköp 260612 spotify ab', 'spotify'],
|
||||
occurrence_count: 2,
|
||||
confidence: 0.6,
|
||||
})
|
||||
enqueue({ data: [renamed] })
|
||||
|
||||
const tx = makeTransaction({
|
||||
merchant_name: null,
|
||||
original_description: 'Kortköp 260705 SPOTIFY AB',
|
||||
description: 'Kortköp 260705 SPOTIFY AB',
|
||||
})
|
||||
const match = await findCounterpartyTemplate(supabase as never, 'company-1', tx)
|
||||
|
||||
expect(match?.template.id).toBe('renamed-1')
|
||||
expect(match?.matchMethod).toBe('exact_normalized')
|
||||
})
|
||||
|
||||
it('a real counterparty_name beats another template carrying the same string as alias', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
const owner = makeCategorizationTemplate({
|
||||
id: 'owner',
|
||||
counterparty_name: 'spotify',
|
||||
counterparty_aliases: [],
|
||||
occurrence_count: 4,
|
||||
})
|
||||
const other = makeCategorizationTemplate({
|
||||
id: 'other',
|
||||
counterparty_name: 'musik',
|
||||
counterparty_aliases: ['spotify'],
|
||||
occurrence_count: 9,
|
||||
})
|
||||
enqueue({ data: [other, owner] })
|
||||
|
||||
const tx = makeTransaction({
|
||||
merchant_name: null,
|
||||
original_description: 'Kortköp 260705 SPOTIFY AB',
|
||||
description: 'Kortköp 260705 SPOTIFY AB',
|
||||
})
|
||||
const match = await findCounterpartyTemplate(supabase as never, 'company-1', tx)
|
||||
|
||||
expect(match?.template.id).toBe('owner')
|
||||
})
|
||||
|
||||
it('a bank line that is exactly a canonical name resolves to its owner, not to a row holding it as alias', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
const owner = makeCategorizationTemplate({
|
||||
id: 'owner',
|
||||
counterparty_name: 'spotify',
|
||||
counterparty_aliases: [],
|
||||
occurrence_count: 4,
|
||||
})
|
||||
const other = makeCategorizationTemplate({
|
||||
id: 'other',
|
||||
counterparty_name: 'musik',
|
||||
counterparty_aliases: ['spotify'],
|
||||
occurrence_count: 9,
|
||||
})
|
||||
enqueue({ data: [other, owner] })
|
||||
|
||||
const tx = makeTransaction({ merchant_name: 'spotify', original_description: 'spotify', description: 'spotify' })
|
||||
const match = await findCounterpartyTemplate(supabase as never, 'company-1', tx)
|
||||
|
||||
expect(match?.template.id).toBe('owner')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -324,19 +324,38 @@ export async function findCounterpartyTemplatesBatch(
|
||||
|
||||
const templates = allTemplates as CategorizationTemplate[]
|
||||
|
||||
// Build alias lookup: lowercase alias → template
|
||||
// Build alias lookup: lowercase alias → template. An alias that equals
|
||||
// another template's canonical counterparty_name (only reachable through a
|
||||
// user rename) must not shadow that template when a bank line is exactly
|
||||
// that string: the canonical owner wins.
|
||||
const canonicalOwner = new Map<string, CategorizationTemplate>()
|
||||
for (const tmpl of templates) {
|
||||
canonicalOwner.set(tmpl.counterparty_name, tmpl)
|
||||
}
|
||||
const aliasMap = new Map<string, CategorizationTemplate>()
|
||||
for (const tmpl of templates) {
|
||||
for (const alias of tmpl.counterparty_aliases || []) {
|
||||
const owner = canonicalOwner.get(alias)
|
||||
if (owner && owner.id !== tmpl.id) continue
|
||||
aliasMap.set(alias, tmpl)
|
||||
}
|
||||
}
|
||||
|
||||
// Build normalized name lookup
|
||||
// Build normalized name lookup. A user rename (PATCH
|
||||
// /api/settings/counterparty-templates) moves the bank-derived key into
|
||||
// counterparty_aliases; without the alias leg here, a template renamed to a
|
||||
// human label ("musik") would keep learning through findTemplateByKey but
|
||||
// never be proposed again, since the alias tier above only sees raw
|
||||
// descriptors. A real counterparty_name always wins over an alias.
|
||||
const nameMap = new Map<string, CategorizationTemplate>()
|
||||
for (const tmpl of templates) {
|
||||
nameMap.set(tmpl.counterparty_name, tmpl)
|
||||
}
|
||||
for (const tmpl of templates) {
|
||||
for (const alias of tmpl.counterparty_aliases || []) {
|
||||
if (!nameMap.has(alias)) nameMap.set(alias, tmpl)
|
||||
}
|
||||
}
|
||||
|
||||
for (const tx of transactions) {
|
||||
// Identity anchors on the immutable bank original, not the working title:
|
||||
@@ -752,6 +771,38 @@ export interface TemplateUpsertParams {
|
||||
defaultDimensions?: Record<string, string> | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the template that owns a learned key: by counterparty_name first,
|
||||
* then by alias. A user rename moves the old key into counterparty_aliases
|
||||
* (PATCH /api/settings/counterparty-templates), so the alias leg is what
|
||||
* keeps re-approvals landing on the renamed row instead of inserting a
|
||||
* second template under the bank-derived name.
|
||||
*/
|
||||
async function findTemplateByKey(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
counterpartyName: string
|
||||
): Promise<CategorizationTemplate | null> {
|
||||
const { data: byName } = await supabase
|
||||
.from('categorization_templates')
|
||||
.select('*')
|
||||
.eq('company_id', companyId)
|
||||
.eq('counterparty_name', counterpartyName)
|
||||
.maybeSingle()
|
||||
if (byName) return byName as CategorizationTemplate
|
||||
|
||||
const { data: byAlias } = await supabase
|
||||
.from('categorization_templates')
|
||||
.select('*')
|
||||
.eq('company_id', companyId)
|
||||
.eq('is_active', true)
|
||||
.contains('counterparty_aliases', [counterpartyName])
|
||||
.order('occurrence_count', { ascending: false })
|
||||
.limit(1)
|
||||
.maybeSingle()
|
||||
return (byAlias as CategorizationTemplate | null) ?? null
|
||||
}
|
||||
|
||||
/**
|
||||
* Low-level insert-or-update for a counterparty template.
|
||||
*
|
||||
@@ -781,13 +832,7 @@ export async function insertOrUpdateTemplate(
|
||||
// Resolve existing template
|
||||
let existing: CategorizationTemplate | null = null
|
||||
if (existingTemplate === undefined) {
|
||||
const { data } = await supabase
|
||||
.from('categorization_templates')
|
||||
.select('*')
|
||||
.eq('company_id', companyId)
|
||||
.eq('counterparty_name', params.counterpartyName)
|
||||
.maybeSingle()
|
||||
existing = data as CategorizationTemplate | null
|
||||
existing = await findTemplateByKey(supabase, companyId, params.counterpartyName)
|
||||
} else {
|
||||
existing = existingTemplate
|
||||
}
|
||||
@@ -1370,11 +1415,19 @@ export async function populateTemplatesFromSieVouchers(
|
||||
.eq('company_id', companyId)
|
||||
.eq('is_active', true)
|
||||
|
||||
// Keyed by counterparty_name, then by alias: a renamed template keeps its
|
||||
// old bank-derived key as an alias (see findTemplateByKey), and a SIE
|
||||
// re-import must update that row rather than insert a duplicate.
|
||||
const templateMap = new Map<string, CategorizationTemplate>()
|
||||
if (existingTemplates) {
|
||||
for (const t of existingTemplates) {
|
||||
templateMap.set(t.counterparty_name, t as CategorizationTemplate)
|
||||
}
|
||||
for (const t of existingTemplates) {
|
||||
for (const alias of (t.counterparty_aliases as string[] | null) || []) {
|
||||
if (!templateMap.has(alias)) templateMap.set(alias, t as CategorizationTemplate)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let count = 0
|
||||
|
||||
+8
-1
@@ -2613,7 +2613,14 @@
|
||||
"confidence_label": "Confidence",
|
||||
"last_seen_label": "Last used",
|
||||
"aliases_label": "Aliases",
|
||||
"delete_button": "Delete template"
|
||||
"delete_button": "Delete template",
|
||||
"rename_button": "Rename",
|
||||
"rename_label": "New name",
|
||||
"rename_save": "Save",
|
||||
"rename_cancel": "Cancel",
|
||||
"toast_renamed": "Template renamed",
|
||||
"toast_rename_failed": "Could not rename the template",
|
||||
"toast_duplicate_name": "A template with this name already exists"
|
||||
},
|
||||
"settings_tax_form": {
|
||||
"entity_form_heading": "Company form",
|
||||
|
||||
+8
-1
@@ -2613,7 +2613,14 @@
|
||||
"confidence_label": "Säkerhet",
|
||||
"last_seen_label": "Senast använd",
|
||||
"aliases_label": "Alias",
|
||||
"delete_button": "Ta bort mall"
|
||||
"delete_button": "Ta bort mall",
|
||||
"rename_button": "Byt namn",
|
||||
"rename_label": "Nytt namn",
|
||||
"rename_save": "Spara",
|
||||
"rename_cancel": "Avbryt",
|
||||
"toast_renamed": "Mallen döptes om",
|
||||
"toast_rename_failed": "Kunde inte byta namn på mallen",
|
||||
"toast_duplicate_name": "Det finns redan en mall med det här namnet"
|
||||
},
|
||||
"settings_tax_form": {
|
||||
"entity_form_heading": "Företagsform",
|
||||
|
||||
Reference in New Issue
Block a user