Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

solution for booking date bug #18 #23

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

solution for booking date bug #18 #23

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d29f598
Merge pull request #1 from mitch000001/master
svenjeppsson Feb 3, 2019
9023d26
test: test for booking date bug
Jan 29, 2019
3ed485f
fix(swift): fix bookingdate bases correct bookingyear
Jan 29, 2019
16bf389
fix: removed debug printlin
svenjeppsson Feb 3, 2019
d826d4e
chore: make a tests better readable
Feb 3, 2019
c4167b5
fix(swift): test and fix for date century difference
Feb 3, 2019
8a3a69c
fix(swift): optimize an fix linting error
Feb 5, 2019
aa6e6b3
chore: add .idea to .gitignore
Feb 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions element/swift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package element
import (
"github.com/mitch000001/go-hbci/domain"
"github.com/mitch000001/go-hbci/swift"
"time"
)

// SwiftMT940DataElement represents a DataElement containing SWIFT MT940
Expand All @@ -26,6 +27,7 @@ func (s *SwiftMT940DataElement) UnmarshalHBCI(value []byte) error {
}
for _, message := range messages {
tr := &swift.MT940{}
tr.ReferenceDate = time.Now()
err = tr.Unmarshal(message)
if err != nil {
return err
Expand Down
20 changes: 17 additions & 3 deletions swift/mt940.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

// MT940 represents a S.W.I.F.T. Transaction Report
type MT940 struct {
ReferenceDate time.Time
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I read it correctly you're using ReferenceDate as a way for dependency injection. As the value is always hardcoded to time.Now at element.swift.go:30. What do you think about not exposing it yet and guarding against zero value within the Unmarshal method at swift/mt940_unmarshaler.go:12?

JobReference *AlphaNumericTag
Reference *AlphaNumericTag
Account *AccountTag
Expand Down Expand Up @@ -162,7 +163,7 @@ func (b *BalanceTag) Balance() domain.Balance {
}

// Unmarshal unmarshals value into b
func (b *BalanceTag) Unmarshal(value []byte) error {
func (b *BalanceTag) Unmarshal(value []byte, today time.Time) error {
elements, err := extractTagElements(value)
if err != nil {
return err
Expand All @@ -174,7 +175,7 @@ func (b *BalanceTag) Unmarshal(value []byte) error {
buf := bytes.NewBuffer(elements[1])
b.DebitCreditIndicator = string(buf.Next(1))
dateBytes := buf.Next(6)
date, err := parseDate(dateBytes, time.Now().Year())
date, err := parseDate(dateBytes, today.Year())
if err != nil {
return errors.WithMessage(err, "unmarshal balance tag: parsing booking date")
}
Expand Down Expand Up @@ -222,7 +223,7 @@ func (t *TransactionTag) Unmarshal(value []byte, bookingYear int) error {
t.Tag = string(elements[0])
buf := bytes.NewBuffer(elements[1])
dateBytes := buf.Next(6)
date, err := parseDate(dateBytes, time.Now().Year())
date, err := parseDate(dateBytes, bookingYear)
if err != nil {
return errors.WithMessage(err, "unmarshal transaction tag: parsing valuta date")
}
Expand Down Expand Up @@ -319,11 +320,24 @@ func parseDate(value []byte, referenceYear int) (time.Time, error) {
yearBegin := fmt.Sprintf("%d", referenceYear)[:offset]
dateString := yearBegin + string(value)
date, err := time.Parse("20060102", dateString)

if err != nil {
if strings.HasSuffix(dateString, "0229") {
return time.Date(referenceYear, 2, 29, 0, 0, 0, 0, time.UTC), nil
}
return time.Time{}, err
}

diff := date.Year() - referenceYear
if diff == 100 {
return time.Date(referenceYear, date.Month(), date.Day(), 0, 0, 0, 0, time.UTC), nil
}
if diff == 99 {
return time.Date(referenceYear-1, date.Month(), date.Day(), 0, 0, 0, 0, time.UTC), nil
}
if diff == -99 {
return time.Date(referenceYear+1, date.Month(), date.Day(), 0, 0, 0, 0, time.UTC), nil

}
return date.Truncate(24 * time.Hour), nil
}
102 changes: 65 additions & 37 deletions swift/mt940_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,66 +153,94 @@ func TestTransactionTagUnmarshal(t *testing.T) {
}

func TestBookingDateBug(t *testing.T) {

tests := []struct {
testdata string
expectedBookingDate string
today string
balanceStartBookingDateString string
Copy link
Owner

@mitch000001 mitch000001 Feb 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are already using the dates as time.Time in line 202ff we should change the type from string to time.Time. Also, the setup gets easier as we don't have to parse the date strings

balanceClosingBookingDateString string
bookingDateString string
valutaDateString string
}{
{
// Valuta Date 2019-01-01 Booking Date 2018-12-28
testdata: "\r\n:20:HBCIKTOLST" + "\r\n:25:12345678/1234123456" +
"\r\n:28C:0" +
"\r\n:60F:C181228EUR1234,56" +
"\r\n:61:1901011228DR50,NMSCNONREF" +
"\r\n/OCMT/EUR50,//CHGS/ 0,/" +
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Maier ?33 ?34000" +
"\r\n:62F:C190125EUR1234,56" +
"\r\n-",
expectedBookingDate: "2018-12-28",
today: "2019-02-10",
bookingDateString: "2018-12-28",
valutaDateString: "2019-01-01",
balanceStartBookingDateString: "2018-12-28",
balanceClosingBookingDateString: "2019-01-25",
},
// Valuta Date 2019-01-01 Booking Date 2019-01-01
{
testdata: "\r\n:20:HBCIKTOLST" + "\r\n:25:12345678/1234123456" +
"\r\n:28C:0" +
"\r\n:60F:C181228EUR1234,56" +
"\r\n:61:1901011228DR50,NMSCNONREF" +
"\r\n/OCMT/EUR50,//CHGS/ 0,/" +
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Maier ?33 ?34000" +
"\r\n:62F:C190125EUR1234,56" +
"\r\n-",
expectedBookingDate: "2018-12-28",
today: "2019-02-10",
bookingDateString: "2019-01-01",
valutaDateString: "2019-01-01",
balanceStartBookingDateString: "2018-12-28",
balanceClosingBookingDateString: "2019-01-25",
}, {
today: "2019-02-10",
bookingDateString: "2019-01-01",
valutaDateString: "2018-12-28",
balanceStartBookingDateString: "2018-12-28",
balanceClosingBookingDateString: "2019-01-25",
}, {
// Valuta Date 2018-12-28 Booking Date 2019-01-01 (not sure if that can happen ? )

testdata: "\r\n:20:HBCIKTOLST" + "\r\n:25:12345678/1234123456" +
"\r\n:28C:0" +
"\r\n:60F:C181228EUR1234,56" +
"\r\n:61:1812280101DR50,NMSCNONREF" +
"\r\n/OCMT/EUR50,//CHGS/ 0,/" +
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Maier ?33 ?34000" +
"\r\n:62F:C190125EUR1234,56" +
"\r\n-",
expectedBookingDate: "2019-01-01",
today: "2100-02-10",
bookingDateString: "2100-01-01",
valutaDateString: "2099-12-28",
balanceStartBookingDateString: "2099-12-28",
balanceClosingBookingDateString: "2100-01-25",
}, {
today: "2100-02-10",
bookingDateString: "2100-01-01",
valutaDateString: "2100-12-28",
balanceStartBookingDateString: "2099-12-28",
balanceClosingBookingDateString: "2100-01-25",
},
}

for _, test := range tests {
mt := &MT940{}

err := mt.Unmarshal([]byte(test.testdata))
today, _ := time.Parse("2006-01-02", test.today)
mt.ReferenceDate = today
expectedBookingDate, _ := time.Parse("2006-01-02", test.bookingDateString)
expectedValutaDate, _ := time.Parse("2006-01-02", test.valutaDateString)
expectedBalanceStartBookingDate, _ := time.Parse("2006-01-02", test.balanceStartBookingDateString)
expectedBalanceClosingBookingDate, _ := time.Parse("2006-01-02", test.balanceClosingBookingDateString)
testdata := "\r\n:20:HBCIKTOLST" + "\r\n:25:12345678/1234123456" +
"\r\n:28C:0" +
"\r\n:60F:C" + expectedBalanceStartBookingDate.Format("060102") + "EUR1234,56" +
"\r\n:61:" + expectedValutaDate.Format("060102") + expectedBookingDate.Format("0102") + "DR50,NMSCNONREF" +
"\r\n/OCMT/EUR50,//CHGS/ 0,/" +
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Maier ?33 ?34000" +
"\r\n:62F:C" + expectedBalanceClosingBookingDate.Format("060102") + "EUR1234,56" +
"\r\n-"
err := mt.Unmarshal([]byte(testdata))
if err != nil {
t.Log(err)
t.Fail()
}

if len(mt.Transactions) != 1 {
t.Log("There should be exactly one transaction")
t.Fail()
}

if test.expectedBookingDate != mt.Transactions[0].Transaction.BookingDate.String() {
if test.bookingDateString != mt.Transactions[0].Transaction.BookingDate.String() {
t.Logf("Booking date should be %s but is %s", test.bookingDateString, mt.Transactions[0].Transaction.BookingDate.String())
t.Fail()
}

if test.valutaDateString != mt.Transactions[0].Transaction.ValutaDate.String() {
t.Logf("Valudate date should be %s but is %s", test.valutaDateString, mt.Transactions[0].Transaction.ValutaDate.String())
t.Fail()
}

t.Logf("Booking date should be %s but is %s", test.expectedBookingDate, mt.Transactions[0].Transaction.BookingDate.String())
if test.balanceStartBookingDateString != mt.StartingBalance.BookingDate.String() {
t.Logf("balance start booking date should be %s but is %s", test.balanceStartBookingDateString, mt.StartingBalance.BookingDate.String())
t.Fail()
}

if test.balanceClosingBookingDateString != mt.ClosingBalance.BookingDate.String() {
t.Logf("balance closing booking date should be %s but is %s", test.balanceClosingBookingDateString, mt.ClosingBalance.BookingDate.String())
t.Fail()
}
}

}
8 changes: 4 additions & 4 deletions swift/mt940_unmarshaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,29 +47,29 @@ func (m *MT940) Unmarshal(value []byte) error {
}
case bytes.HasPrefix(tag, []byte(":60")):
m.StartingBalance = &BalanceTag{}
err = m.StartingBalance.Unmarshal(tag)
err = m.StartingBalance.Unmarshal(tag, m.ReferenceDate)
if err != nil {
return errors.WithMessage(err, "unmarshal starting balance tag")
}
balanceTagOpen = true
case bytes.HasPrefix(tag, []byte(":62")):

m.ClosingBalance = &BalanceTag{}
err = m.ClosingBalance.Unmarshal(tag)
err = m.ClosingBalance.Unmarshal(tag, m.ReferenceDate)
if err != nil {
return errors.WithMessage(err, "unmarshal closing balance tag")
}

balanceTagOpen = false
case bytes.HasPrefix(tag, []byte(":64:")):
m.CurrentValutaBalance = &BalanceTag{}
err = m.CurrentValutaBalance.Unmarshal(tag)
err = m.CurrentValutaBalance.Unmarshal(tag, m.ReferenceDate)
if err != nil {
return errors.WithMessage(err, "unmarshal current valuta balance tag")
}
case bytes.HasPrefix(tag, []byte(":65:")):
m.FutureValutaBalance = &BalanceTag{}
err = m.FutureValutaBalance.Unmarshal(tag)
err = m.FutureValutaBalance.Unmarshal(tag, m.ReferenceDate)
if err != nil {
return errors.WithMessage(err, "unmarshal future valuta balance tag")
}
Expand Down