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.

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

Allow deleting members with payments again #3707

Open
DeD1rk opened this issue Jun 3, 2024 · 2 comments
Open

Allow deleting members with payments again #3707

DeD1rk opened this issue Jun 3, 2024 · 2 comments
Assignees
Labels
app:events Issues regarding the events-app app:members Issues regarding the members-app app:moneybirdsynchronization Issues regarding the Moneybird synchronization app:payments Issues regarding the payments-app app:pizzas Issues regarding the pizzas-app board Need to involve the board bug Something that should be fixed priority: high Must be dealt with before the next release is deployed. security Security related issues
Milestone

Comments

@DeD1rk
Copy link
Member

DeD1rk commented Jun 3, 2024

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:

member = models.ForeignKey(
"members.Member",
models.CASCADE,
blank=True,
null=True,
)

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

  1. Try to delete a user.
  2. 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.

@DeD1rk DeD1rk added priority: medium A new feature or a bugfix that is non-critical. board Need to involve the board app:members Issues regarding the members-app bug Something that should be fixed app:payments Issues regarding the payments-app security Security related issues app:moneybirdsynchronization Issues regarding the Moneybird synchronization app:events Issues regarding the events-app app:pizzas Issues regarding the pizzas-app priority: high Must be dealt with before the next release is deployed. and removed priority: medium A new feature or a bugfix that is non-critical. labels Jun 3, 2024
@DeD1rk DeD1rk self-assigned this Jun 5, 2024
@DeD1rk DeD1rk added this to the Release 53.0 milestone Jun 10, 2024
@T8902
Copy link
Contributor

T8902 commented Jun 11, 2024

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
Copy link
Contributor

T8902 commented Jun 11, 2024

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.

@DeD1rk DeD1rk modified the milestones: Release 53.0, Release 54.0 Jun 12, 2024
@DeD1rk DeD1rk removed this from the Release 54.0 milestone Sep 11, 2024
@T8902 T8902 added this to the Release 56 milestone Nov 4, 2024
@T8902 T8902 assigned T8902 and unassigned DeD1rk Nov 20, 2024
@T8902 T8902 modified the milestones: Release 56, Release 57 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:events Issues regarding the events-app app:members Issues regarding the members-app app:moneybirdsynchronization Issues regarding the Moneybird synchronization app:payments Issues regarding the payments-app app:pizzas Issues regarding the pizzas-app board Need to involve the board bug Something that should be fixed priority: high Must be dealt with before the next release is deployed. security Security related issues
Projects
None yet
Development

No branches or pull requests

2 participants