Page MenuHomePhorge

Limit profit by wallet balance
Changes PlannedPublic

Authored by mollekopf on Jun 25 2024, 10:53 AM.
Tags
None
Referenced Files
F15414144: D4812.id13782.diff
Thu, Sep 19, 6:38 PM
F15414126: D4812.id13788.diff
Thu, Sep 19, 6:37 PM
F15413886: D4812.id13785.diff
Thu, Sep 19, 6:10 PM
Unknown Object (File)
Tue, Sep 17, 7:40 AM
Unknown Object (File)
Sun, Sep 8, 3:36 AM
Unknown Object (File)
Wed, Aug 21, 8:35 AM
Unknown Object (File)
Aug 20 2024, 3:03 PM
Unknown Object (File)
Aug 1 2024, 1:51 AM
Subscribers

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Prevent a reseller from earning more than they make.

Diff Detail

Repository
rK kolab
Lint
Lint Skipped
Unit
No Test Coverage
Build Status
Buildable 47841
Build 18150: arc lint + arc unit

Event Timeline

mollekopf created this revision.
mollekopf added a reviewer: Restricted Project.Jun 25 2024, 10:53 AM
machniak added inline comments.
src/app/Observers/UserObserver.php
119

The code above needs to go away in any case, no problem here.

src/app/Wallet.php
126

This is wrong. This is a loop over entitlements, You can't use balance on every iteration. It does not make sense.

626

We balanceUpdate() when we charge entitlements, so this code looks like a duplication. It does not make sense. What if user pre-paid 100 CHF so his balance is essentially always positive. I don't understand this approach.

src/app/Wallet.php
126

This is indeed wrong.

626

The only point of this part is to compensate for limiting the profit based on wallet balance. So if the cost of entitlements is 10 and the fee is 3, and the users wallet is at 5; when it goes from 5 to -5, sum up the total cost of 10 and fee of 3, but we first limit the cost by the wallet balance (5) so we end up with 5 and subtract the fee of 3 so the user ends up with a profit of 2. Now once the balance goes above 0 again we want to compensate the reseller for the "lost" profit (for the total of 7) hence we credit the negative balance to the reseller wallet.
This should always work about since we always subtract our fee, and therefore the "top up" amount always belongs to the reseller. If the balance remains in the positive this codepath never triggers, as intended.
Does that make more sense?

Sum up charges and fees first, then limit by wallet balance.

Don't break all existing logic related to cost

src/app/Wallet.php
155

This is not going to fly. What if balance is negative? We'll take that from reseller every time a charge is made. Also note that we do not charge once a month, we charge whenever an entitlement has a full period, e.g. quota might be charged independently from the mailbox entitlement. Not mentioning partial period change on delete. With this we'll be taking too much money from the reseller.

626

Maybe now I understand what you propose, but I think it will not work. What if user balance=-10CHF and pays 10CHF. Reseller would never get 10CHF.
Even if you correct the if constraint to use >= there's another case that will be problematic. Instead user pays only 5 CHF, and then again 5CHF? Reseller will get only 5CHF.
So, probably this should add profit whenever balance is changed in the positive direction (could still be negative).

But then I'd have another question, should awards given to the user be taken into account (become reseller profit) or be excluded?

mollekopf marked an inline comment as done.
mollekopf added inline comments.
src/app/Wallet.php
155

This implementation assumed that we only go into negative once and then degrade the account and any further charging. But yes, it does give us "too much" by charging our full fee while going into negative.