fix: SIE import duplicate check, performance, and UX improvements (#162)
* fix: catch duplicate SIE import early with clear error message - Add duplicate check in execute route before doing any work (defense in depth) - Handle duplicate error from execute route in frontend - Show "Filen har redan importerats" heading instead of generic "Kunde inte läsa filen" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: catch duplicate SIE import early and batch account creation for performance - Add duplicate check in execute route before doing any work (defense in depth) - Handle duplicate error from execute route in frontend with clear Swedish message - Replace sequential ensureAccountExists loop (50-100 DB round trips) with single batch SELECT + batch INSERT — reduces import time from 3+ min to seconds Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update mappings optimistically after creating missing accounts Previously re-parsed the SIE file after account creation, which could fail with a 409 duplicate error (leaving the "create accounts" card stuck). Now optimistically marks created accounts as self-mapped and updates the preview stats immediately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Greptile P2 feedback — error handling and typed errorType prop - Check batch insert error in executeSIEImport account creation safety net - Replace brittle string-match error detection with typed errorType prop - Remove stale file dependency from handleCreateAccounts useCallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
@@ -322,6 +322,7 @@ function SIEImportWizard() {
|
||||
const [step, setStep] = useState<ImportWizardStep>('upload')
|
||||
const [isLoading, setIsLoading] = useState(false)
|
||||
const [error, setError] = useState<string | null>(null)
|
||||
const [errorType, setErrorType] = useState<'duplicate' | 'duplicate_period' | 'validation' | 'parse' | undefined>()
|
||||
|
||||
const [file, setFile] = useState<File | null>(null)
|
||||
const [, setParsed] = useState<ParsedSIEFile | null>(null)
|
||||
@@ -345,6 +346,7 @@ function SIEImportWizard() {
|
||||
const handleFileSelect = useCallback(async (selectedFile: File) => {
|
||||
setFile(selectedFile)
|
||||
setError(null)
|
||||
setErrorType(undefined)
|
||||
setIsLoading(true)
|
||||
|
||||
try {
|
||||
@@ -360,10 +362,13 @@ function SIEImportWizard() {
|
||||
|
||||
if (!res.ok) {
|
||||
if (data.error === 'duplicate' || data.error === 'duplicate_period') {
|
||||
setErrorType(data.error)
|
||||
setError(data.message)
|
||||
} else if (data.error === 'validation') {
|
||||
setErrorType('validation')
|
||||
setError(`${data.message}: ${data.errors?.join(', ') || 'Unknown validation error'}`)
|
||||
} else {
|
||||
setErrorType('parse')
|
||||
setError(data.error || 'Failed to parse file')
|
||||
}
|
||||
return
|
||||
@@ -450,30 +455,38 @@ function SIEImportWizard() {
|
||||
|
||||
toast({ title: 'Konton skapade', description: `${data.created} nya konton har lagts till i din kontoplan` })
|
||||
|
||||
if (file) {
|
||||
const formData = new FormData()
|
||||
formData.append('file', file)
|
||||
|
||||
const parseRes = await fetch('/api/import/sie/parse', { method: 'POST', body: formData })
|
||||
const parseData = await parseRes.json()
|
||||
|
||||
if (parseRes.ok) {
|
||||
setMappings(parseData.mappings)
|
||||
setPreview(parseData.preview)
|
||||
|
||||
const accountsRes = await fetch('/api/bookkeeping/accounts')
|
||||
if (accountsRes.ok) {
|
||||
const accountsData = await accountsRes.json()
|
||||
setBasAccounts(accountsData.data || [])
|
||||
}
|
||||
// Optimistically update mappings: mark created accounts as self-mapped
|
||||
const createdSet = new Set(missingAccounts.map(a => a.number))
|
||||
setMappings(prev => prev.map(m =>
|
||||
!m.targetAccount && createdSet.has(m.sourceAccount)
|
||||
? { ...m, targetAccount: m.sourceAccount, targetName: m.sourceName, confidence: 1.0 }
|
||||
: m
|
||||
))
|
||||
setPreview(prev => {
|
||||
if (!prev) return prev
|
||||
const newMapped = prev.mappingStatus.mapped + createdSet.size
|
||||
return {
|
||||
...prev,
|
||||
mappingStatus: {
|
||||
...prev.mappingStatus,
|
||||
mapped: newMapped,
|
||||
unmapped: Math.max(0, prev.mappingStatus.unmapped - createdSet.size),
|
||||
},
|
||||
}
|
||||
})
|
||||
|
||||
// Also refresh BAS accounts list
|
||||
const accountsRes = await fetch('/api/bookkeeping/accounts')
|
||||
if (accountsRes.ok) {
|
||||
const accountsData = await accountsRes.json()
|
||||
setBasAccounts(accountsData.data || [])
|
||||
}
|
||||
} catch (err) {
|
||||
toast({ title: 'Kunde inte skapa konton', description: err instanceof Error ? err.message : 'Försök igen.', variant: 'destructive' })
|
||||
} finally {
|
||||
setIsCreatingAccounts(false)
|
||||
}
|
||||
}, [missingAccounts, file, toast])
|
||||
}, [missingAccounts, toast])
|
||||
|
||||
const handleExecuteImport = useCallback(async (options: ImportExecuteOptions) => {
|
||||
if (!file) { setError('No file selected'); return }
|
||||
@@ -491,6 +504,10 @@ function SIEImportWizard() {
|
||||
const data = await res.json()
|
||||
|
||||
if (!res.ok) {
|
||||
if (data.error === 'duplicate') {
|
||||
setError(data.message || 'Denna fil har redan importerats')
|
||||
return
|
||||
}
|
||||
if (data.result) { setImportResult(data.result) } else { setError(data.error || 'Import failed'); return }
|
||||
} else {
|
||||
setImportResult(data.result)
|
||||
@@ -513,7 +530,7 @@ function SIEImportWizard() {
|
||||
|
||||
const handleNewImport = () => {
|
||||
setStep('upload'); setFile(null); setParsed(null); setMappings([])
|
||||
setPreview(null); setIssues([]); setImportResult(null); setError(null)
|
||||
setPreview(null); setIssues([]); setImportResult(null); setError(null); setErrorType(undefined)
|
||||
setSieAccounts([]); setIsCreatingAccounts(false)
|
||||
}
|
||||
|
||||
@@ -540,7 +557,7 @@ function SIEImportWizard() {
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
{step === 'upload' && <SIEUploadStep onFileSelect={handleFileSelect} isLoading={isLoading} error={error} />}
|
||||
{step === 'upload' && <SIEUploadStep onFileSelect={handleFileSelect} isLoading={isLoading} error={error} errorType={errorType} />}
|
||||
{step === 'preview' && preview && (
|
||||
<SIEPreviewStep preview={preview} issues={issues} missingAccounts={missingAccounts}
|
||||
onCreateAccounts={handleCreateAccounts} isCreatingAccounts={isCreatingAccounts}
|
||||
|
||||
@@ -4,7 +4,7 @@ import { NextResponse } from 'next/server'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { parseSIEFile, detectEncoding, decodeBuffer } from '@/lib/import/sie-parser'
|
||||
import { suggestMappings } from '@/lib/import/account-mapper'
|
||||
import { executeSIEImport } from '@/lib/import/sie-import'
|
||||
import { executeSIEImport, checkDuplicateImport } from '@/lib/import/sie-import'
|
||||
import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-data'
|
||||
import { getBASReference } from '@/lib/bookkeeping/bas-reference'
|
||||
import type { AccountMapping, SIEAccountMappingRecord } from '@/lib/import/types'
|
||||
@@ -56,6 +56,15 @@ export async function POST(request: Request) {
|
||||
// Parse the SIE file
|
||||
const parsed = parseSIEFile(content)
|
||||
|
||||
// Check for duplicate import before doing any work
|
||||
const duplicate = await checkDuplicateImport(supabase, companyId, content)
|
||||
if (duplicate) {
|
||||
return NextResponse.json({
|
||||
error: 'duplicate',
|
||||
message: `Denna fil har redan importerats ${duplicate.imported_at ? new Date(duplicate.imported_at).toLocaleDateString('sv-SE') : ''}`.trim(),
|
||||
}, { status: 409 })
|
||||
}
|
||||
|
||||
// Get mappings - either from request or generate new ones
|
||||
let mappings: AccountMapping[]
|
||||
|
||||
|
||||
@@ -16,9 +16,10 @@ interface SIEUploadStepProps {
|
||||
onFileSelect: (file: File) => void
|
||||
isLoading: boolean
|
||||
error: string | null
|
||||
errorType?: 'duplicate' | 'duplicate_period' | 'validation' | 'parse'
|
||||
}
|
||||
|
||||
export default function SIEUploadStep({ onFileSelect, isLoading, error }: SIEUploadStepProps) {
|
||||
export default function SIEUploadStep({ onFileSelect, isLoading, error, errorType }: SIEUploadStepProps) {
|
||||
const [isDragging, setIsDragging] = useState(false)
|
||||
const [selectedFile, setSelectedFile] = useState<File | null>(null)
|
||||
const [loadingPhase, setLoadingPhase] = useState(0)
|
||||
@@ -161,7 +162,11 @@ export default function SIEUploadStep({ onFileSelect, isLoading, error }: SIEUpl
|
||||
<div className="mt-4 p-4 bg-destructive/10 border border-destructive/20 rounded-lg flex gap-3">
|
||||
<AlertCircle className="h-5 w-5 text-destructive flex-shrink-0 mt-0.5" />
|
||||
<div>
|
||||
<p className="font-medium text-destructive">Kunde inte läsa filen</p>
|
||||
<p className="font-medium text-destructive">
|
||||
{errorType === 'duplicate' || errorType === 'duplicate_period'
|
||||
? 'Filen har redan importerats'
|
||||
: 'Kunde inte läsa filen'}
|
||||
</p>
|
||||
<p className="text-sm text-muted-foreground">{error}</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
+66
-14
@@ -1250,20 +1250,72 @@ export async function executeSIEImport(
|
||||
const accountMap = mappingsToMap(mappings)
|
||||
|
||||
// Ensure all mapped target accounts exist in chart_of_accounts.
|
||||
// The mapping contains every account referenced in the SIE file; accounts
|
||||
// that were not seeded during onboarding need to be created here so that
|
||||
// journal entry lines can link to them via account_id.
|
||||
const seenTargets = new Set<string>()
|
||||
for (const mapping of mappings) {
|
||||
if (mapping.targetAccount && !seenTargets.has(mapping.targetAccount)) {
|
||||
seenTargets.add(mapping.targetAccount)
|
||||
await ensureAccountExists(
|
||||
supabase,
|
||||
companyId,
|
||||
userId,
|
||||
mapping.targetAccount,
|
||||
mapping.targetName
|
||||
)
|
||||
// Uses a single batch query + batch insert instead of per-account round trips.
|
||||
const targetAccounts = [...new Set(
|
||||
mappings.filter(m => m.targetAccount).map(m => m.targetAccount!)
|
||||
)]
|
||||
|
||||
if (targetAccounts.length > 0) {
|
||||
const { data: existing } = await supabase
|
||||
.from('chart_of_accounts')
|
||||
.select('account_number')
|
||||
.eq('company_id', companyId)
|
||||
.in('account_number', targetAccounts)
|
||||
|
||||
const existingSet = new Set((existing || []).map(a => a.account_number))
|
||||
const missing = targetAccounts.filter(num => !existingSet.has(num))
|
||||
|
||||
if (missing.length > 0) {
|
||||
const targetNameMap = new Map<string, string>()
|
||||
for (const m of mappings) {
|
||||
if (m.targetAccount) targetNameMap.set(m.targetAccount, m.targetName || m.sourceName)
|
||||
}
|
||||
|
||||
const inserts = missing.map(num => {
|
||||
const basRef = getBASReference(num)
|
||||
if (basRef) {
|
||||
return {
|
||||
user_id: userId,
|
||||
company_id: companyId,
|
||||
account_number: num,
|
||||
account_name: basRef.account_name,
|
||||
account_class: basRef.account_class,
|
||||
account_group: basRef.account_group,
|
||||
account_type: basRef.account_type,
|
||||
normal_balance: basRef.normal_balance,
|
||||
sru_code: basRef.sru_code ?? computeSRUCode(num),
|
||||
k2_excluded: basRef.k2_excluded,
|
||||
plan_type: 'full_bas' as const,
|
||||
is_active: true,
|
||||
is_system_account: false,
|
||||
}
|
||||
}
|
||||
const classNum = parseInt(num.charAt(0), 10)
|
||||
const group = num.substring(0, 2)
|
||||
const accountType = classNum === 1 ? 'asset'
|
||||
: classNum === 2 ? (group === '21' ? 'untaxed_reserves' : (group === '20' ? 'equity' : 'liability'))
|
||||
: classNum === 3 ? 'revenue' : 'expense'
|
||||
return {
|
||||
user_id: userId,
|
||||
company_id: companyId,
|
||||
account_number: num,
|
||||
account_name: targetNameMap.get(num) || `Konto ${num}`,
|
||||
account_class: classNum,
|
||||
account_group: group,
|
||||
account_type: accountType,
|
||||
normal_balance: classNum <= 1 || classNum >= 4 ? 'debit' : 'credit',
|
||||
sru_code: computeSRUCode(num),
|
||||
plan_type: 'full_bas' as const,
|
||||
is_active: true,
|
||||
is_system_account: false,
|
||||
}
|
||||
})
|
||||
|
||||
const { error: insertError } = await supabase.from('chart_of_accounts').insert(inserts)
|
||||
if (insertError && !insertError.message.includes('duplicate')) {
|
||||
result.errors.push(`Failed to create accounts: ${insertError.message}`)
|
||||
return result
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user