From 25f5cdbcdd225e6a0d382fb7b6775343fe9d7d68 Mon Sep 17 00:00:00 2001 From: Hillel Coren Date: Sun, 31 Jan 2016 15:10:33 +0200 Subject: [PATCH] Added check to ensure expense currency matches client\invoice currency --- app/Http/Controllers/ExpenseController.php | 27 +++++++++++++------- app/Http/Controllers/InvoiceController.php | 1 + app/Models/Expense.php | 5 ++++ app/Ninja/Repositories/ExpenseRepository.php | 2 +- app/Services/ExpenseService.php | 8 +++--- public/js/built.js | 1 + public/js/script.js | 2 ++ resources/lang/en/texts.php | 2 ++ resources/views/invoices/edit.blade.php | 11 ++++++++ resources/views/invoices/knockout.blade.php | 1 + resources/views/invoices/pdf.blade.php | 2 +- 11 files changed, 48 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/ExpenseController.php b/app/Http/Controllers/ExpenseController.php index 5520478d52..bb2a81fbba 100644 --- a/app/Http/Controllers/ExpenseController.php +++ b/app/Http/Controllers/ExpenseController.php @@ -182,22 +182,30 @@ class ExpenseController extends BaseController switch($action) { case 'invoice': - $expenses = Expense::scope($ids)->get(); - $clientId = null; - $data = []; + $expenses = Expense::scope($ids)->with('client')->get(); + $clientPublicId = null; + $currencyId = null; + $data = []; // Validate that either all expenses do not have a client or if there is a client, it is the same client foreach ($expenses as $expense) { - if ($expense->client_id) { - if (!$clientId) { - $clientId = $expense->client_id; - } elseif ($clientId != $expense->client_id) { + if ($expense->client) { + if (!$clientPublicId) { + $clientPublicId = $expense->client->public_id; + } elseif ($clientPublicId != $expense->client->public_id) { Session::flash('error', trans('texts.expense_error_multiple_clients')); return Redirect::to('expenses'); } } + if (!$currencyId) { + $currencyId = $expense->getCurrencyId(); + } elseif ($currencyId != $expense->getCurrencyId() && $expense->getCurrencyId()) { + Session::flash('error', trans('texts.expense_error_multiple_currencies')); + return Redirect::to('expenses'); + } + if ($expense->invoice_id) { Session::flash('error', trans('texts.expense_error_invoiced')); return Redirect::to('expenses'); @@ -212,8 +220,9 @@ class ExpenseController extends BaseController ]; } - $clientPublicId = $clientId ? Client::findOrFail($clientId)->public_id : ''; - return Redirect::to("invoices/create/{$clientPublicId}")->with('expenses', $data); + return Redirect::to("invoices/create/{$clientPublicId}") + ->with('expenseCurrencyId', $currencyId) + ->with('expenses', $data); break; default: diff --git a/app/Http/Controllers/InvoiceController.php b/app/Http/Controllers/InvoiceController.php index 9ef2eed164..bb124c07c4 100644 --- a/app/Http/Controllers/InvoiceController.php +++ b/app/Http/Controllers/InvoiceController.php @@ -320,6 +320,7 @@ class InvoiceController extends BaseController 'invoiceLabels' => Auth::user()->account->getInvoiceLabels(), 'tasks' => Session::get('tasks') ? json_encode(Session::get('tasks')) : null, 'expenses' => Session::get('expenses') ? json_encode(Session::get('expenses')) : null, + 'expenseCurrencyId' => Session::get('expenseCurrencyId') ?: null, ]; } diff --git a/app/Models/Expense.php b/app/Models/Expense.php index 4251a85768..b4e1060052 100644 --- a/app/Models/Expense.php +++ b/app/Models/Expense.php @@ -60,6 +60,11 @@ class Expense extends EntityModel return $this->public_id; } + public function getCurrencyId() + { + return $this->client ? $this->client->currency_id : $this->currency_id; + } + public function getDisplayName() { return $this->getName(); diff --git a/app/Ninja/Repositories/ExpenseRepository.php b/app/Ninja/Repositories/ExpenseRepository.php index aa07097181..fd2f78afc7 100644 --- a/app/Ninja/Repositories/ExpenseRepository.php +++ b/app/Ninja/Repositories/ExpenseRepository.php @@ -64,9 +64,9 @@ class ExpenseRepository extends BaseRepository ->orWhere('contacts.is_primary', '=', null); }) ->select( + DB::raw('COALESCE(clients.currency_id, expenses.currency_id, accounts.currency_id) currency_id'), 'expenses.account_id', 'expenses.amount', - 'expenses.currency_id', 'expenses.deleted_at', 'expenses.exchange_rate', 'expenses.expense_date', diff --git a/app/Services/ExpenseService.php b/app/Services/ExpenseService.php index 9500ba65ee..874c17ea71 100644 --- a/app/Services/ExpenseService.php +++ b/app/Services/ExpenseService.php @@ -90,11 +90,13 @@ class ExpenseService extends BaseService 'amount', function ($model) { // show both the amount and the converted amount - $str = Utils::formatMoney($model->amount, $model->account_currency_id, $model->account_country_id); if ($model->exchange_rate != 1) { - $str .= ' | ' . Utils::formatMoney(round($model->amount * $model->exchange_rate,2), $model->currency_id, $model->client_country_id); + $converted = round($model->amount * $model->exchange_rate, 2); + return Utils::formatMoney($model->amount, $model->account_currency_id, $model->account_country_id) . ' | ' . + Utils::formatMoney($converted, $model->currency_id, $model->client_country_id); + } else { + return Utils::formatMoney($model->amount, $model->currency_id, $model->account_country_id); } - return $str; } ], [ diff --git a/public/js/built.js b/public/js/built.js index aa8e5120e0..7354679dc6 100644 --- a/public/js/built.js +++ b/public/js/built.js @@ -29883,6 +29883,7 @@ var isSafari = Object.prototype.toString.call(window.HTMLElement).indexOf('Const var isEdge = navigator.userAgent.indexOf('Edge/') >= 0; var isChrome = !!window.chrome && !isOpera && !isEdge; // Chrome 1+ var isChromium = isChrome && navigator.userAgent.indexOf('Chromium') >= 0; +var isChrome48 = isChrome && navigator.userAgent.indexOf('Chrome/48') >= 0; var isIE = /*@cc_on!@*/false || !!document.documentMode; // At least IE6 var refreshTimer; diff --git a/public/js/script.js b/public/js/script.js index f1d3287930..e1feaefd8f 100644 --- a/public/js/script.js +++ b/public/js/script.js @@ -5,6 +5,8 @@ var isSafari = Object.prototype.toString.call(window.HTMLElement).indexOf('Const var isEdge = navigator.userAgent.indexOf('Edge/') >= 0; var isChrome = !!window.chrome && !isOpera && !isEdge; // Chrome 1+ var isChromium = isChrome && navigator.userAgent.indexOf('Chromium') >= 0; +// https://code.google.com/p/chromium/issues/detail?id=574648 +var isChrome48 = isChrome && navigator.userAgent.indexOf('Chrome/48') >= 0; var isIE = /*@cc_on!@*/false || !!document.documentMode; // At least IE6 var refreshTimer; diff --git a/resources/lang/en/texts.php b/resources/lang/en/texts.php index 31372724b3..56cada46c7 100644 --- a/resources/lang/en/texts.php +++ b/resources/lang/en/texts.php @@ -1131,4 +1131,6 @@ return array( 'imported_expenses' => 'Successfully created :count_vendors vendor(s) and :count_expenses expense(s)', 'iframe_url_help3' => 'Note: if you plan on accepting credit cards we strongly recommend having HTTPS enabled on your site.', + 'expense_error_multiple_currencies' => 'The expenses can\'t have different currencies.', + 'expense_error_mismatch_currencies' => 'The client\'s currency does not match the expense currency.', ); diff --git a/resources/views/invoices/edit.blade.php b/resources/views/invoices/edit.blade.php index 2a18faac51..8bc559bd8e 100644 --- a/resources/views/invoices/edit.blade.php +++ b/resources/views/invoices/edit.blade.php @@ -745,6 +745,8 @@ @endif @if (isset($expenses) && $expenses) + model.expense_currency_id({{ $expenseCurrencyId }}); + // move the blank invoice line item to the end var blank = model.invoice().invoice_items.pop(); var expenses = {!! $expenses !!}; @@ -1109,6 +1111,15 @@ model.showClientForm(); return; } + + // check currency matches for expenses + var expenseCurrencyId = model.expense_currency_id(); + var clientCurrencyId = model.invoice().client().currency_id() || {{ $account->getCurrencyId() }}; + if (expenseCurrencyId != clientCurrencyId) { + alert("{!! trans('texts.expense_error_mismatch_currencies') !!}"); + return; + } + onPartialChange(true); $('#action').val(value); $('#submitButton').click(); diff --git a/resources/views/invoices/knockout.blade.php b/resources/views/invoices/knockout.blade.php index 34b86eec20..1ba64a8f28 100644 --- a/resources/views/invoices/knockout.blade.php +++ b/resources/views/invoices/knockout.blade.php @@ -6,6 +6,7 @@ function ViewModel(data) { //self.invoice = data ? false : new InvoiceModel(); self.invoice = ko.observable(data ? false : new InvoiceModel()); + self.expense_currency_id = ko.observable(); self.tax_rates = ko.observableArray(); self.tax_rates.push(new TaxRateModel()); // add blank row diff --git a/resources/views/invoices/pdf.blade.php b/resources/views/invoices/pdf.blade.php index eba6f44531..f185c9df06 100644 --- a/resources/views/invoices/pdf.blade.php +++ b/resources/views/invoices/pdf.blade.php @@ -98,7 +98,7 @@ function refreshPDFCB(string) { if (!string) return; PDFJS.workerSrc = '{{ asset('js/pdf_viewer.worker.js') }}'; - if ({{ Auth::check() && Auth::user()->force_pdfjs ? 'false' : 'true' }} && (isFirefox || isChrome)) { + if ({{ Auth::check() && Auth::user()->force_pdfjs ? 'false' : 'true' }} && (isFirefox || (isChrome && !isChrome48))) { $('#theFrame').attr('src', string).show(); } else { if (isRefreshing) {