Skip to content

Commit

Permalink
fix(nordigen): nil pointer
Browse files Browse the repository at this point in the history
Refactor toYnabber into a single function, the nil check was on a layer
too high before. Remove that pit fall by just having one function for
the translation.

Move the strip function into the mapper, it was a leftover from when the
config value was defined on the YNAB side. We want to mangle data closet
to the read point.

commit-id:ca48367d
  • Loading branch information
martinohansen committed Sep 19, 2024
1 parent 533bc42 commit 7168309
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 99 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ require github.com/frieser/nordigen-go-lib/v2 v2.1.7
require github.com/kelseyhightower/envconfig v1.4.0

require github.com/carlmjohnson/versioninfo v0.22.5

require github.com/google/go-cmp v0.6.0
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ github.com/carlmjohnson/versioninfo v0.22.5 h1:O00sjOLUAFxYQjlN/bzYTuZiS0y6fWDQj
github.com/carlmjohnson/versioninfo v0.22.5/go.mod h1:QT9mph3wcVfISUKd0i9sZfVrPviHuSF+cUtLjm2WSf8=
github.com/frieser/nordigen-go-lib/v2 v2.1.7 h1:n6qhksPY9iPPXBmbdnIxwWQeaMM2fsQece4BlSNmfvc=
github.com/frieser/nordigen-go-lib/v2 v2.1.7/go.mod h1:NejYisqD8GvynCN0vDGw7J66slnj7jB25c8tS1tr8bw=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8=
github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg=
21 changes: 21 additions & 0 deletions reader/nordigen/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nordigen

import (
"fmt"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -37,6 +38,21 @@ func parseDate(t nordigen.Transaction) (time.Time, error) {
return date, nil
}

// payeeStripNonAlphanumeric removes all non-alphanumeric characters from payee
func payeeStripNonAlphanumeric(payee string) (x string) {
reg := regexp.MustCompile(`[^\p{L}]+`)
x = reg.ReplaceAllString(payee, " ")
return strings.TrimSpace(x)
}

// Strip removes each string in strips from s
func strip(s string, strips []string) string {
for _, strip := range strips {
s = strings.ReplaceAll(s, strip, "")
}
return strings.TrimSpace(s)
}

// defaultMapper is generic and tries to identify the appropriate mapping
func (r Reader) defaultMapper(a ynabber.Account, t nordigen.Transaction) (*ynabber.Transaction, error) {
PayeeSource := r.Config.Nordigen.PayeeSource
Expand Down Expand Up @@ -89,6 +105,11 @@ func (r Reader) defaultMapper(a ynabber.Account, t nordigen.Transaction) (*ynabb
}
}

// Remove elements in payee that is defined in config
if r.Config.Nordigen.PayeeStrip != nil {
payee = strip(payee, r.Config.Nordigen.PayeeStrip)
}

// Set the transaction ID according to config
var id string
switch TransactionID {
Expand Down
48 changes: 48 additions & 0 deletions reader/nordigen/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,51 @@ func TestParseAmount(t *testing.T) {
})
}
}

func TestPayeeStripNonAlphanumeric(t *testing.T) {
want := "Im just alphanumeric"
got := payeeStripNonAlphanumeric("Im just alphanumeric")
if want != got {
t.Fatalf("alphanumeric: %s != %s", want, got)
}

want = "你好世界"
got = payeeStripNonAlphanumeric("你好世界")
if want != got {
t.Fatalf("non-english: %s != %s", want, got)
}

want = "Im not j ust alphanumeric"
got = payeeStripNonAlphanumeric("Im! not j.ust alphanumeric42 69")
if want != got {
t.Fatalf("non-alphanumeric: %s != %s", want, got)
}
}

func TestStrip(t *testing.T) {
type args struct {
s string
strips []string
}
tests := []struct {
name string
args args
want string
}{
{name: "single",
args: args{s: "Im not here", strips: []string{"not "}},
want: "Im here",
},
{name: "multiple",
args: args{s: "Im not really here", strips: []string{"not ", "really "}},
want: "Im here",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := strip(tt.args.s, tt.args.strips); got != tt.want {
t.Errorf("Payee.Strip() = %v, want %v", got, tt.want)
}
})
}
}
29 changes: 5 additions & 24 deletions reader/nordigen/nordigen.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"errors"
"fmt"
"log/slog"
"regexp"
"strings"

