-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
d29f598
9023d26
3ed485f
16bf389
d826d4e
c4167b5
8a3a69c
aa6e6b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ | |
.fints300.json | ||
.fints300_haspa.json | ||
vendor/ | ||
.idea |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ import ( | |
|
||
// MT940 represents a S.W.I.F.T. Transaction Report | ||
type MT940 struct { | ||
ReferenceDate time.Time | ||
JobReference *AlphaNumericTag | ||
Reference *AlphaNumericTag | ||
Account *AccountTag | ||
|
@@ -163,7 +164,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 | ||
|
@@ -175,7 +176,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") | ||
} | ||
|
@@ -212,7 +213,7 @@ type TransactionTag struct { | |
} | ||
|
||
// Unmarshal unmarshals value into t | ||
func (t *TransactionTag) Unmarshal(value []byte) error { | ||
func (t *TransactionTag) Unmarshal(value []byte, bookingYear int) error { | ||
elements, err := extractTagElements(value) | ||
if err != nil { | ||
return err | ||
|
@@ -223,7 +224,7 @@ func (t *TransactionTag) Unmarshal(value []byte) 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") | ||
} | ||
|
@@ -235,13 +236,13 @@ func (t *TransactionTag) Unmarshal(value []byte) error { | |
if unicode.IsDigit(r) { | ||
buf.UnreadRune() | ||
dateBytes = buf.Next(4) | ||
date, err = parseDate(dateBytes, t.ValutaDate.Year()) | ||
date, err = parseDate(dateBytes, bookingYear) | ||
if err != nil { | ||
return errors.WithMessage(err, "unmarshal transaction tag: parsing booking date") | ||
} | ||
t.BookingDate = domain.NewShortDate(date) | ||
monthDiff := int(math.Abs(float64(t.ValutaDate.Month() - t.BookingDate.Month()))) | ||
if monthDiff > 1 { | ||
diff := t.ValutaDate.Sub(t.BookingDate.Time).Hours() / 24 | ||
if diff > 31 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I haven't yet understood what we're calculating here and why we're checking for 31. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how may days can be between a booking and a valuta date? I am sure it us less then 31 days. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. regarding this thread https://homebanking-hilfe.de/forum/topic.php?t=8970 which may or may not be representative the number of days can be way higher |
||
t.BookingDate = domain.NewShortDate(t.BookingDate.AddDate(1, 0, 0)) | ||
} | ||
} | ||
|
@@ -320,11 +321,28 @@ 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 | ||
//the referenceYear should be year of today, | ||
// if we are close to century stepp in in 2099 wie could have differences from 99 years to 100 years | ||
// assuming the maximum of fetched years can be 1 year. This logic could work with fetching up to 100 years | ||
// for this we should pass fetchingYearsInPast as param to this method. If we would be able to fetch a longer time, | ||
// the 2 digit year of swift is not enough. But this very theoretical | ||
fetchingYearsInPast := float64(1) | ||
if math.Abs(float64(diff)) >= (100 - fetchingYearsInPast) { | ||
if math.Signbit(float64(diff)) { | ||
date = time.Date(referenceYear+(diff+100), date.Month(), date.Day(), 0, 0, 0, 0, time.UTC) | ||
} else { | ||
date = time.Date(referenceYear+(diff-100), date.Month(), date.Day(), 0, 0, 0, 0, time.UTC) | ||
} | ||
|
||
} | ||
return date.Truncate(24 * time.Hour), nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,6 @@ package swift | |
|
||
import ( | ||
"reflect" | ||
"strconv" | ||
"strings" | ||
"testing" | ||
"time" | ||
|
||
|
@@ -139,7 +137,7 @@ func TestTransactionTagUnmarshal(t *testing.T) { | |
for _, test := range tests { | ||
tag := &TransactionTag{} | ||
|
||
err := tag.Unmarshal([]byte(test.marshaledValue)) | ||
err := tag.Unmarshal([]byte(test.marshaledValue), 2015) | ||
|
||
if err != nil { | ||
t.Logf("Expected no error, got %T:%v\n", err, err) | ||
|
@@ -154,65 +152,95 @@ func TestTransactionTagUnmarshal(t *testing.T) { | |
} | ||
} | ||
|
||
func TestTransactionTagOrder(t *testing.T) { | ||
testdata := "\r\n:20:HBCIKTOLST" | ||
for i := 0; i < 10; i++ { | ||
testdata += "\r\n:25:12345678/1234123456" + | ||
"\r\n:28C:0" + | ||
"\r\n:60F:C181105EUR1234,56" + | ||
"\r\n:61:1811051105DR50,NMSCNONREF" + | ||
"\r\n/OCMT/EUR50,//CHGS/ 0,/" + | ||
"\r\n:86:177?00SB-SEPA-Ueberweisung?20" + strconv.Itoa(i+10) + " ?30?31?32Max Meier ?33 ?34000" + | ||
"\r\n:62F:C190125EUR1234,56" | ||
func TestBookingDateBug(t *testing.T) { | ||
|
||
tests := []struct { | ||
today string | ||
balanceStartBookingDateString string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are already using the dates as |
||
balanceClosingBookingDateString string | ||
bookingDateString string | ||
valutaDateString string | ||
}{ | ||
{ | ||
today: "2019-02-10", | ||
bookingDateString: "2018-12-28", | ||
valutaDateString: "2019-01-01", | ||
balanceStartBookingDateString: "2018-12-28", | ||
balanceClosingBookingDateString: "2019-01-25", | ||
}, | ||
{ | ||
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", | ||
}, { | ||
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", | ||
}, | ||
} | ||
testdata += "\r\n-" | ||
mt := &MT940{} | ||
mt.Unmarshal([]byte(testdata)) | ||
for i, tr := range mt.Transactions { | ||
if strings.TrimSpace(tr.Description.Purpose[0]) != strconv.Itoa(i+10) { | ||
t.Logf("Purpose at index %d should be %d but is %s", i, i+10, tr.Description.Purpose[0]) | ||
|
||
for _, test := range tests { | ||
mt := &MT940{} | ||
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.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() | ||
} | ||
|
||
func TestTransactionListWithUnvalidData(t *testing.T) { | ||
testdata := "\r\n:20:HBCIKTOLST" | ||
testdata += "\r\n:25:12345678/1234123456" + | ||
"\r\n:28C:0" + | ||
"\r\n:60F:C181105EUR1234,56" + | ||
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Meier ?33 ?34000" + | ||
"\r\n:62F:C190125EUR1234,56" | ||
|
||
testdata += "\r\n-" | ||
mt := &MT940{} | ||
error := mt.Unmarshal([]byte(testdata)) | ||
if error == nil { | ||
t.Log("Error expected because of CustomTag without TransactionTag") | ||
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() | ||
} | ||
|
||
} | ||
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() | ||
} | ||
|
||
func TestTransactionWithRedefineCustomDataTag(t *testing.T) { | ||
testdata := "\r\n:20:HBCIKTOLST" | ||
testdata += "\r\n:25:12345678/1234123456" + | ||
"\r\n:28C:0" + | ||
"\r\n:60F:C181105EUR1234,56" + | ||
|
||
"\r\n:61:1811051105DR50,NMSCNONREF" + | ||
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Meier ?33 ?34000" + | ||
"\r\n:86:177?00SB-SEPA-Ueberweisung?20 ?30?31?32Max Meier ?33 ?34000" + | ||
"\r\n:62F:C190125EUR1234,56" | ||
|
||
testdata += "\r\n-" | ||
mt := &MT940{} | ||
error := mt.Unmarshal([]byte(testdata)) | ||
if error == nil { | ||
t.Log("Error expected because of more than CustomTag after TransactionTag") | ||
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() | ||
} | ||
} | ||
|
||
} |
There was a problem hiding this comment.
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 totime.Now
atelement.swift.go:30
. What do you think about not exposing it yet and guarding against zero value within theUnmarshal
method atswift/mt940_unmarshaler.go:12
?