Ability to change bank account details in user profile #811

Closed thaliawww-service closed 4 years ago

In GitLab by @JobDoesburg on Jan 24, 2019, 14:05

One-sentence description

A field in a users profile to change the bank account details

Motivation

Currently this can only be done by an admin. It would be nice if people could do it themselves as well.

Desired functionality

Less paper work when people want to reimburse costs, as a consequence of Conscribo sync (and it could also be some preparation to the wallet system but it has a practical use now as well).

Suggested implementation

In the public profile, a field 'IBAN', 'BIC' and 'ACCOUNT HOLDER' that people can edit. For IBAN I guess there are libraries to validate IBANs, for BIC maybe there is as well or you could make dict with common BICs.

In GitLab by @se-bastiaan on Jan 24, 2019, 14:09

The fields are already implemented using django-localflavor which provides the validation of the IBANs and for the BICs (#811) based on the ISO specs. The solution is purely exposing the field in the frontend.

The reason the field is currently not user-editable is because they are supposed to be used as value for direct debits together with the initials and direct-debit field. If we make this field publicly editable the value cannot be used for direct-debits anymore.

In GitLab by @JobDoesburg on Jan 24, 2019, 14:51

With Conscribo sync it would be nice to use the IBAN not only for direct debits, but for reimbursements as well (which occurs more often of course). In that case the user-editable field would be desirable. I agree that regarding SEPA debit mandates this is not desirable. At this moment however, this is all checked manually whatsoever since a mandate number and date have to provided to the bank as well so the paperwork is checked manually whatsoever. With digital mandates this problem would not exist anymore, but that is a different topic (which the board will discuss soon™).

Maybe a possibility for 2 different bank accounts, one for reimbursements that is user-editable and a hidden one that we have now for contribution and mandates...? (I don't really like this though since it would be an intermediate thing).

Maybe it is good to wait until we decided something about digital mandates?

In GitLab by @se-bastiaan on Jan 24, 2019, 21:07

Are we really using the values we have saved to do the direct debits? If not then we can safely remove 'use for direct debit' from our list and start using the field as database of account details that can be used for conscribo. If we start accepting digital mandates we just require users to resign a mandate if they change their account details, something we have to do anyways once we get there.

In GitLab by @se-bastiaan on Jan 24, 2019, 21:39

Check #811 about BICs.

In GitLab by @se-bastiaan on Feb 13, 2019, 23:42

@JobDoesburg Do you have an answer for me? Don't know if you got a notification. :)

In GitLab by @JobDoesburg on Feb 14, 2019, 16:29

I recieve now! What would you like an answer to? The BIC is already closed.

In GitLab by @JobDoesburg on Feb 15, 2019, 10:47

Maybe to summarize: The ING bank told us, after many times contacting them, it was no problem if our mandates are digital as long as they are legally valid. So what I would suggest is:

Users can edit their bank account details via the Thalia website. Those will be used by Thalia for reimbursements for example.

People can set on their profile also that they want to pay their contribution via direct debits. If they select that, they need to digitally sign a mandate (the signature path solution looks good, but technically a button would be sufficient). If that is set, every time they change their bank account details, a new mandate has to be created. Those mandates should have a unique number.

Maybe on a longer run this could lead to #632 where a payment batch is generated automatically.

In GitLab by @se-bastiaan on Mar 4, 2019, 19:19

assigned to @se-bastiaan

In GitLab by @se-bastiaan on Mar 6, 2019, 20:12

Split this from the Profile model to a new model in the payments app.
Don't forget to remove this data in data minimisation
Remove signature when mandates are invalid

BankAccount

Initials
Lastname
IBAN
Valid from (default entered on sign)
Valid until
Signature
Mandate number

If signature, initials, lastname or IBAN change: create new model with new mandate number (once it's signed). The model can exist without signature, which means that the validity fields are empty.

In GitLab by @se-bastiaan on Mar 20, 2019, 21:47

unassigned @se-bastiaan

In GitLab by @se-bastiaan on Apr 13, 2019, 13:32

@JobDoesburg Do you ever use the direct debit iban export?

In GitLab by @JobDoesburg on Apr 13, 2019, 17:39

Once a year, when exporting the contribution direct debits. But it still remains some manual work because I need to combine it with amounts and mandate reference numbers (manually). So I only use it to check easily who wants to pay their contribution via direct debit and then I start looking up the mandates itself.

In GitLab by @se-bastiaan on Apr 14, 2019, 18:55

assigned to @se-bastiaan

In GitLab by @se-bastiaan on Apr 27, 2019, 20:38

Current progress:

Screenshot_2019-04-27_Add_bank_account___Finances___Study_Association_Thalia_5_

Screenshot_2019-04-27_Add_bank_account___Finances___Study_Association_Thalia_6_

Screenshot_2019-04-27_Add_bank_account___Finances___Study_Association_Thalia_7_

Screenshot_2019-04-27_Add_bank_account___Finances___Study_Association_Thalia_8_

In GitLab by @JobDoesburg on Apr 27, 2019, 20:51

Suuuuuuuuuper awesome! Only detail: currently ‘reimbursement’ is more often used instead of ‘declaration’

In GitLab by @JobDoesburg on Apr 27, 2019, 20:53

Also it might be Nice to display a mandate reference for the direct debits (since they should exist anyway in the admin)

In GitLab by @se-bastiaan on Apr 27, 2019, 21:08

The reference is already visible in the overview but I'll add it to the form as well. Still need to do test coverage (because the payments module has ~100%). And adjusting the data minimisation and conscribo sync.

I wrote in https://gitlab.science.ru.nl/thalia/concrexit/issues/812#note_68369 that we should remove signatures from revoked mandates but I figured that this is impossible if we want to keep the info for checks by the bank. So we cannot do that.

Maybe we should remove accounts in case a user has more than 1 and the valid until is more than 8 weeks ago? I really don't like adding more cronjobs but I have no better option. Although I cannot imagine that this would really be a problem since most people have the same account their whole study period and all data will be removed when their membership stops.

update: only writing tests left.

In GitLab by @JobDoesburg on May 4, 2019, 15:20

Important to know:

"Machtigingen moet u tot 14 maanden na uitvoering van de laatste incasso bewaren. De machtiging vervalt als u gedurende 36 maanden geen SEPA-incasso doet. Wilt u daarna opnieuw incasseren? Dan moet uw klant een nieuwe machtiging tekenen."

In GitLab by @se-bastiaan on May 4, 2019, 15:24

I thought I had read that somewhere but couldn't find it anymore. Not sure what's best, add a last-used date or maybe even never remove mandates?

In GitLab by @se-bastiaan on May 4, 2019, 15:27

Looking at https://gocardless.com/guides/sepa/managing-mandates/ (https://gocardless.com/nl/handleidingen/sepa/machtigingen-managen/ for Dutch) seems like it should be good if we just save the last-used date and really automatically revoke them to make sure we follow the rules.

In GitLab by @JobDoesburg on May 4, 2019, 15:27

I think the last-used date is best

In GitLab by thijsdejong on May 15, 2019, 20:05

closed via merge request !1241

In GitLab by thijsdejong on May 15, 2019, 20:05

mentioned in commit fd02959bef2a6cb861597ea0cfdaad2272fbb3f2

svthalia / concrexit