Skip to content

Commit

Permalink
fix: import error handling for broken CSV files, matchRuleId is null …
Browse files Browse the repository at this point in the history
…when there is no match
  • Loading branch information
morremeyer committed Dec 1, 2023
1 parent f1fec00 commit 8515e2c
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 12 deletions.
32 changes: 31 additions & 1 deletion api/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6274,7 +6274,7 @@ const docTemplate = `{
"description": "List of transaction previews",
"type": "array",
"items": {
"$ref": "#/definitions/importer.TransactionPreview"
"$ref": "#/definitions/importer.TransactionPreviewV3"
}
},
"error": {
Expand Down Expand Up @@ -7089,6 +7089,36 @@ const docTemplate = `{
}
}
},
"importer.TransactionPreviewV3": {
"type": "object",
"properties": {
"destinationAccountName": {
"description": "Name of the destination account from the CSV file",
"type": "string",
"example": "Deutsche Bahn"
},
"duplicateTransactionIds": {
"description": "IDs of transactions that this transaction duplicates",
"type": "array",
"items": {
"type": "string"
}
},
"matchRuleId": {
"description": "ID of the match rule that was applied to this transaction preview",
"type": "string",
"example": "042d101d-f1de-4403-9295-59dc0ea58677"
},
"sourceAccountName": {
"description": "Name of the source account from the CSV file",
"type": "string",
"example": "Employer"
},
"transaction": {
"$ref": "#/definitions/models.TransactionCreate"
}
}
},
"models.AccountCreate": {
"type": "object",
"properties": {
Expand Down
32 changes: 31 additions & 1 deletion api/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -6263,7 +6263,7 @@
"description": "List of transaction previews",
"type": "array",
"items": {
"$ref": "#/definitions/importer.TransactionPreview"
"$ref": "#/definitions/importer.TransactionPreviewV3"
}
},
"error": {
Expand Down Expand Up @@ -7078,6 +7078,36 @@
}
}
},
"importer.TransactionPreviewV3": {
"type": "object",
"properties": {
"destinationAccountName": {
"description": "Name of the destination account from the CSV file",
"type": "string",
"example": "Deutsche Bahn"
},
"duplicateTransactionIds": {
"description": "IDs of transactions that this transaction duplicates",
"type": "array",
"items": {
"type": "string"
}
},
"matchRuleId": {
"description": "ID of the match rule that was applied to this transaction preview",
"type": "string",
"example": "042d101d-f1de-4403-9295-59dc0ea58677"
},
"sourceAccountName": {
"description": "Name of the source account from the CSV file",
"type": "string",
"example": "Employer"
},
"transaction": {
"$ref": "#/definitions/models.TransactionCreate"
}
}
},
"models.AccountCreate": {
"type": "object",
"properties": {
Expand Down
24 changes: 23 additions & 1 deletion api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ definitions:
data:
description: List of transaction previews
items:
$ref: '#/definitions/importer.TransactionPreview'
$ref: '#/definitions/importer.TransactionPreviewV3'
type: array
error:
description: The error, if any occurred for this Match Rule
Expand Down Expand Up @@ -1264,6 +1264,28 @@ definitions:
transaction:
$ref: '#/definitions/models.TransactionCreate'
type: object
importer.TransactionPreviewV3:
properties:
destinationAccountName:
description: Name of the destination account from the CSV file
example: Deutsche Bahn
type: string
duplicateTransactionIds:
description: IDs of transactions that this transaction duplicates
items:
type: string
type: array
matchRuleId:
description: ID of the match rule that was applied to this transaction preview
example: 042d101d-f1de-4403-9295-59dc0ea58677
type: string
sourceAccountName:
description: Name of the source account from the CSV file
example: Employer
type: string
transaction:
$ref: '#/definitions/models.TransactionCreate'
type: object
models.AccountCreate:
properties:
budgetId:
Expand Down
12 changes: 9 additions & 3 deletions pkg/controllers/import_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
)

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

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

c.JSON(http.StatusOK, ImportPreviewListV3{Data: transactions})
// We need to transform the responses for v3
v3Transactions := make([]importer.TransactionPreviewV3, 0, len(transactions))
for _, t := range transactions {
v3Transactions = append(v3Transactions, t.TransformV3())
}

c.JSON(http.StatusOK, ImportPreviewListV3{Data: v3Transactions})
}

