exporting files to autogirot

[hanneskod]

The goal of this pr is to solve issue #5

Needs some work om byrokrat/autogiro first though. [DONE]

And needs pr #7 [DONE]

[hanneskod]

Almost done. Two issues though, see MandateApprovedState:

1. When adding a monthly transaction we need to compute a date. There should be some kind of setting for the end user, like 'draw money on the 28th of each month'. Then we should compute the next possible date when registering our transaction. The problem is how this setting is accessed in MandateApprovedState? The only current source of data is $donor. Is it ok to add a getTransactionDay() method to Donor?

2. Autogirot supports tracking transaction using a reference. For a monthly transaction the same reference would be reused. It would be cool to use getMandateKey() for this. That way we can track transactions to the correct mandate if our db contains multiple entries with the same payer number. The problem is that references may contain only 16 chars. Is there a way to let getMandateKey() be 16 chars and still have an acceptably low risk of collisions?? Or do we add getTransactionReference()?

@nonbinary Any suggestions?

[nonbinary]

1: Yes, we'll should put it in Donor, for now at least. In an end-user situation, this will probably not be set on a per-Donor basis, rather by group, or by setting a default. But that can be implemented in the frontend. Unless a more suitable place comes up, Donor is the only logical place. Will implement.

[nonbinary]

2. There should definitely be a 16-char solution. The avoidance of MD5 is mainly considering malicious attacks, not actual random hash collisions. I still feel we should avoid MD5, to help its dying process. But I think we could probably get a substring from a sha256 hash, and still avoid randomly occuring collisions. I will double-check, and adjust Donor.

[nonbinary]

Ok, here's the math: Our 16 hex digits create roughly 1.8*10¹⁹ possible combinations. We know that the two parts that we're combining are unique, so the hash function itself won't create collisions here. All parts of the hash are equally random, so all we'd do is to increase the risk of randomly occuring collisions. The birthday paradox says that we'll see a rapid increase in randomly occuring collisions after 10⁸ entries. With 6 million entries, we'd have a collision risk (risk that two entries randomly getting the same key) of 10⁻⁶, which I'd say is acceptable. 200K entries have a risk of 10⁻⁹. If we want to completely fail-safe this, we could save one digit. If we'd want to fail-safe this, we could check the unique key on insertion, and add a check digit. It wouldn't affect the collision risk much, and it would future-proof this. So for now: Hash, substring 15 characters, and add a 0.

[hanneskod]

Cool. I love the math! I actually have a different solution.. Using Hashids which isn't really a hash but an integer-base64 encoding process with some extra sugar. Its collision free and variable length. Now if we remove the check digits from our id and account objects (they don't add entropy anyway) the largest possible Id + account number we need to "hash" is 999999999999999999999999 (9 + 15 nines..). And that fits in a 16 char Hashid.

By the way you don't need to patch donor with getTransactionDay. I'm doing it in my pr. Just haven't pushed yet..

[hanneskod]

In this latest commit I solved problem number 1 by adding a DateBuilder. The builder pattern was really helpfull here in moving the need to speficy day of month from MandateApprovedState. This way we can set day-of-month earlier on the setup phase. Probable some work to be done in DonorStateFactory for this to work. But I'm leaving this out for now.. Will open a new issue not to forget..

As for the second problem this commit simply assumes that getMandateKey() returnes a 16 char string. Will open another pr with my suggestion for meeting this asumption. And we can continue the discussion and mathematical reasoning there... =)

[hanneskod]

Pending on your thoughts @nonbinary I feel this is ready to merge..

[nonbinary]

Agree!