"github.com/frieser/nordigen-go-lib/v2"
"github.com/martinohansen/ynabber"
Expand Down Expand Up @@ -33,31 +31,12 @@ func NewReader(cfg *ynabber.Config) Reader {
}
}

// payeeStripNonAlphanumeric removes all non-alphanumeric characters from payee
func payeeStripNonAlphanumeric(payee string) (x string) {
reg := regexp.MustCompile(`[^\p{L}]+`)
x = reg.ReplaceAllString(payee, " ")
return strings.TrimSpace(x)
}

func (r Reader) toYnabber(a ynabber.Account, t nordigen.Transaction) (*ynabber.Transaction, error) {
transaction, err := r.Mapper(a, t)
if err != nil {
return nil, err
}

// Execute strip method on payee if defined in config
if r.Config.Nordigen.PayeeStrip != nil {
transaction.Payee = transaction.Payee.Strip(r.Config.Nordigen.PayeeStrip)
}

return transaction, nil
}

func (r Reader) toYnabbers(a ynabber.Account, t nordigen.AccountTransactions) ([]ynabber.Transaction, error) {
skipped := 0

y := []ynabber.Transaction{}
for _, v := range t.Transactions.Booked {
transaction, err := r.toYnabber(a, v)
transaction, err := r.Mapper(a, v)
if err != nil {
return nil, err
}
Expand All @@ -67,10 +46,12 @@ func (r Reader) toYnabbers(a ynabber.Account, t nordigen.AccountTransactions) ([
r.logger.Debug("mapped transaction", "from", v, "to", transaction)
y = append(y, *transaction)
} else {
skipped++
r.logger.Debug("skipping", "transaction", v)
}

}
r.logger.Info("read transactions", "total", len(y)+skipped, "skipped", skipped)
return y, nil
}

Expand Down
96 changes: 61 additions & 35 deletions reader/nordigen/nordigen_test.go
Original file line number Diff line number Diff line change
@@ -1,39 +1,54 @@
package nordigen

import (
"reflect"
"log/slog"
"testing"
"time"

"github.com/frieser/nordigen-go-lib/v2"
"github.com/google/go-cmp/cmp"
"github.com/kelseyhightower/envconfig"
"github.com/martinohansen/ynabber"
)

func TestToYnabber(t *testing.T) {
// getAccountTransactions returns a nordigen.AccountTransactions with a single
// transaction for testing purposes.
func getAccountTransactions(t nordigen.Transaction) nordigen.AccountTransactions {
return nordigen.AccountTransactions{
Transactions: struct {
Booked []nordigen.Transaction `json:"booked,omitempty"`
Pending []nordigen.Transaction `json:"pending,omitempty"`
}{
Booked: []nordigen.Transaction{t},
},
}
}

func TestToYnabber(t *testing.T) {
logger := slog.Default()
var defaultConfig ynabber.Config
_ = envconfig.Process("", &defaultConfig)

type args struct {
account ynabber.Account
t nordigen.Transaction
t nordigen.AccountTransactions
}
tests := []struct {
name string
bankID string
reader Reader
args args
want *ynabber.Transaction
want []ynabber.Transaction
wantErr bool
}{
{
// Tests a common Nordigen transaction from NORDEA_NDEADKKK with the
// default config to highlight any breaking changes.
bankID: "NORDEA_NDEADKKK",
reader: Reader{Config: &defaultConfig},
reader: Reader{Config: &defaultConfig, logger: logger},
args: args{
account: ynabber.Account{Name: "foo", IBAN: "bar"},
t: nordigen.Transaction{
t: getAccountTransactions(nordigen.Transaction{
TransactionId: "H00000000000000000000",
EntryReference: "",
BookingDate: "2023-02-24",
Expand All @@ -56,24 +71,50 @@ func TestToYnabber(t *testing.T) {
RemittanceInformationUnstructuredArray: []string{""},
BankTransactionCode: "",
AdditionalInformation: "VISA KØB"},
),
},
want: &ynabber.Transaction{
want: []ynabber.Transaction{{
Account: ynabber.Account{Name: "foo", IBAN: "bar"},
ID: ynabber.ID("H00000000000000000000"),
Date: time.Date(2023, time.February, 24, 0, 0, 0, 0, time.UTC),
Payee: "Visa køb DKK HELLOFRESH Copenha Den",
Memo: "Visa køb DKK 424,00 HELLOFRESH Copenha Den 23.02",
Amount: ynabber.Milliunits(10000),
Amount: ynabber.Milliunits(10000)},
},
wantErr: false,
},
{
// Nordea should remove P transactions
name: "Remove P transactions",
bankID: "NORDEA_NDEADKKK",
reader: Reader{Config: &defaultConfig, logger: logger},
args: args{
account: ynabber.Account{Name: "foo", IBAN: "bar"},
t: getAccountTransactions(nordigen.Transaction{
TransactionId: "P4392858879202309260000000524",
BookingDate: "2023-09-26",
ValueDate: "2023-09-26",
TransactionAmount: struct {
Amount string `json:"amount,omitempty"`
Currency string `json:"currency,omitempty"`
}{
Amount: "-4200.00",
Currency: "DKK",
},
RemittanceInformationUnstructured: "Ovf. til, konto nr. 1111-222-333",
AdditionalInformation: "OVERFØRT TIL",
}),
},
want: []ynabber.Transaction{},
wantErr: false,
},
{
// Test transaction from SEB_KORT_AB_NO_SKHSFI21
bankID: "SEB_KORT_AB_NO_SKHSFI21",
reader: Reader{Config: &defaultConfig},
reader: Reader{Config: &defaultConfig, logger: logger},
args: args{
account: ynabber.Account{Name: "foo", IBAN: "bar"},
t: nordigen.Transaction{
t: getAccountTransactions(nordigen.Transaction{
TransactionId: "foobar",
EntryReference: "",
BookingDate: "2023-02-24",
Expand All @@ -96,54 +137,39 @@ func TestToYnabber(t *testing.T) {
RemittanceInformationUnstructuredArray: []string{""},
BankTransactionCode: "PURCHASE",
AdditionalInformation: "PASCAL AS"},
),
},
want: &ynabber.Transaction{
want: []ynabber.Transaction{{
Account: ynabber.Account{Name: "foo", IBAN: "bar"},
ID: ynabber.ID("foobar"),
Date: time.Date(2023, time.February, 24, 0, 0, 0, 0, time.UTC),
Payee: "PASCAL AS",
Memo: "",
Amount: ynabber.Milliunits(10000),
Amount: ynabber.Milliunits(10000)},
},
wantErr: false,
},
}

for _, tt := range tests {
t.Run(tt.bankID, func(t *testing.T) {
name := tt.bankID
if tt.name != "" {
name = name + "_" + tt.name
}
t.Run(name, func(t *testing.T) {

// Set the BankID to the test case but keep the rest of the config
// as is
tt.reader.Config.Nordigen.BankID = tt.bankID

got, err := tt.reader.toYnabber(tt.args.account, tt.args.t)
got, err := tt.reader.toYnabbers(tt.args.account, tt.args.t)
if (err != nil) != tt.wantErr {
t.Errorf("error = %+v, wantErr %+v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if cmp.Equal(got, tt.want) == false {
t.Errorf("got = \n%+v, want \n%+v", got, tt.want)
}
})
}
}

func TestPayeeStripNonAlphanumeric(t *testing.T) {
want := "Im just alphanumeric"
got := payeeStripNonAlphanumeric("Im just alphanumeric")
if want != got {
t.Fatalf("alphanumeric: %s != %s", want, got)
}

want = "你好世界"
got = payeeStripNonAlphanumeric("你好世界")
if want != got {
t.Fatalf("non-english: %s != %s", want, got)
}

want = "Im not j ust alphanumeric"
got = payeeStripNonAlphanumeric("Im! not j.ust alphanumeric42 69")
if want != got {
t.Fatalf("non-alphanumeric: %s != %s", want, got)
}
}
10 changes: 0 additions & 10 deletions ynabber.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ynabber

import (
"strconv"
"strings"
"time"
)

Expand All @@ -29,15 +28,6 @@ type ID string

type Payee string

// Strip removes the elements from s from the payee
func (p Payee) Strip(s []string) Payee {
x := string(p)
for _, strip := range s {
x = strings.ReplaceAll(x, strip, "")
}
return Payee(strings.TrimSpace(x))
}

type Milliunits int64

// Negate changes the sign of m to the opposite
Expand Down
Loading

0 comments on commit 7168309

Please sign in to comment.