// ImportYnab4 imports a YNAB 4 budget
Expand Down
8 changes: 3 additions & 5 deletions pkg/controllers/import_v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,9 @@ func (suite *TestSuiteStandard) TestImportYnabImportPreviewV3Match() {
assert.Equal(t, tt.destinationAccountIDs[i], transaction.Transaction.DestinationAccountID, "destinationAccountID does not match in line %d", line)
}

assert.Equal(t, matchRuleIDs[i], transaction.MatchRuleID, "Expected match rule has match '%s', actual match rule has match '%s'", matchRuleIDs[i], transaction.MatchRuleID)

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

assert.Equal(t, tt.envelopeIDs[i], transaction.Transaction.EnvelopeID, "proposed envelope ID does not match in line %d", line)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/importer/parser/ynab-import/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ func Parse(f io.Reader, account models.Account) ([]importer.TransactionPreview,
headerRow, err := reader.Read()
if err == io.EOF {
return []importer.TransactionPreview{}, nil
} else if err != nil {
// csv reading always returns usable error messages
return []importer.TransactionPreview{}, err

Check warning on line 33 in pkg/importer/parser/ynab-import/parse.go

View check run for this annotation

Codecov / codecov/patch

pkg/importer/parser/ynab-import/parse.go#L32-L33

Added lines #L32 - L33 were not covered by tests
}

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

date, err := time.Parse("01/02/2006", record[headers["Date"]])
Expand Down
1 change: 1 addition & 0 deletions pkg/importer/parser/ynab-import/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestErrors(t *testing.T) {
{"error-decimal-outflow.csv", "error in line 2 of the CSV: outflow could not be parsed to a decimal"},
{"error-missing-amount.csv", "error in line 3 of the CSV: no amount is set for the transaction"},
{"error-outflow-and-inflow.csv", "error in line 2 of the CSV: both outflow and inflow are set for the transaction"},
{"unparsed-file.csv", "parse error on line 2, column 28: extraneous or missing \" in quoted-field"},
}

for _, tt := range tests {
Expand Down
25 changes: 25 additions & 0 deletions pkg/importer/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,28 @@ type TransactionPreview struct {

MatchRuleID uuid.UUID `json:"matchRuleId" example:"042d101d-f1de-4403-9295-59dc0ea58677"` // ID of the match rule that was applied to this transaction preview
}

// transformV3 transforms a TransactionPreview to a TransactionPreviewV3.
func (t TransactionPreview) TransformV3() TransactionPreviewV3 {
id := &t.MatchRuleID
if t.MatchRuleID == uuid.Nil {
id = nil
}

return TransactionPreviewV3{
Transaction: t.Transaction,
SourceAccountName: t.SourceAccountName,
DestinationAccountName: t.DestinationAccountName,
DuplicateTransactionIDs: t.DuplicateTransactionIDs,
MatchRuleID: id,
}
}

// TransactionPreviewV3 is used to preview transactions that will be imported to allow for editing.
type TransactionPreviewV3 struct {
Transaction models.TransactionCreate `json:"transaction"`
SourceAccountName string `json:"sourceAccountName" example:"Employer"` // Name of the source account from the CSV file
DestinationAccountName string `json:"destinationAccountName" example:"Deutsche Bahn"` // Name of the destination account from the CSV file
DuplicateTransactionIDs []uuid.UUID `json:"duplicateTransactionIds"` // IDs of transactions that this transaction duplicates
MatchRuleID *uuid.UUID `json:"matchRuleId" example:"042d101d-f1de-4403-9295-59dc0ea58677"` // ID of the match rule that was applied to this transaction preview
}
10 changes: 10 additions & 0 deletions testdata/importer/ynab-import/unparsed-file.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
;
"Ums�tze Verrechnungskonto ";"Zeitraum: 30 Tage";
"Neuer Kontostand";"16,94 EUR";

"Buchungstag";"Wertstellung (Valuta)";"Vorgang";"Buchungstext";"Umsatz in EUR";
"01.04.2019";"03.04.2019";"Wertpapiere";" Buchungstext: Test Ref. 000000000/0 ";"-59,97";
"01.04.2019";"03.04.2019";"Wertpapiere";" Buchungstext: ISHSII-MSCI EUR.SRI EOACC WPKNR: A1H7ZS ISIN: IE00B52VJ196 Ref. 0000000000000/0 ";"-119,98";
"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";

"Alter Kontostand";"16,89 EUR";

0 comments on commit 8515e2c

Please sign in to comment.