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

Ability to remove member from project / set weigth to 0 #1053

Open
Ka55i0peia opened this issue Aug 2, 2022 · 7 comments
Open

Ability to remove member from project / set weigth to 0 #1053

Ka55i0peia opened this issue Aug 2, 2022 · 7 comments

Comments

@Ka55i0peia
Copy link

Ka55i0peia commented Aug 2, 2022

Use case:

You started a project. Bills where created. Later in time you've found out that one is paying for another person. (E.g. mother/father for their children). It make sense to weight the parent by the number of childs and set the child weight to zero.

On the android app MoneyBuster this works like a charm, but you'll get a sync error coming from ihatemoney backend.

Message / Error:

Weight: Weights should be positive

Steps to repoduce:

  1. Make a bill with at least two participants
  2. Edit a particpant weight to 0
  • Make this in MoneyBuster: works as expected, but leads to the error message while syncing. (Project now is out of sync)
  • Make this in ihatemoney (web frontend): Error message getting displayed when editing particpant.

Expected:

Partipant with weight 0 now don't have any open bills. The other partipants now have to "pay more". (Indeed, editing the weight of an participant -- the parent -- "heals" this.)

@zorun
Copy link
Collaborator

zorun commented Aug 3, 2022

See #361 for a discussion about zero weight.

I'm not sure I understand the use-case. You would have to manually edit each bill to replace the child by the parent, right? In that case, you don't need to change the weight, since you have already removed the child from all bills.

Maybe what you want is a quick way a removing a participant from all bills? But this needs more specification: only remove the participant when they are in the for whom? list? Or also when the participant has paid a bill? But in that case, what happens to the bill? There needs to be somebody paying the bill.

@Ka55i0peia
Copy link
Author

Ka55i0peia commented Aug 3, 2022

Hey @zorun ,

thanks for pointing to #361 (-- i don't see any feedback for the reason of the internal server error there -- ) and for providing (in my eyes a workaround) by editing all bills.

The workaround solves the removement of a participant (the "childs"):

  1. Remove the childs from all bills
  2. Optional: Update a participant's weight ("parent" resp. payer for the childs)

But its in my eyes more complicated. My idea was not to update the bills, but setting the child weight to 0 (and increase the "parent" weight accordingly). -- Since this also works in the MoneyBuster App.


I searched a bit in code and found no reason why weights shall not allow to be zero. (Potentially "div by zero" shall be prevented, but I don't see a case this happens, expect all participant have a weight of zero that might be catchable during weight-update-process.)

@almet
Copy link
Member

almet commented Aug 3, 2022

Hey, and thanks for opening this. Cool to see this "weights" feature used more :-)

I believe the logic for having weights strictly superior to zero is, as you mention, to avoid divisions by zero.

With your use case in mind, it seems the weights trick is… a trick.

It seems more logical to increase the weight of the "parent", then remove the person from the project at some point, but that means in that case that all the expenses of the "parent" and the "child" users will be mixed, so be sure that's what you want.

That's being said, I believe having the same behavior in ihatemoney and moneybuster is a requirement, so I believe we should allow zero weights and find a way to solve this.

@Ka55i0peia, are you willing to work on this?

@zorun
Copy link
Collaborator

zorun commented Aug 3, 2022

Ok, if I understand correctly, your bills already had the child and parent listed on each bill.

If we allow zero weights, the most logical behaviour is the following:

  • if the participant with zero weight is listed in "for whom?" in a bill, then they will basically be ignored for this bill (they have a share of zero)
  • however, if the participant with zero weight is the one that paid the bill, then that will still count as a non-zero expense

So, in the end, the balance of this participant will always be positive (or equal to zero if they never paid a bill)

@Ka55i0peia does that make sense for your use-case?

Regarding the implementation, I agree there is a tricky case: what happens when all participants in a bill have zero weight? Either we forbid it (not easy to do at all), or we allow it but it breaks an invariant: the overall balance of all participants will no longer be zero. I have no idea if we use this invariant somewhere.

@Ka55i0peia
Copy link
Author

Ka55i0peia commented Aug 4, 2022

@zorun you are 💯 % correct! Parent and the childs have been initially in the bills. (Interresting possibility I haven't in mind, which doesn't make sense in this use case but can possibly occure: "the participant with zero weight is the one that paid the bill" 🤔 )

The more I am thinking on this, the more setting weigth to zero sounds like a trick/hack for disabling participants. Removing the childs from the bills is more what is intended to correct this initally falsy participant/payer creation. (Thus e.g. bulk editing might support this editing.)

The point that leads me to this ticket is, the some strange behavior when setting weigth to zero in MoneyBuster. There setting weigth to zero works as I expected, but MoneyBuster is not awaiting an error when updating the participants weigth. As a result a sync error occurs while using the app. Thus a bug ticket in the app project makes more sense after think a bit on it.

@almet : Handling zero weighted participants might be a possilophy question. As @zorun pointed to it leads to other corner cases that shall be handled properly (and intuitive).

@almet
Copy link
Member

almet commented Aug 9, 2022

@Ka55i0peia did you open an issue on MoneyBuster ? I can't find it there. We probably should not close this until it's reported there.

@almet almet reopened this Aug 9, 2022
@Ka55i0peia
Copy link
Author

@almet : you are right. I haven't had the time to create one. Good idea to keep this open until a ticket there is linked here. I'll keep this ticket up to date.

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

3 participants