Rewrote some methods to fix #341

app/Repositories/Bill/BillRepository.php

@@ -16,6 +16,7 @@ namespace FireflyIII\Repositories\Bill;
 use Carbon\Carbon;
 use DB;
 use FireflyIII\Models\Bill;
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Support\CacheProperties;
@@ -224,7 +225,8 @@ class BillRepository implements BillRepositoryInterface
 
     /**
      * Get the total amount of money paid for the users active bills in the date range given.
-     * This amount will be negative (they're expenses).
+     * This amount will be negative (they're expenses). This method is equal to
+     * getBillsUnpaidInRange. So the debug comments are gone.
      *
      * @param Carbon $start
      * @param Carbon $end
@@ -233,29 +235,37 @@ class BillRepository implements BillRepositoryInterface
      */
     public function getBillsPaidInRange(Carbon $start, Carbon $end): string
     {
-        $amount = '0';
+        $bills = $this->getActiveBills();
-        $bills  = $this->getActiveBills();
+        $sum   = '0';
-
         /** @var Bill $bill */
         foreach ($bills as $bill) {
-            $ranges = $this->getRanges($bill, $start, $end);
+            $currentStart = clone $start;
-
+            while ($currentStart <= $end) {
-            foreach ($ranges as $range) {
+                $nextExpectedMatch = $this->nextDateMatch($bill, $currentStart);
-                $paid      = $bill->transactionJournals()
+                if ($nextExpectedMatch > $end) {
-                                  ->before($range['end'])
+                    break;
-                                  ->after($range['start'])
+                }
-                                  ->leftJoin(
+                /** @var Collection $set */
-                                      'transactions', function (JoinClause $join) {
+                $set = $bill->transactionJournals()->after($currentStart)->before($nextExpectedMatch)->get(['transaction_journals.*']);
-                                      $join->on('transactions.transaction_journal_id', '=', 'transaction_journals.id')->where('transactions.amount', '<', 0);
+                if ($set->count() > 0) {
-                                  }
+                    $journalIds = $set->pluck('id')->toArray();
-                                  )
+                    $amount     = strval(Transaction::whereIn('transaction_journal_id', $journalIds)->where('amount', '<', 0)->sum('amount'));
-                                  ->first([DB::raw('SUM(transactions.amount) AS sum_amount')]);
+                    $sum        = bcadd($sum, $amount);
-                $sumAmount = $paid->sum_amount ?? '0';
+                    Log::info(
-                $amount    = bcadd($amount, $sumAmount);
+                        sprintf(
+                            'Bill "%s" is PAID in period %s to %s (%d transaction(s)), add %f to sum (sum is now %f).', $bill->name,
+                            $currentStart->format('Y-m-d'),
+                            $nextExpectedMatch->format('Y-m-d'),
+                            $set->count(),
+                            $amount, $sum
+                        )
+                    );
+                }
+                $currentStart = clone $nextExpectedMatch;
             }
         }
 
-        return $amount;
+        return $sum;
     }
 
     /**
@@ -268,32 +278,56 @@ class BillRepository implements BillRepositoryInterface
      */
     public function getBillsUnpaidInRange(Carbon $start, Carbon $end): string
     {
-        $amount = '0';
+        $bills = $this->getActiveBills();
-        $bills  = $this->getActiveBills();
+        $sum   = '0';
-
         /** @var Bill $bill */
         foreach ($bills as $bill) {
-            $ranges   = $this->getRanges($bill, $start, $end);
+            Log::debug(sprintf('Now at bill "%s" (%s)', $bill->name, $bill->repeat_freq));
-            $paidBill = '0';
-            foreach ($ranges as $range) {
-                $paid      = $bill->transactionJournals()
-                                  ->before($range['end'])
-                                  ->after($range['start'])
-                                  ->leftJoin(
-                                      'transactions', function (JoinClause $join) {
-                                      $join->on('transactions.transaction_journal_id', '=', 'transaction_journals.id')->where('transactions.amount', '>', 0);
-                                  }
-                                  )
-                                  ->first([DB::raw('SUM(transactions.amount) AS sum_amount')]);
-                $sumAmount = $paid->sum_amount ?? '0';
-                $paidBill  = bcadd($sumAmount, $paidBill);
-            }
-            if ($paidBill == 0) {
-                $amount = bcadd($amount, $bill->expectedAmount);
-            }
-        }
 
-        return $amount;
+            /*
+             * Start at 2016-10-01, see when we expect the bill to hit:
+             */
+            $currentStart = clone $start;
+            Log::debug(sprintf('First currentstart is %s', $currentStart->format('Y-m-d')));
+
+            while ($currentStart <= $end) {
+                Log::debug(sprintf('Currentstart is now %s.', $currentStart->format('Y-m-d')));
+                $nextExpectedMatch = $this->nextDateMatch($bill, $currentStart);
+                Log::debug(sprintf('next Expected match after %s is %s', $currentStart->format('Y-m-d'), $nextExpectedMatch->format('Y-m-d')));
+                /*
+                 * If $nextExpectedMatch is after $end, we continue:
+                 */
+                if ($nextExpectedMatch > $end) {
+                    Log::debug(sprintf('nextExpectedMatch %s is after %s, so we skip this bill now.', $nextExpectedMatch, $end));
+                    break;
+                }
+                /*
+                 * If it is not, we search for transactions between $currentStart and $nextExpectedMatch
+                 */
+                $count = $bill->transactionJournals()->after($currentStart)->before($nextExpectedMatch)->count();
+                Log::debug(sprintf('%d transactions found', $count));
+
+                if ($count === 0) {
+                    $average = bcdiv(bcadd($bill->amount_max, $bill->amount_min), '2', 4);
+                    $sum     = bcadd($sum, $average);
+                    Log::info(
+                        sprintf(
+                            'Bill "%s" is unpaid in period %s to %s, add %f to sum (sum is now %f).', $bill->name,
+                            $currentStart->format('Y-m-d'),
+                            $nextExpectedMatch->format('Y-m-d'),
+                            $average, $sum
+                        )
+                    );
+                }
+
+                Log::debug(sprintf('Currentstart (%s) has become %s.', $currentStart->format('Y-m-d'), $nextExpectedMatch->format('Y-m-d')));
+                $currentStart = clone $nextExpectedMatch;
+            }
+            Log::debug(sprintf('end of bill "%s"', $bill->name));
+        }
+        Log::debug(sprintf('Sum became %f', $sum));
+
+        return $sum;
     }
 
     /**
@@ -463,6 +497,44 @@ class BillRepository implements BillRepositoryInterface
     }
 
     /**
+     * Given a bill and a date, this method will tell you at which moment this bill expects its next
+     * transaction. Whether or not it is there already, is not relevant.
+     *
+     * @param Bill   $bill
+     * @param Carbon $date
+     *
+     * @return \Carbon\Carbon
+     */
+    public function nextDateMatch(Bill $bill, Carbon $date): Carbon
+    {
+        $cache = new CacheProperties;
+        $cache->addProperty($bill->id);
+        $cache->addProperty('nextDateMatch');
+        $cache->addProperty($date);
+        if ($cache->has()) {
+            return $cache->get();
+        }
+        // find the most recent date for this bill NOT in the future. Cache this date:
+        $start = clone $bill->date;
+        Log::debug('NextDatematch: Start is ' . $start->format('Y-m-d'));
+
+
+        while ($start <= $date) {
+            $start = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
+            Log::debug('NextDateMatch: Start is now ' . $start->format('Y-m-d'));
+        }
+
+        $end = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
+        Log::debug('NextDateMatch: Final start is ' . $start->format('Y-m-d'));
+        Log::debug('NextDateMatch: Matching end is ' . $end->format('Y-m-d'));
+        $cache->store($start);
+
+        return $start;
+    }
+
+    /**
+     * Given the date in $date, this method will return a moment in the future where the bill is expected to be paid.
+     *
      * @param Bill   $bill
      * @param Carbon $date
      *
@@ -473,18 +545,19 @@ class BillRepository implements BillRepositoryInterface
         $cache = new CacheProperties;
         $cache->addProperty($bill->id);
         $cache->addProperty('nextExpectedMatch');
+        $cache->addProperty($date);
         if ($cache->has()) {
             return $cache->get();
         }
         // find the most recent date for this bill NOT in the future. Cache this date:
         $start = clone $bill->date;
-        $now   = new Carbon;
         Log::debug('Start is ' . $start->format('Y-m-d'));
 
-        do {
+
+        while ($start <= $date) {
             $start = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
             Log::debug('Start is now ' . $start->format('Y-m-d'));
-        } while ($start <= $now);
+        }
 
         $end = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
         Log::debug('Final start is ' . $start->format('Y-m-d'));
@@ -495,8 +568,9 @@ class BillRepository implements BillRepositoryInterface
 
         if ($journalCount > 0) {
             // this period had in fact a bill. The new start is the current end, and we create a new end.
+            Log::debug(sprintf('Journal count is %d, so start becomes %s', $journalCount, $end->format('Y-m-d')));
             $start = clone $end;
-            $end   = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
+            //$end   = Navigation::addPeriod($start, $bill->repeat_freq, $bill->skip);
         }
         $cache->store($start);
 

app/Repositories/Bill/BillRepositoryInterface.php

@@ -167,6 +167,17 @@ interface BillRepositoryInterface
     public function lastFoundMatch(Bill $bill): Carbon;
 
 
+    /**
+     * Given a bill and a date, this method will tell you at which moment this bill expects its next
+     * transaction. Whether or not it is there already, is not relevant.
+     *
+     * @param Bill   $bill
+     * @param Carbon $date
+     *
+     * @return \Carbon\Carbon
+     */
+    public function nextDateMatch(Bill $bill, Carbon $date): Carbon;
+
     /**
      * @param Bill   $bill
      * @param Carbon $date

storage/database/seed.bill-test.json

@@ -41,6 +41,59 @@
       "automatch": 1,
       "repeat_freq": "weekly",
       "skip": 0
+    },
+
+
+    {
+      "name": "Rent",
+      "match": "rent,land,lord",
+      "amount_min": 795,
+      "amount_max": 805,
+      "user_id": 1,
+      "date": "2015-01-02",
+      "active": 1,
+      "automatch": 1,
+      "repeat_freq": "monthly",
+      "skip": 0
+    },
+
+    {
+      "name": "Netflix subscription per quarter",
+      "match": "netflix,subscription",
+      "amount_min": 36,
+      "amount_max": 36,
+      "user_id": 1,
+      "date": "2016-09-06",
+      "active": 1,
+      "automatch": 1,
+      "repeat_freq": "quarterly",
+      "skip": 0
+    },
+
+    {
+      "name": "Travel insurance",
+      "match": "abn,travel,insurance",
+      "amount_min": 55,
+      "amount_max": 62,
+      "user_id": 1,
+      "date": "2015-03-04",
+      "active": 1,
+      "automatch": 1,
+      "repeat_freq": "yearly",
+      "skip": 0
+    },
+
+    {
+      "name": "Health insurance",
+      "match": "insurer,insurance,health",
+      "amount_min": 120,
+      "amount_max": 140,
+      "user_id": 1,
+      "date": "2015-01-07",
+      "active": 1,
+      "automatch": 1,
+      "repeat_freq": "monthly",
+      "skip": 0
     }
   ],
   "budgets": [