Allow deleting members with payments again

DeD1rk commented 4 weeks ago

Describe the bug

Since #3417 it's not possible to delete a Member when they've made a payment.

For privacy reasons, the user coudl request being deleted. While we do need to maintain payment history (so the old CASCADE of payments was not correct either), I think we can probably be fine allowing the delete with SET_NULL on Payment.paid_by.

I'm reasonably confident this will work fine:

BankAccounts (including sepa mandates) contain a copy of the real name, and have SET_NULL on they FK to the user. So we still keep any sepa mandate that we're legally required to store.
Payments don't need to have a payer (we do currently payments without a known payer, and can push those to moneybird).

Something we also need to fix is this CASCADE, and the equivalent in FoodOrder: https://github.com/svthalia/concrexit/blob/ad15aba9e660184e10f841a4909e90d47adebdba/website/events/models/event_registration.py#L31-L36 Basically, if we're deleting a user, we shouldn't CASCADE objects that in the normal dataminimisation simply get their FK to Member set to null. This goes for eventregistrations but also FoodOrder.

How to reproduce

Try to delete a user.
It's not allowed because of protected foreign keys.

Expected behaviour

There is a warning that people should be very careful about it, but it is allowed.

T8902 commented 3 weeks ago

Should we not keep the payer for a certain amount of time, thus still have some kind of check on how long ago the last payment by this user was made?

T8902 commented 3 weeks ago

Should we not keep the payer for a certain amount of time, thus still have some kind of check on how long ago the last payment by this user was made?

If I remember correctly, Roel did say something about it having to be linked to a payer for about a year. I might misremember though.

svthalia / concrexit