Skip to content

Commit bcc9209

Browse files
morremeyermorremeyer
authored
fix: import error handling for broken CSV files, matchRuleId is null when there is no match (#867)
1 parent f1fec00 commit bcc9209

File tree

9 files changed

+138
-12
lines changed

9 files changed

+138
-12
lines changed

‎api/docs.go‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6274,7 +6274,7 @@ const docTemplate = `{
62746274
"description": "List of transaction previews",
62756275
"type": "array",
62766276
"items": {
6277-
"$ref": "#/definitions/importer.TransactionPreview"
6277+
"$ref": "#/definitions/importer.TransactionPreviewV3"
62786278
}
62796279
},
62806280
"error": {
@@ -7089,6 +7089,36 @@ const docTemplate = `{
70897089
}
70907090
}
70917091
},
7092+
"importer.TransactionPreviewV3": {
7093+
"type": "object",
7094+
"properties": {
7095+
"destinationAccountName": {
7096+
"description": "Name of the destination account from the CSV file",
7097+
"type": "string",
7098+
"example": "Deutsche Bahn"
7099+
},
7100+
"duplicateTransactionIds": {
7101+
"description": "IDs of transactions that this transaction duplicates",
7102+
"type": "array",
7103+
"items": {
7104+
"type": "string"
7105+
}
7106+
},
7107+
"matchRuleId": {
7108+
"description": "ID of the match rule that was applied to this transaction preview",
7109+
"type": "string",
7110+
"example": "042d101d-f1de-4403-9295-59dc0ea58677"
7111+
},
7112+
"sourceAccountName": {
7113+
"description": "Name of the source account from the CSV file",
7114+
"type": "string",
7115+
"example": "Employer"
7116+
},
7117+
"transaction": {
7118+
"$ref": "#/definitions/models.TransactionCreate"
7119+
}
7120+
}
7121+
},
70927122
"models.AccountCreate": {
70937123
"type": "object",
70947124
"properties": {

‎api/swagger.json‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6263,7 +6263,7 @@
62636263
"description": "List of transaction previews",
62646264
"type": "array",
62656265
"items": {
6266-
"$ref": "#/definitions/importer.TransactionPreview"
6266+
"$ref": "#/definitions/importer.TransactionPreviewV3"
62676267
}
62686268
},
62696269
"error": {
@@ -7078,6 +7078,36 @@
70787078
}
70797079
}
70807080
},
7081+
"importer.TransactionPreviewV3": {
7082+
"type": "object",
7083+
"properties": {
7084+
"destinationAccountName": {
7085+
"description": "Name of the destination account from the CSV file",
7086+
"type": "string",
7087+
"example": "Deutsche Bahn"
7088+
},
7089+
"duplicateTransactionIds": {
7090+
"description": "IDs of transactions that this transaction duplicates",
7091+
"type": "array",
7092+
"items": {
7093+
"type": "string"
7094+
}
7095+
},
7096+
"matchRuleId": {
7097+
"description": "ID of the match rule that was applied to this transaction preview",
7098+
"type": "string",
7099+
"example": "042d101d-f1de-4403-9295-59dc0ea58677"
7100+
},
7101+
"sourceAccountName": {
7102+
"description": "Name of the source account from the CSV file",
7103+
"type": "string",
7104+
"example": "Employer"
7105+
},
7106+
"transaction": {
7107+
"$ref": "#/definitions/models.TransactionCreate"
7108+
}
7109+
}
7110+
},
70817111
"models.AccountCreate": {
70827112
"type": "object",
70837113
"properties": {

‎api/swagger.yaml‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ definitions:
654654
data:
655655
description: List of transaction previews
656656
items:
657-
$ref: '#/definitions/importer.TransactionPreview'
657+
$ref: '#/definitions/importer.TransactionPreviewV3'
658658
type: array
659659
error:
660660
description: The error, if any occurred for this Match Rule
@@ -1264,6 +1264,28 @@ definitions:
12641264
transaction:
12651265
$ref: '#/definitions/models.TransactionCreate'
12661266
type: object
1267+
importer.TransactionPreviewV3:
1268+
properties:
1269+
destinationAccountName:
1270+
description: Name of the destination account from the CSV file
1271+
example: Deutsche Bahn
1272+
type: string
1273+
duplicateTransactionIds:
1274+
description: IDs of transactions that this transaction duplicates
1275+
items:
1276+
type: string
1277+
type: array
1278+
matchRuleId:
1279+
description: ID of the match rule that was applied to this transaction preview
1280+
example: 042d101d-f1de-4403-9295-59dc0ea58677
1281+
type: string
1282+
sourceAccountName:
1283+
description: Name of the source account from the CSV file
1284+
example: Employer
1285+
type: string
1286+
transaction:
1287+
$ref: '#/definitions/models.TransactionCreate'
1288+
type: object
12671289
models.AccountCreate:
12681290
properties:
12691291
budgetId:

‎pkg/controllers/import_v3.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import (
1717
)
1818

1919
type ImportPreviewListV3 struct {
20-
Data []importer.TransactionPreview `json:"data"` // List of transaction previews
21-
Error *string `json:"error" example:"the specified resource ID is not a valid UUID"` // The error, if any occurred for this Match Rule
20+
Data []importer.TransactionPreviewV3 `json:"data"` // List of transaction previews
21+
Error *string `json:"error" example:"the specified resource ID is not a valid UUID"` // The error, if any occurred for this Match Rule
2222
}
2323

2424
// RegisterImportRoutes registers the routes for imports.
@@ -211,7 +211,13 @@ func (co Controller) ImportYnabImportPreviewV3(c *gin.Context) {
211211
transactions[i] = transaction
212212
}
213213

214-
c.JSON(http.StatusOK, ImportPreviewListV3{Data: transactions})
214+
// We need to transform the responses for v3
215+
v3Transactions := make([]importer.TransactionPreviewV3, 0, len(transactions))
216+
for _, t := range transactions {
217+
v3Transactions = append(v3Transactions, t.TransformV3())
218+
}
219+
220+
c.JSON(http.StatusOK, ImportPreviewListV3{Data: v3Transactions})
215221
}
216222

217223
// ImportYnab4 imports a YNAB 4 budget

‎pkg/controllers/import_v3_test.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,9 @@ func (suite *TestSuiteStandard) TestImportYnabImportPreviewV3Match() {
310310
assert.Equal(t, tt.destinationAccountIDs[i], transaction.Transaction.DestinationAccountID, "destinationAccountID does not match in line %d", line)
311311
}
312312

313-
assert.Equal(t, matchRuleIDs[i], transaction.MatchRuleID, "Expected match rule has match '%s', actual match rule has match '%s'", matchRuleIDs[i], transaction.MatchRuleID)
314-
315-
// This is kept for backwards compatibility and will be removed with API version 3
316-
// https://github.com/envelope-zero/backend/issues/763
317-
assert.Equal(t, matchRuleIDs[i], transaction.RenameRuleID, "Expected rename rule has match '%s', actual rename rule has match '%s'", matchRuleIDs[i], transaction.MatchRuleID)
313+
if matchRuleIDs[i] != uuid.Nil {
314+
assert.Equal(t, matchRuleIDs[i], *transaction.MatchRuleID, "Expected match rule has match '%s', actual match rule has match '%s'", matchRuleIDs[i], transaction.MatchRuleID)
315+
}
318316

319317
assert.Equal(t, tt.envelopeIDs[i], transaction.Transaction.EnvelopeID, "proposed envelope ID does not match in line %d", line)
320318
}

