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

Adding invalid reimbursements can cause the debt solver to fail #1295

Open
TomRoussel opened this issue Mar 24, 2024 · 2 comments
Open

Adding invalid reimbursements can cause the debt solver to fail #1295

TomRoussel opened this issue Mar 24, 2024 · 2 comments

Comments

@TomRoussel
Copy link
Contributor

TomRoussel commented Mar 24, 2024

Description

I was playing around with the new bill types when I ran into this problem. When you manually add a reimbursement that is invalid, this error triggers on the settle page:

image

Looks like this is related to reimbursement types with multiple people in the "payed_for" field. This does not happen when you use the "settle" button, as it creates valid reimbursements, but users can also manually add reimbursements. If they do this incorrectly, this completely breaks the settle page.

I think we can fix this by adding a constraint on bill creation that any reimbursement type bills should only have 1 'payed_for' entry.

To reproduce

Because of #1293, you can't reproduce this on the current master (as of commit 4af4c10). You can reproduce this issue using 483b94c.
Steps to reproduce:

  • Open demo project
  • Add a bill with type "reimbursement" with any amount and multiple participants
  • Open settle page
@zorun
Copy link
Collaborator

zorun commented Mar 29, 2024

Indeed, and statistics are also off in that case.

Alternatively, we could fix all places doing computation so that they take into account multiple beneficiaries. It's not hard, just reusing the same code as expense bill should do the trick (and that would factor code which is good).

But as you said, it doesn't really make sense to settle to multiple people at once.

I suggest doing both so that we are robust: fix computation, but also prevent creation of settlements with multiple beneficiairies. Also, tests would be good.

@zorun
Copy link
Collaborator

zorun commented Mar 29, 2024

The "fix computation" part is in #1300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants