Update the unpaid invoice report based on the issue #6184 #6196
Conversation
There was a problem hiding this comment.
It looks like you have addressed all the items in the issue. Looks good!
A few minor issues
- The currency should default to the enterprise currency. See client/src/modules/reports/generate/avg_med_costs_per_patient/avg_med_costs_per_patient.js for an example -- especially in checkCachedConfiguration().
- When you chose a debtor Group and put its name as subtitle, I suggest using a little larger font.
- When you select a Service, there is no need to display a separate column for it since the totals column is the same (since you are only showing one service). However, you also need to add a subtitle for the service when you remove its column.
| @@ -24,6 +24,10 @@ function UnbalancedInvoicePaymentsConfigController($sce, Notify, SavedReports, A | |||
| vm.reportDetails = angular.copy(report); | |||
| }; | |||
|
|
|||
| vm.onSelectCurrency = currency => { | |||
| vm.reportDetails.currencyId = currency.id; | |||
There was a problem hiding this comment.
Could you make this default to the enterprise currency?
| @@ -66,6 +66,7 @@ async function getUnbalancedInvoices(options) { | |||
| const params = [ | |||
| new Date(options.dateFrom), | |||
| new Date(options.dateTo), | |||
| +options.currencyId, | |||
There was a problem hiding this comment.
This is cool, but I really worry about this being NaN. What happens then? What does MySQL convert it to?
It would be much better to convert this explicitly to something.
|
@mbayopanda if you can get these issues sorted out, we can try and include this report in the next release. I would like to do a release this Friday. |
mbayopanda
force-pushed
the
update-unpaid-invoice-report
branch
from
c553dc9
to
acbb840
Compare
|
@jmcameron @jniles changes landed |
| const vm = this; | ||
| const cache = new AppCache('configure_unpaid_invoice_payments'); | ||
| const reportUrl = 'reports/finance/unpaid_invoice_payments'; | ||
|
|
||
| vm.previewGenerated = false; | ||
| vm.reportDetails = {}; | ||
| vm.reportDetails = { | ||
| currencyId : Session.enterprise.currency_id, |
| @@ -980,11 +984,13 @@ CREATE PROCEDURE UnbalancedInvoicePaymentsTable( | |||
| CREATE TEMPORARY TABLE `unbalanced_invoices` AS ( | |||
| SELECT BUID(ivc.uuid) as invoice_uuid , em.text AS debtorReference, debtor.text AS debtorName, | |||
| BUID(debtor.uuid) as debtorUuid, | |||
| balances.debit_equiv AS debit, | |||
| balances.credit_equiv AS credit, iv.date AS creation_date, balances.balance, | |||
| (balances.debit_equiv * IFNULL(GetExchangeRate(_enterpriseId, currencyId, dateTo), 1)) AS debit, | |||
There was a problem hiding this comment.
It looks like you can speed this up a lot by moving the GetExchangeRate(_enterpriseId, currencyId, dateTo) out of the SQL query. Like this:
DECLARE exchangeRate DOUBLE;
SET exchangeRate = (SELECT IFNULL(GetExchangeRate(_enterpriseId, currencyId, dateTo), 1));
CREATE TEMPORARY TABLE `unbalanced_invoices` AS (
SELECT BUID(ivc.uuid) as invoice_uuid , em.text AS debtorReference, debtor.text AS debtorName,
BUID(debtor.uuid) as debtorUuid,
(balances.debit_equiv * exchangeRate) AS debit,
/* etc ... */This has the advantage of speeding up the query since the GetExchangeRate() function doesn't depend on the data in the table. Instead of calling it 3X per row, we only call it a single time.
| {{date dateFrom "DD/MM/YYYY"}} - {{date dateTo "DD/MM/YYYY"}} | ||
| </h5> | ||
| </h4> |
There was a problem hiding this comment.
If the currency is not the enterprise currency, we should show a warning that this is only temporary using this warning:
| @@ -976,15 +981,19 @@ CREATE PROCEDURE UnbalancedInvoicePaymentsTable( | |||
| -- even though this column is called "balance", it is actually the amount remaining | |||
| -- on the invoice. | |||
|
|
|||
| SET exchangeRate = (SELECT IFNULL(GetExchangeRate(_enterpriseId, currencyId, dateTo), 1)); | |||
|
Build succeeded: |
This PR is related to #6184
Close #6184