‎pkg/importer/parser/ynab-import/parse.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ func Parse(f io.Reader, account models.Account) ([]importer.TransactionPreview,
2828
headerRow, err := reader.Read()
2929
if err == io.EOF {
3030
return []importer.TransactionPreview{}, nil
31+
} else if err != nil {
32+
// csv reading always returns usable error messages
33+
return []importer.TransactionPreview{}, err
3134
}
3235

3336
// Build map for header keys
@@ -42,7 +45,8 @@ func Parse(f io.Reader, account models.Account) ([]importer.TransactionPreview,
4245
break
4346
}
4447
if err != nil {
45-
return csvReadError(reader, fmt.Errorf("could not read line in CSV: %w", err))
48+
// csv reading always returns usable error messages
49+
return []importer.TransactionPreview{}, err
4650
}
4751

4852
date, err := time.Parse("01/02/2006", record[headers["Date"]])

‎pkg/importer/parser/ynab-import/parse_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ func TestErrors(t *testing.T) {
6666
{"error-decimal-outflow.csv", "error in line 2 of the CSV: outflow could not be parsed to a decimal"},
6767
{"error-missing-amount.csv", "error in line 3 of the CSV: no amount is set for the transaction"},
6868
{"error-outflow-and-inflow.csv", "error in line 2 of the CSV: both outflow and inflow are set for the transaction"},
69+
{"unparsed-file.csv", "parse error on line 2, column 28: extraneous or missing \" in quoted-field"},
6970
}
7071

7172
for _, tt := range tests {

‎pkg/importer/types.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,28 @@ type TransactionPreview struct {
6363

6464
MatchRuleID uuid.UUID `json:"matchRuleId" example:"042d101d-f1de-4403-9295-59dc0ea58677"` // ID of the match rule that was applied to this transaction preview
6565
}
66+
67+
// transformV3 transforms a TransactionPreview to a TransactionPreviewV3.
68+
func (t TransactionPreview) TransformV3() TransactionPreviewV3 {
69+
id := &t.MatchRuleID
70+
if t.MatchRuleID == uuid.Nil {
71+
id = nil
72+
}
73+
74+
return TransactionPreviewV3{
75+
Transaction: t.Transaction,
76+
SourceAccountName: t.SourceAccountName,
77+
DestinationAccountName: t.DestinationAccountName,
78+
DuplicateTransactionIDs: t.DuplicateTransactionIDs,
79+
MatchRuleID: id,
80+
}
81+
}
82+
83+
// TransactionPreviewV3 is used to preview transactions that will be imported to allow for editing.
84+
type TransactionPreviewV3 struct {
85+
Transaction models.TransactionCreate `json:"transaction"`
86+
SourceAccountName string `json:"sourceAccountName" example:"Employer"` // Name of the source account from the CSV file
87+
DestinationAccountName string `json:"destinationAccountName" example:"Deutsche Bahn"` // Name of the destination account from the CSV file
88+
DuplicateTransactionIDs []uuid.UUID `json:"duplicateTransactionIds"` // IDs of transactions that this transaction duplicates
89+
MatchRuleID *uuid.UUID `json:"matchRuleId" example:"042d101d-f1de-4403-9295-59dc0ea58677"` // ID of the match rule that was applied to this transaction preview
90+
}

‎testdata/importer/ynab-import/unparsed-file.csv‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
;
2+
"Ums�tze Verrechnungskonto ";"Zeitraum: 30 Tage";
3+
"Neuer Kontostand";"16,94 EUR";
4+
5+
"Buchungstag";"Wertstellung (Valuta)";"Vorgang";"Buchungstext";"Umsatz in EUR";
6+
"01.04.2019";"03.04.2019";"Wertpapiere";" Buchungstext: Test Ref. 000000000/0 ";"-59,97";
7+
"01.04.2019";"03.04.2019";"Wertpapiere";" Buchungstext: ISHSII-MSCI EUR.SRI EOACC WPKNR: A1H7ZS ISIN: IE00B52VJ196 Ref. 0000000000000/0 ";"-119,98";
8+
"01.04.2019";"01.04.2019";"DTA-glt. Buchung";" Zahlungspflichtiger: Leo BernardKto/IBAN: DE84100000012000035900 BLZ/BIC: XXXXXXXXX Buchungstext: Lastschrift Sparplan 1 Ref. H9219087I4644658/2 ";"180,00";
9+
10+
"Alter Kontostand";"16,89 EUR";

0 commit comments

Comments
 (0)