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

Bug? | Future bills already affect current balance #310

Open
bertwesarg opened this issue Oct 17, 2024 · 8 comments
Open

Bug? | Future bills already affect current balance #310

bertwesarg opened this issue Oct 17, 2024 · 8 comments

Comments

@bertwesarg
Copy link

Cospend 1.6.1

A future bill already affects the balance from today.

  1. Create a new project
  2. Add two users u1 and u2
  3. Create a new bill with 1000 a date in the future payed by u1
  4. Balance already shows 500 for u1 and -500 for u2

This makes it rather complicated to jump start a new project where there are already known repeating bills over the year. With this behavior in place, I would need to wait creating these bills after the first due date, and it still is not clear, if not the next due date already affects the current balance.

@mortee
Copy link

mortee commented Oct 18, 2024

Had this annoyance for a long time, just didn't bother to file a ticket. I guess it could be a checkbox or flip switch to choose whether to exclude future bills from the balance.

@bertwesarg
Copy link
Author

I think this is related to #261. Not sure which one is the cause though. But I have the impression, that the handling of repeated bills is weird. A repeat from a repeated bill is not created on the day of the next due date, but one day later. But to account this next repeated bill already in today‘s balance, the balance needs to also account the repeated bill. And because of the latter, it also accounts for any future bill in today‘s balance.

julien-nc added a commit that referenced this issue Oct 20, 2024
@bertwesarg
Copy link
Author

bertwesarg commented Oct 28, 2024

@julien-nc thanks. I mentioned that its also not comprehensible for me, why repeated bills are not repeated on the day of the next repeat date. While applying your fix for this issue to my local 1.6.1 installation, I also poked around in the cronRepeatBills functions, only to find, that based on the comment and code a repeat bill should already be repeated on the first day of the next repeat day, see here. Based on this, I would expect the following to work:

  1. Create empty project
  2. Create repeating bill with first day today and a repeat of daily
  3. Run cospend:repeat-bills
  4. The repeated bill for today was created and the bills next repeat date is set to tomorrow

But no bill for today was created. What could prevent this from working?

@bertwesarg
Copy link
Author

bertwesarg commented Oct 28, 2024

Got it. Here is patch which is in line with my understanding of repeating bills. It uses the current bill‘s date for the check, not the next date.

diff -urp cospend-nc-1.6.2-fix/lib/Service/ProjectService.php cospend-nc-1.6.2-fix2/lib/Service/ProjectService.php
--- cospend-nc-1.6.2-fix/lib/Service/ProjectService.php 2024-10-28 20:56:33.802921527 +0100
+++ cospend-nc-1.6.2-fix2/lib/Service/ProjectService.php        2024-10-28 23:47:07.433359910 +0100
@@ -2518,8 +2518,8 @@ class ProjectService {
 
                                // Repeat if $nextDate is in the past (or today)
                                $nowTs = $now->getTimestamp();
-                               $nextDateTs = $nextDate->getTimestamp();
-                               if ($nowTs > $nextDateTs || $nextDate->format('Y-m-d') === $now->format('Y-m-d')) {
+                               $billDateTs = $billDate->getTimestamp();
+                               if ($nowTs > $billDateTs || $billDate->format('Y-m-d') === $now->format('Y-m-d')) {
                                        $newBillId = $this->repeatBill($bill['projectid'], $bill['id'], $nextDate);
                                        // bill was not repeated (because of disabled owers or repeatuntil)
                                        if ($newBillId === null) {
diff -urp cospend-nc-1.6.2-fix/lib/Command/RepeatBills.php cospend-nc-1.6.2-fix2/lib/Command/RepeatBills.php
--- cospend-nc-1.6.2-fix/lib/Command/RepeatBills.php    2024-10-15 22:16:01.792586243 +0200
+++ cospend-nc-1.6.2-fix2/lib/Command/RepeatBills.php   2024-10-29 00:22:03.562504986 +0100
@@ -33,7 +33,7 @@ class RepeatBills extends Base {
                foreach ($repeated as $r) {
                        $output->writeln(
                                '[Project "'.$r['project_name'].'"] Bill "'.$r['what'].
-                               '" ('.$r['date_orig'].') repeated on ('.$r['date_repeat'].')'
+                               '" ('.$r['date_orig'].') repeated, next repeat on ('.$r['date_repeat'].')'
                        );
                }
                return 0;

For me, that makes it possible to use "approximation" for repeating bills and adjust the value to the actual value already for the current bill. To be more specific:

  1. Create a repeating bill with an approximated value of $100 for the first day of a month, and let it repeat every month.
  2. The "physical" bill arrives at the 10th of the month. I can now already adjust the value (and if needed also the date) to the repeated bill, not the next future bill
  3. The future bill is unaffected by these edits

Without the above patch/understand, I would need to wait until the first of the next month to adjust the bill for this current month (then past month).

Hope that makes my understanding clear.

@bertwesarg
Copy link
Author

Btw, the balance in the project settlements with no date still accounts future bills

@bertwesarg
Copy link
Author

Btw, the balance in the project settlements with no date still accounts future bills

Possible change:

diff -urp cospend-nc-1.6.2-fix/lib/Service/ProjectService.php cospend-nc-1.6.2-fix2/lib/Service/ProjectService.php
--- cospend-nc-1.6.2-fix/lib/Service/ProjectService.php 2024-10-28 20:56:33.802921527 +0100
+++ cospend-nc-1.6.2-fix2/lib/Service/ProjectService.php        2024-10-29 07:39:16.993083990 +0100
@@ -1104,6 +1104,9 @@ class ProjectService {
         * @return array
         */
        public function getProjectSettlement(string $projectId, ?int $centeredOn = null, ?int $maxTimestamp = null): array {
+               if ($maxTimestamp === null) {
+                       $maxTimestamp = time();
+               }
                $balances = $this->getBalance($projectId, $maxTimestamp);
                if ($centeredOn === null) {
                        $transactions = $this->settle($balances);

@bertwesarg
Copy link
Author

Could it be that MoneyBuster also needs these changes? I see already a non-zero balance if I only have future repeating bills. This does not make sense to me, the server should be the only authoritative source for the current balance.

@julien-nc
Copy link
Owner

@bertwesarg I'm not sure it's a good thing to change how the settlement is computed because there is an optional "settlement date" that can be set in the UI. This is enough IMO. If no date is set, the project is "completely" settled, considering all the bills.

About the balance calculation, there is now an admin settings to choose if only past bills should be considered or all bills. This setting applies to all projects. It is disabled by default (use all bills).

This will be included in the next release which is coming soon.

About the bill repetition, can you create another issue? I think this is unrelated.

Thanks for your patience and perseverance.

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