From 17b6cc43d5e65bb172bbb644c15bec5fffca6107 Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 5 Jun 2017 22:11:54 +0200 Subject: [PATCH] Fix display of foreign currencies in charts. --- app/Http/Controllers/AccountController.php | 4 +- .../Controllers/Chart/AccountController.php | 12 ++- .../Controllers/Chart/ReportController.php | 3 +- app/Repositories/Account/AccountTasker.php | 5 +- app/Support/Steam.php | 84 ++++++++++++------- .../Controllers/AccountControllerTest.php | 2 +- .../Chart/AccountControllerTest.php | 4 +- .../Chart/ReportControllerTest.php | 3 +- 8 files changed, 67 insertions(+), 50 deletions(-) diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index 362b2fd14b..16cac5c261 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -219,8 +219,8 @@ class AccountController extends Controller $start->subDay(); $ids = $accounts->pluck('id')->toArray(); - $startBalances = Steam::balancesById($ids, $start); - $endBalances = Steam::balancesById($ids, $end); + $startBalances = Steam::balancesByAccounts($accounts, $start); + $endBalances = Steam::balancesByAccounts($accounts, $end); $activities = Steam::getLastActivities($ids); $accounts->each( diff --git a/app/Http/Controllers/Chart/AccountController.php b/app/Http/Controllers/Chart/AccountController.php index 2a48a2876d..81fcf35c81 100644 --- a/app/Http/Controllers/Chart/AccountController.php +++ b/app/Http/Controllers/Chart/AccountController.php @@ -114,9 +114,8 @@ class AccountController extends Controller $start->subDay(); $accounts = $repository->getAccountsByType([AccountType::EXPENSE, AccountType::BENEFICIARY]); - $ids = $accounts->pluck('id')->toArray(); - $startBalances = Steam::balancesById($ids, $start); - $endBalances = Steam::balancesById($ids, $end); + $startBalances = Steam::balancesByAccounts($accounts, $start); + $endBalances = Steam::balancesByAccounts($accounts, $end); $chartData = []; foreach ($accounts as $account) { @@ -350,7 +349,7 @@ class AccountController extends Controller $cache->addProperty('chart.account.period'); $cache->addProperty($account->id); if ($cache->has()) { - return Response::json($cache->get()); // @codeCoverageIgnore + //return Response::json($cache->get()); // @codeCoverageIgnore } $format = (string)trans('config.month_and_day'); @@ -410,9 +409,8 @@ class AccountController extends Controller $accounts = $repository->getAccountsByType([AccountType::REVENUE]); $start->subDay(); - $ids = $accounts->pluck('id')->toArray(); - $startBalances = Steam::balancesById($ids, $start); - $endBalances = Steam::balancesById($ids, $end); + $startBalances = Steam::balancesByAccounts($accounts, $start); + $endBalances = Steam::balancesByAccounts($accounts, $end); foreach ($accounts as $account) { $id = $account->id; diff --git a/app/Http/Controllers/Chart/ReportController.php b/app/Http/Controllers/Chart/ReportController.php index 3310a0b64b..7df9b25e58 100644 --- a/app/Http/Controllers/Chart/ReportController.php +++ b/app/Http/Controllers/Chart/ReportController.php @@ -67,11 +67,10 @@ class ReportController extends Controller if ($cache->has()) { return Response::json($cache->get()); // @codeCoverageIgnore } - $ids = $accounts->pluck('id')->toArray(); $current = clone $start; $chartData = []; while ($current < $end) { - $balances = Steam::balancesById($ids, $current); + $balances = Steam::balancesByAccounts($accounts, $current); $sum = $this->arraySum($balances); $label = $current->formatLocalized(strval(trans('config.month_and_day'))); $chartData[$label] = $sum; diff --git a/app/Repositories/Account/AccountTasker.php b/app/Repositories/Account/AccountTasker.php index ce2e437880..3759522c41 100644 --- a/app/Repositories/Account/AccountTasker.php +++ b/app/Repositories/Account/AccountTasker.php @@ -41,11 +41,10 @@ class AccountTasker implements AccountTaskerInterface */ public function getAccountReport(Collection $accounts, Carbon $start, Carbon $end): array { - $ids = $accounts->pluck('id')->toArray(); $yesterday = clone $start; $yesterday->subDay(); - $startSet = Steam::balancesById($ids, $yesterday); - $endSet = Steam::balancesById($ids, $end); + $startSet = Steam::balancesByAccounts($accounts, $yesterday); + $endSet = Steam::balancesByAccounts($accounts, $end); Log::debug('Start of accountreport'); diff --git a/app/Support/Steam.php b/app/Support/Steam.php index 0572693961..e97b0f2c03 100644 --- a/app/Support/Steam.php +++ b/app/Support/Steam.php @@ -18,6 +18,7 @@ use Crypt; use DB; use FireflyIII\Models\Account; use FireflyIII\Models\Transaction; +use Illuminate\Support\Collection; /** * Class Steam @@ -107,7 +108,7 @@ class Steam ->where('transactions.foreign_currency_id', $currencyId) ->sum('transactions.foreign_amount') ); - $balance = bcadd($nativeBalance, $foreignBalance); + $balance = bcadd($nativeBalance, $foreignBalance); $cache->store($balance); @@ -134,30 +135,59 @@ class Steam $cache->addProperty($start); $cache->addProperty($end); if ($cache->has()) { - return $cache->get(); // @codeCoverageIgnore + //return $cache->get(); // @codeCoverageIgnore } - $balances = []; $start->subDay(); $end->addDay(); - $startBalance = $this->balance($account, $start); - $balances[$start->format('Y-m-d')] = $startBalance; + $balances = []; + $formatted = $start->format('Y-m-d'); + $startBalance = $this->balance($account, $start); + $balances[$formatted] = $startBalance; + $currencyId = intval($account->getMeta('currency_id')); $start->addDay(); // query! - $set = $account->transactions() - ->leftJoin('transaction_journals', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->where('transaction_journals.date', '>=', $start->format('Y-m-d')) - ->where('transaction_journals.date', '<=', $end->format('Y-m-d')) - ->groupBy('transaction_journals.date') - ->orderBy('transaction_journals.date', 'ASC') - ->whereNull('transaction_journals.deleted_at') - ->get(['transaction_journals.date', DB::raw('SUM(transactions.amount) AS modified')]); + $set = $account->transactions() + ->leftJoin('transaction_journals', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->where('transaction_journals.date', '>=', $start->format('Y-m-d')) + ->where('transaction_journals.date', '<=', $end->format('Y-m-d')) + ->groupBy('transaction_journals.date') + ->groupBy('transactions.transaction_currency_id') + ->groupBy('transactions.foreign_currency_id') + ->orderBy('transaction_journals.date', 'ASC') + ->whereNull('transaction_journals.deleted_at') + ->get( + [ + 'transaction_journals.date', + 'transactions.transaction_currency_id', + DB::raw('SUM(transactions.amount) AS modified'), + 'transactions.foreign_currency_id', + DB::raw('SUM(transactions.foreign_amount) AS modified_foreign'), + ] + ); + +// echo '
';
+//        var_dump($set->toArray());
+//        exit;
+
         $currentBalance = $startBalance;
         /** @var Transaction $entry */
         foreach ($set as $entry) {
+            // normal amount and foreign amount
             $modified        = is_null($entry->modified) ? '0' : strval($entry->modified);
-            $currentBalance  = bcadd($currentBalance, $modified);
+            $foreignModified = is_null($entry->modified_foreign) ? '0' : strval($entry->modified_foreign);
+            $amount          = '0';
+            if ($currencyId === $entry->transaction_currency_id) {
+                // use normal amount:
+                $amount = $modified;
+            }
+            if ($currencyId === $entry->foreign_currency_id) {
+                // use normal amount:
+                $amount = $foreignModified;
+            }
+
+            $currentBalance  = bcadd($currentBalance, $amount);
             $carbon          = new Carbon($entry->date);
             $date            = $carbon->format('Y-m-d');
             $balances[$date] = $currentBalance;
@@ -173,38 +203,30 @@ class Steam
     /**
      * This method always ignores the virtual balance.
      *
-     * @param array          $ids
-     * @param \Carbon\Carbon $date
+     * @param \Illuminate\Support\Collection $accounts
+     * @param \Carbon\Carbon                 $date
      *
      * @return array
      */
-    public function balancesById(array $ids, Carbon $date): array
+    public function balancesByAccounts(Collection $accounts, Carbon $date): array
     {
-
+        $ids = $accounts->pluck('id')->toArray();
         // cache this property.
         $cache = new CacheProperties;
         $cache->addProperty($ids);
         $cache->addProperty('balances');
         $cache->addProperty($date);
         if ($cache->has()) {
-            return $cache->get(); // @codeCoverageIgnore
+            //return $cache->get(); // @codeCoverageIgnore
         }
 
-        $balances = Transaction::leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
-                               ->where('transaction_journals.date', '<=', $date->format('Y-m-d'))
-                               ->groupBy('transactions.account_id')
-                               ->whereIn('transactions.account_id', $ids)
-                               ->whereNull('transaction_journals.deleted_at')
-                               ->get(['transactions.account_id', DB::raw('sum(transactions.amount) AS aggregate')]);
-
+        // need to do this per account.
         $result = [];
-        foreach ($balances as $entry) {
-            $accountId          = intval($entry->account_id);
-            $balance            = $entry->aggregate;
-            $result[$accountId] = $balance;
+        /** @var Account $account */
+        foreach ($accounts as $account) {
+            $result[$account->id] = $this->balance($account, $date);
         }
 
-
         $cache->store($result);
 
         return $result;
diff --git a/tests/Feature/Controllers/AccountControllerTest.php b/tests/Feature/Controllers/AccountControllerTest.php
index d80c7d0917..fd68a3480f 100644
--- a/tests/Feature/Controllers/AccountControllerTest.php
+++ b/tests/Feature/Controllers/AccountControllerTest.php
@@ -132,7 +132,7 @@ class AccountControllerTest extends TestCase
         $journalRepos = $this->mock(JournalRepositoryInterface::class);
         $repository->shouldReceive('getAccountsByType')->andReturn(new Collection([$account]));
         $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal);
-        Steam::shouldReceive('balancesById')->andReturn([$account->id => '100']);
+        Steam::shouldReceive('balancesByAccounts')->andReturn([$account->id => '100']);
         Steam::shouldReceive('getLastActivities')->andReturn([]);
 
         $this->be($this->user());
diff --git a/tests/Feature/Controllers/Chart/AccountControllerTest.php b/tests/Feature/Controllers/Chart/AccountControllerTest.php
index 25a766e2c7..c5c73f8c6c 100644
--- a/tests/Feature/Controllers/Chart/AccountControllerTest.php
+++ b/tests/Feature/Controllers/Chart/AccountControllerTest.php
@@ -68,7 +68,7 @@ class AccountControllerTest extends TestCase
 
         $accountRepos->shouldReceive('getAccountsByType')->withArgs([[AccountType::EXPENSE, AccountType::BENEFICIARY]])->andReturn(new Collection([$account]));
         $generator->shouldReceive('singleSet')->andReturn([]);
-        Steam::shouldReceive('balancesById')->twice()->andReturn([]);
+        Steam::shouldReceive('balancesByAccounts')->twice()->andReturn([]);
 
 
         $this->be($this->user());
@@ -333,7 +333,7 @@ class AccountControllerTest extends TestCase
 
         $accountRepos->shouldReceive('getAccountsByType')->withArgs([[AccountType::REVENUE]])->andReturn(new Collection([$account]));
         $generator->shouldReceive('singleSet')->andReturn([]);
-        Steam::shouldReceive('balancesById')->twice()->andReturn([]);
+        Steam::shouldReceive('balancesByAccounts')->twice()->andReturn([]);
 
         $this->be($this->user());
         $this->changeDateRange($this->user(), $range);
diff --git a/tests/Feature/Controllers/Chart/ReportControllerTest.php b/tests/Feature/Controllers/Chart/ReportControllerTest.php
index 7e1a3909c3..e896005f78 100644
--- a/tests/Feature/Controllers/Chart/ReportControllerTest.php
+++ b/tests/Feature/Controllers/Chart/ReportControllerTest.php
@@ -33,9 +33,8 @@ class ReportControllerTest extends TestCase
     public function testNetWorth()
     {
         $generator = $this->mock(GeneratorInterface::class);
-        $tasker    = $this->mock(AccountTaskerInterface::class);
 
-        Steam::shouldReceive('balancesById')->andReturn(['5', '10']);
+        Steam::shouldReceive('balancesByAccounts')->andReturn(['5', '10']);
         $generator->shouldReceive('singleSet')->andReturn([]);
 
         $this->be($this->user());