Bug 6934 - New report Cash Register Statistics
Summary: New report Cash Register Statistics
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P3 enhancement (vote)
Assignee: Bouzid Fergani
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 17635 18734 19061 19551
  Show dependency treegraph
 
Reported: 2011-09-28 21:43 UTC by Maxime Pelletier
Modified: 2018-06-04 20:18 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
new report (17.14 KB, patch)
2011-09-28 21:43 UTC, Maxime Pelletier
Details | Diff | Splinter Review
screenshot of the report (50.00 KB, image/png)
2011-09-28 21:43 UTC, Maxime Pelletier
Details
few problems (57.90 KB, image/png)
2011-09-29 14:04 UTC, Nicole C. Engard
Details
New patch with date format fixes and better indentation (21.29 KB, patch)
2011-09-29 14:59 UTC, Maxime Pelletier
Details | Diff | Splinter Review
New patch that fixes problems (16.42 KB, patch)
2012-04-30 20:41 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Patch with 4 spaces (21.16 KB, patch)
2012-05-04 15:06 UTC, Maxime Pelletier
Details | Diff | Splinter Review
New patch rebased on 3.8.5 (22.21 KB, patch)
2012-09-28 18:08 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Calendar icon missing (21.88 KB, image/jpeg)
2012-10-03 13:33 UTC, Marc Véron
Details
Bug 6934 New report Cash Register Statistics (22.42 KB, patch)
2014-08-27 18:37 UTC, simith.doliveira
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6934 New report Cash Register Statistics (22.49 KB, text/plain)
2015-03-04 12:59 UTC, Nicole C. Engard
Details
Bug 6934 New report Cash Register Statistics (22.48 KB, patch)
2015-09-15 15:20 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 6934 - Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (7.71 KB, patch)
2015-09-15 15:20 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 6934 - Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.14 KB, patch)
2015-11-13 15:23 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 6934 - New features, note added to transaction information, total caculated for negative types of transaction (6.06 KB, patch)
2015-11-13 16:50 UTC, Genevieve Plantin
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: New report Cash Register Statistics (22.45 KB, patch)
2015-12-24 02:08 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.21 KB, text/plain)
2015-12-24 02:08 UTC, Héctor Eduardo Castro Avalos
Details
[SIGNED-OFF]Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction (6.14 KB, text/plain)
2015-12-24 02:08 UTC, Héctor Eduardo Castro Avalos
Details
Bug 6934 - QA Follow-up (11.04 KB, patch)
2016-03-03 16:33 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Cash register image (150.48 KB, image/png)
2016-03-03 17:23 UTC, Héctor Eduardo Castro Avalos
Details
Bug 6934 Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.15 KB, patch)
2016-03-04 20:14 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
bug 6934 - second fix to calendars (1.06 KB, patch)
2016-03-04 21:13 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: QA Follow-up (11.11 KB, text/plain)
2016-03-04 21:31 UTC, Héctor Eduardo Castro Avalos
Details
[SIGNED-OFF]Bug 6934 Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.22 KB, text/plain)
2016-03-04 21:31 UTC, Héctor Eduardo Castro Avalos
Details
[SIGNED-OFF]Bug 6934: second fix to calendars (1.13 KB, text/plain)
2016-03-04 21:32 UTC, Héctor Eduardo Castro Avalos
Details
Bug 6934: fixed encoding issue (873 bytes, patch)
2016-04-19 16:12 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: fixed encoding issue (969 bytes, text/plain)
2016-04-19 19:03 UTC, Héctor Eduardo Castro Avalos
Details
Bug 6934: QA follow-up (3.54 KB, patch)
2016-04-28 18:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.51 KB, patch)
2016-07-19 16:07 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.05 KB, patch)
2016-07-19 18:58 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934 New report Cash Register Statistics (23.44 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.20 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction (6.13 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: QA Follow-up (11.10 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934 Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.21 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: second fix to calendars (1.11 KB, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: fixed encoding issue (951 bytes, patch)
2016-07-21 14:57 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: QA follow-up (3.54 KB, patch)
2016-07-21 14:58 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.51 KB, patch)
2016-07-21 14:58 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.05 KB, patch)
2016-07-21 14:58 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.05 KB, patch)
2016-07-21 15:13 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6934 New report Cash Register Statistics (23.44 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.20 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction (6.13 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: QA Follow-up (11.10 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934 Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.21 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: second fix to calendars (1.11 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: fixed encoding issue (951 bytes, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: QA follow-up (3.54 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.51 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.05 KB, patch)
2016-07-21 15:34 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 6934: Fix the href for datables in link rel stylesheet (1.31 KB, patch)
2016-07-22 15:13 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: QA follow-up (3.61 KB, patch)
2016-07-22 15:39 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.58 KB, patch)
2016-07-22 15:39 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.13 KB, patch)
2016-07-22 15:39 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934: Fix the href for datables in link rel stylesheet (1.38 KB, patch)
2016-07-22 15:39 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug6934 - fix errors pointed out by the QA script (3.85 KB, patch)
2016-07-28 13:39 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[SIGNED-OFF]Bug6934: fix errors pointed out by the QA script (4.17 KB, patch)
2016-07-28 15:35 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug6934: rename active transaction with All payments to the library and grouping in it the relevant account types 'Pay' 'C' (4.20 KB, patch)
2016-08-04 19:05 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: New report Cash Register Statistics (23.51 KB, patch)
2016-08-07 08:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.26 KB, patch)
2016-08-07 08:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction (6.20 KB, patch)
2016-08-07 08:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: QA Follow-up (11.17 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.28 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: second fix to calendars (1.18 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fixed encoding issue (1021 bytes, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: QA follow-up (3.66 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.64 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.18 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fix the href for datables in link rel stylesheet (1.43 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fix errors pointed out by the QA script (4.22 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: rename active transaction with All payments to the library and grouping in it the relevant account types 'Pay' 'C' (4.27 KB, patch)
2016-08-07 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6934 - Fix problem C4::Branch::GetBranchesLoop not exist (933 bytes, patch)
2016-09-29 17:32 UTC, Bouzid Fergani
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: New report Cash Register Statistics (23.51 KB, patch)
2016-10-03 21:28 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils) (12.27 KB, patch)
2016-10-03 21:28 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction (6.20 KB, patch)
2016-10-03 21:28 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: QA Follow-up (11.17 KB, patch)
2016-10-03 21:29 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency (3.28 KB, patch)
2016-10-03 21:29 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: second fix to calendars (1.19 KB, patch)
2016-10-03 21:29 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fixed encoding issue (1.00 KB, patch)
2016-10-03 21:30 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: QA follow-up (3.67 KB, patch)
2016-10-03 21:30 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin (3.64 KB, patch)
2016-10-03 21:30 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Make CSV header column translatable and add total amounts in csv file (5.18 KB, patch)
2016-10-03 21:30 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: Fix the href for datables in link rel stylesheet (1.43 KB, patch)
2016-10-03 21:31 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: fix errors pointed out by the QA script (4.11 KB, patch)
2016-10-03 21:31 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934: rename active transaction with All payments to the library and grouping in it the relevant account types 'Pay' 'C' (4.33 KB, patch)
2016-10-03 21:31 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6934 - Fix problem C4::Branch::GetBranchesLoop not exist (1008 bytes, patch)
2016-10-03 21:32 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 6934 - Fix problem C4::Branch::GetBranchesLoop not exist (1.04 KB, patch)
2016-10-24 12:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6934: Fix tiny capitalization issue missed before (1.19 KB, patch)
2016-10-24 12:48 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Pelletier 2011-09-28 21:43:01 UTC Comment hidden (obsolete)
Comment 1 Maxime Pelletier 2011-09-28 21:43:46 UTC Comment hidden (obsolete)
Comment 2 Maxime Pelletier 2011-09-29 13:42:09 UTC
I have to say that this is sponsored by our client, http://www.ccsr.qc.ca/.
Comment 3 Nicole C. Engard 2011-09-29 14:04:58 UTC Comment hidden (obsolete)
Comment 4 Nicole C. Engard 2011-09-29 14:06:39 UTC
Also, this isn't necessary to get the patch pushed, but it would be awesome to also show the librarian/branch who collected the money (maybe a future enhancement to this feature).
Comment 5 Maxime Pelletier 2011-09-29 14:59:01 UTC Comment hidden (obsolete)
Comment 6 Maxime Pelletier 2011-10-06 17:22:40 UTC
Comment on attachment 5641 [details] [review]
New patch with date format fixes and better indentation

I'm afraid that this was not meant to be sent yet.
I will update this bug once we can really share it.
Comment 7 Maxime Pelletier 2012-03-06 22:53:32 UTC
Looks like I forgot to update the bug.
I hope it stills applies to master.
Comment 8 Jared Camins-Esakov 2012-03-17 10:22:32 UTC
Is this code unencumbered, license-wise, and suitable for inclusion at this time into Koha's GPL codebase?
Comment 9 Maxime Pelletier 2012-03-21 00:51:50 UTC
Yes. Sorry if I was unclear. Also sorry to have copied the header form another file. The Katipo copyright is of course wrong and should be removed.
Comment 10 MathildeF 2012-03-23 14:04:33 UTC
The report works only with "Pay fines" 
and not when we create manual invoice or credit
Comment 11 Paul Poulain 2012-03-23 15:47:45 UTC
QA comments (even if the patch is not signed off, I watched it with Mathilde a few minuts ago):

+    Join borrowers t2 On t1.borrowernumber=t2.borrowernumber
+    Where amount<0  AND accounttype like 'Pay' AND CAST(t1.date AS DATE) between ? AND ?
=> you must write SQL keywords in capital cases (WHERE, JOIN,...)
Comment 12 Paul Poulain 2012-03-23 15:49:20 UTC
(In reply to comment #10)
> The report works only with "Pay fines" 
> and not when we create manual invoice or credit

A "manual invoice" should not appear in this report, we agree. but a manual credit should, that's why the patch failed QA

The SQL request should not be "accounttype ='Pay' " but "accounttype in('L','Pay')"
Comment 13 Maxime Pelletier 2012-04-30 20:41:18 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2012-05-04 14:43:08 UTC
Hi Maxime, 
Thanks for your patch.
Sorry but can you repropose it with tabulation characters replaced with 4 spaces please ?
You can read the Koha guidelines on the wiki: http://wiki.koha-community.org/wiki/Coding_Guidelines#Perl
Comment 15 Maxime Pelletier 2012-05-04 15:06:31 UTC Comment hidden (obsolete)
Comment 16 Maxime Pelletier 2012-05-23 14:30:47 UTC
(In reply to comment #12)
> (In reply to comment #10)
> > The report works only with "Pay fines" 
> > and not when we create manual invoice or credit
> 
> A "manual invoice" should not appear in this report, we agree. but a manual
> credit should, that's why the patch failed QA
> 
> The SQL request should not be "accounttype ='Pay' " but "accounttype
> in('L','Pay')"

Just realized that I blindly copy and pasted this, but I think it's wrong. L is for lost. Manual credits are 'C' or 'FOR'.
I will send a new patch soon if this makes sense to you.
Comment 17 Owen Leonard 2012-07-20 12:00:37 UTC
Changing this bug's status back to "assigned" since it appears we're waiting for a new patch?
Comment 18 Maxime Pelletier 2012-09-28 18:08:00 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2012-10-03 13:33:56 UTC Comment hidden (obsolete)
Comment 20 Maxime Pelletier 2013-03-13 18:53:23 UTC
We made many changes to this report recently. I will try to send a new patch soon.
Comment 21 simith.doliveira 2014-08-27 18:37:28 UTC Comment hidden (obsolete)
Comment 22 Nicole C. Engard 2015-03-04 12:59:56 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2015-03-24 23:00:08 UTC
Comment on attachment 36476 [details]
[SIGNED-OFF] Bug 6934 New report Cash Register Statistics

0 of attachment 36476 [details]:
-----------------------------------------------------------------

Hi Simith,

I have added a few comments from code review - please fix in a follow-up. I have only marked the first occurrence of each, please also look below and above to make sure to get them all. 

Thx!

::: koha-tmpl/intranet-tmpl/prog/en/includes/reports-menu.inc
@@ +10,4 @@
>  		<li><a href="/cgi-bin/koha/reports/catalogue_stats.pl">Catalog</a></li>
>  		<li><a href="/cgi-bin/koha/reports/issues_stats.pl">Circulation</a></li>
>          <li><a href="/cgi-bin/koha/reports/serials_stats.pl">Serials</a></li>
> +        <li><a href="/cgi-bin/koha/reports/cash_register_stats.pl">Cash Register</a></li>

Please fix capitalization here and in the other template files to obey our rules. Thx!

::: koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt
@@ +7,5 @@
> +<script type="text/javascript" src="[% themelang %]/js/datatables.js"></script>
> +<script type="text/javascript" id="js">$(document).ready(function() {
> + $(document).ready(function() {
> +    $("#tbl_cash_register_stats").dataTable($.extend(true, {}, dataTablesDefaults, {
> +        "iDisplayLength": 50

Be careful, without adding a paging option here, the paging on the table is broken (see recent 'paging' bugs for examples on how to fix)

@@ +143,5 @@
> +                        <option value="F">Fine</option>
> +                        [% END %]
> +
> +                        [% IF transaction_type == "FU" %]
> +                        <option value="FU" selected="selected">Fine - long period</option>

Please check fine descriptions with those in the other templates - I think this one is not quite right.

@@ +233,5 @@
> +        <thead>
> +        <tr>
> +            <th>Manager name</th>
> +            <th>Borrower cardnumber</th>
> +            <th>Borrower name</th>

Please use patron instead of borrower.

@@ +234,5 @@
> +        <tr>
> +            <th>Manager name</th>
> +            <th>Borrower cardnumber</th>
> +            <th>Borrower name</th>
> +            <th>Branch</th>

Please use library instead of branch.

@@ +240,5 @@
> +            <th>Transaction type</th>
> +            <th>Amount</th>
> +            <th>Biblio title</th>
> +            <th>Barcode</th>
> +            <th>Document type</th>

Please use item type.

::: reports/cash_register_stats.pl
@@ +14,5 @@
> +# with Koha; if not, write to the Free Software Foundation, Inc.,
> +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +
> +use strict;
> +use warnings;

Please use Modern::Perl; instead.

@@ +22,5 @@
> +use C4::Reports;
> +use C4::Output;
> +use C4::Koha;
> +use C4::Circulation;
> +use C4::Dates qw/format_date format_date_in_iso/;

Please avoid use of C4::Dates as it's deprecated.

@@ +45,5 @@
> +my $output           = $input->param("output");
> +my $basename         = $input->param("basename");
> +my $transaction_type = $input->param("transaction_type") || 'ACT';
> +my $branchcode       = $input->param("branch") || C4::Context->userenv->{'branch'};
> +our $sep = ",";

I think the separator should not be hardcoded, it would be better to use a pull down like on the other reports or using the delimiter system preference.

@@ +49,5 @@
> +our $sep = ",";
> +
> +$template->param(
> +    do_it => $do_it,
> +    DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(),

We are no longer using DHTML calendar, please use the TT Dates plugin instead.
Comment 24 Genevieve Plantin 2015-09-15 15:20:37 UTC Comment hidden (obsolete)
Comment 25 Genevieve Plantin 2015-09-15 15:20:49 UTC Comment hidden (obsolete)
Comment 26 Héctor Eduardo Castro Avalos 2015-11-12 02:39:46 UTC
Hi Genevieve

Sorry but patch fails

This is when invalid date is enter in both dates.

Marc can help you with this kind of issues. Or see bugs related with Remove C4::Dates.

Software error:

The 'month' parameter ("32") to DateTime::new did not pass the 'an integer between 1 and 12' callback
 at /usr/lib/i386-linux-gnu/perl5/5.20/DateTime.pm line 197.
	DateTime::new(undef, "month", 32, "minute", 0, "hour", 0, "year", 2015, ...) called at /var/koha/koha/Koha/DateUtils.pm line 164
	Koha::DateUtils::dt_from_string("12/32/2015", "metric") called at /var/koha/koha/reports/cash_register_stats.pl line 75

Another think is patch does not follow dateformat sypref at all

I can see "Cash Register statistics 2015-11-12 to 2015-11-12" in form title
but in interval from 12/11/2015 to 12/11/2015, both most display equal.

Regards
Comment 27 Héctor Eduardo Castro Avalos 2015-11-12 02:49:38 UTC
Fix capitalization in form title "Cash Register statistics 2015-11-12 to 2015-11-12"

Patch still apply, cvs exported successful data in screen display OK.
Comment 28 Marc Véron 2015-11-12 07:37:22 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #26)
(...)
> Marc can help you with this kind of issues. Or see bugs related with Remove
> C4::Dates.
(...)

- Jonathan introduced Bug 15166 - Make output_pref accept a string as parameter.
  An example is to be found in
  acqui/invoice.pl  line 84 (with Bug 14946 applied).

- Todays date as dt: dt_from_string without any params. See example from Bug
  14946, acqui/histsearch.pl, line 81:  	

my $from_placed_on = eval { dt_from_string( $input->param('from') ) } || dt_from_string;
Comment 29 Paul Poulain 2015-11-12 09:33:50 UTC
according to comment #26 it's more failed QA than need discussion
Comment 30 Genevieve Plantin 2015-11-13 15:23:32 UTC Comment hidden (obsolete)
Comment 31 Genevieve Plantin 2015-11-13 15:27:55 UTC
Thanks for the tips, I obsoleted the previous patch and I fix the rest in it.
I understood the KohaDates better, thanks really.

There are some features and improvements that I need to add, it will come in another path soon.
Comment 32 Genevieve Plantin 2015-11-13 16:50:43 UTC Comment hidden (obsolete)
Comment 33 Genevieve Plantin 2015-11-13 16:51:11 UTC
Thanks for the tips, I obsoleted the previous patch and I fix the rest in it.
I understood the KohaDates better, thanks really.

There are some features and improvements that I need to add, it will come in another path soon.
Comment 34 Genevieve Plantin 2015-12-18 20:31:08 UTC
Sorry, I didn't notice the comments #33 was duplicated.

This patch is ready to be tested :)
Comment 35 Héctor Eduardo Castro Avalos 2015-12-24 02:08:17 UTC Comment hidden (obsolete)
Comment 36 Héctor Eduardo Castro Avalos 2015-12-24 02:08:32 UTC Comment hidden (obsolete)
Comment 37 Héctor Eduardo Castro Avalos 2015-12-24 02:08:46 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2016-01-04 14:16:21 UTC
QA comments:
1/ +function isNull(f,noalert) {
not used later

2/ The datepicker from/to should be done using the datepickerfrom and datepickerto css class (see acqui/histsearch.tt for an example).

3/ Why do we have to fill a date?

4/ Ergonomic: the date interval is in a table with only 1 column and 1 row, it could be better to move them in the same fieldset as the 2 dropdown lists.

5/ $fullreportname is only used once, not needed.

6/ +my $fromDate = dt_from_string;
and later
+    beginDate        => dt_from_string($fromDate),
You are passing a DT to dt_from_string, a string is expected.

7/ Remove the unneeded commented lines ("^###")

8/ The csv file should be generated using Text::CSV::Encoded

9/ The column names should be retrieved using C4::Reports::Guided to get the translated values
Comment 39 Rémi Mayrand-Provencher 2016-03-03 16:33:01 UTC Comment hidden (obsolete)
Comment 40 Héctor Eduardo Castro Avalos 2016-03-03 17:23:03 UTC
Created attachment 48642 [details]
Cash register image

Hi Rémi

Still capitalization in reports-menu.inc

>        <li><a href="/cgi-bin/koha/reports/serials_stats.pl">Serials</a></li>
>        <li><a href="/cgi-bin/koha/reports/cash_register_stats.pl">Cash Register</a></li>

I could observe this:

When you start both calendars are set with the current date, i.e., 2016-03-03, but if you change the "From:" date to 2016-01-01, the date in "To:" change also and if you try to change it, for example, to the current date (2016-03-03), you cannot do it at all with the calendar, just manually.

See image attached.
Comment 41 Héctor Eduardo Castro Avalos 2016-03-03 17:30:07 UTC
Of course otherwise works correctly. About QA comments (comment 38) we need to await Jonathan's reply.

Regards
Comment 42 Jonathan Druart 2016-03-04 08:23:13 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #41)
> Of course otherwise works correctly. About QA comments (comment 38) we need
> to await Jonathan's reply.

No :)
Please signoff if everything works as expected.
Then I, or another QAer, will be able to qa it again.
Comment 43 Héctor Eduardo Castro Avalos 2016-03-04 14:13:39 UTC
Hi Rémi

About my comment 40, i don't know if is that the behavior expected, or, needs to be corrected?

Is the patch ready to sign-off? If yes, I'll sign-off immediately.

Regards
Comment 44 Rémi Mayrand-Provencher 2016-03-04 20:14:49 UTC Comment hidden (obsolete)
Comment 45 Rémi Mayrand-Provencher 2016-03-04 20:16:11 UTC
This bug should be ready to be signed-off now, thank you for noticing that the calendar weren't functionning as they should have. I Fixed that now.
Comment 46 Héctor Eduardo Castro Avalos 2016-03-04 21:05:10 UTC
(In reply to Rémi Mayrand-Provencher from comment #45)
> This bug should be ready to be signed-off now, thank you for noticing that
> the calendar weren't functionning as they should have. I Fixed that now.

Sorry Rémi but doesn't work

The message presented is:

Form not submitted because of the following problem(s)
-------------------------------------------------------------------


- Dates cannot be empty

Probably about this bug: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15084
Comment 47 Rémi Mayrand-Provencher 2016-03-04 21:13:56 UTC Comment hidden (obsolete)
Comment 48 Rémi Mayrand-Provencher 2016-03-04 21:15:11 UTC
Really sorry, it was my mistake, I should have tested it again just before sending my patch.
Comment 49 Héctor Eduardo Castro Avalos 2016-03-04 21:31:37 UTC Comment hidden (obsolete)
Comment 50 Héctor Eduardo Castro Avalos 2016-03-04 21:31:56 UTC Comment hidden (obsolete)
Comment 51 Héctor Eduardo Castro Avalos 2016-03-04 21:32:09 UTC Comment hidden (obsolete)
Comment 52 Héctor Eduardo Castro Avalos 2016-03-04 21:36:17 UTC
Rémi just another issue, when exporting there are problem with utf-8:

branchname	date	accounttype	amount	title	barcode	itype
Midway	2016-01-25T00:00:00	Rent	5	A lot like love /	2015-0004	
Midway	2016-01-28T00:00:00	Rent	5	Antología de entremeses del Siglo de Oro /	2016-0001	

But I think IMHO this could be treated in another bug.
Comment 53 Jonathan Druart 2016-03-07 08:24:30 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #52)
> Rémi just another issue, when exporting there are problem with utf-8:
> 
> branchname	date	accounttype	amount	title	barcode	itype
> Midway	2016-01-25T00:00:00	Rent	5	A lot like love /	2015-0004	
> Midway	2016-01-28T00:00:00	Rent	5	Antología de entremeses del Siglo de Oro
> /	2016-0001	
> 
> But I think IMHO this could be treated in another bug.

No, an enhancement won't be pushed with known encoding issues.
Comment 54 Katrin Fischer 2016-03-07 18:47:23 UTC
I agree with Jonathan - please fix the encoding issue!
Comment 55 Rémi Mayrand-Provencher 2016-04-19 16:12:16 UTC Comment hidden (obsolete)
Comment 56 Rémi Mayrand-Provencher 2016-04-19 16:12:56 UTC
Fixed encoding, issue, changing status to needs signoff, thank you!
Comment 57 Héctor Eduardo Castro Avalos 2016-04-19 19:03:33 UTC Comment hidden (obsolete)
Comment 58 Katrin Fischer 2016-04-28 17:32:14 UTC
There are some complaints from the QA script, I can take care of the tabs, but not sure about the last complaint:

 FAIL	reports/cash_register_stats.pl
   OK	  critic
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 176)
		forbidden pattern: tab char (line 159)
		forbidden pattern: tab char (line 162)
		forbidden pattern: tab char (line 160)
		forbidden pattern: tab char (line 156)
		forbidden pattern: tab char (line 163)
		forbidden pattern: tab char (line 179)
		forbidden pattern: tab char (line 178)
		forbidden pattern: tab char (line 157)
		forbidden pattern: tab char (line 180)
		forbidden pattern: tab char (line 164)
		forbidden pattern: tab char (line 174)
		forbidden pattern: tab char (line 181)
		forbidden pattern: tab char (line 177)
		forbidden pattern: tab char (line 158)
		forbidden pattern: tab char (line 171)
		forbidden pattern: tab char (line 161)
		forbidden pattern: tab char (line 175)
   OK	  pod
   OK	  spelling
		0
   FAIL	  valid
		Useless use of hash element in void context
Comment 59 Katrin Fischer 2016-04-28 18:26:09 UTC
I will upload a follow-up for fixing tabs and a capitalization issue. 

But there are some other things that need to be looked at and fixed by the author:
- Remaining QA tools complaint
- What is the definition of an "Active transaction"? (was not clear to me, so asking :) )
- The CSV when exported won't open automatically in LibreOffice for me as all the other reports do - I think something is odd there. Firefox reports it as 'html document'. It's also missing the option to choose a file format and separator for the file as the other reports do.
- when you limit the search, the total amount is no longer correct. On the acq start page there is an example where this works, but not a blocker now.
- itype column is empty in the exported CSV file.
- Please make the CSV header column translatable - take a look at acq for examples (late orders or similar).
- Please use the new TT plugin for formatting the amounts - examples can be found in the acquisition module.
- Please use the TT plugin to display the item type descriptions instead of the codes.

Please come back with this and a follow-up!
Comment 60 Katrin Fischer 2016-04-28 18:27:04 UTC Comment hidden (obsolete)
Comment 61 Bouzid Fergani 2016-07-19 16:07:03 UTC Comment hidden (obsolete)
Comment 62 Bouzid Fergani 2016-07-19 18:58:05 UTC Comment hidden (obsolete)
Comment 63 Héctor Eduardo Castro Avalos 2016-07-19 21:26:44 UTC
Hi Bouzid

For me the patch fails in master when apply

>Applying: Bug 6934 New report Cash Register Statistics
>Using index info to reconstruct a base tree...
>Falling back to patching base and 3-way merge...
>Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
>CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
>Failed to merge in the changes.
>Patch failed at 0001 Bug 6934 New report Cash Register Statistics

Do I need to apply all patches or just few of them?

Regards
Comment 64 Bouzid Fergani 2016-07-20 12:08:44 UTC
Hi Hèctor,
You need apply all patch.
For this conflict, I will rebase it.
Bouzid.
Comment 65 Bouzid Fergani 2016-07-21 14:57:34 UTC Comment hidden (obsolete)
Comment 66 Bouzid Fergani 2016-07-21 14:57:38 UTC Comment hidden (obsolete)
Comment 67 Bouzid Fergani 2016-07-21 14:57:41 UTC Comment hidden (obsolete)
Comment 68 Bouzid Fergani 2016-07-21 14:57:45 UTC Comment hidden (obsolete)
Comment 69 Bouzid Fergani 2016-07-21 14:57:49 UTC Comment hidden (obsolete)
Comment 70 Bouzid Fergani 2016-07-21 14:57:53 UTC Comment hidden (obsolete)
Comment 71 Bouzid Fergani 2016-07-21 14:57:57 UTC Comment hidden (obsolete)
Comment 72 Bouzid Fergani 2016-07-21 14:58:00 UTC Comment hidden (obsolete)
Comment 73 Bouzid Fergani 2016-07-21 14:58:04 UTC Comment hidden (obsolete)
Comment 74 Bouzid Fergani 2016-07-21 14:58:09 UTC Comment hidden (obsolete)
Comment 75 Bouzid Fergani 2016-07-21 15:13:11 UTC Comment hidden (obsolete)
Comment 76 Bouzid Fergani 2016-07-21 15:34:12 UTC
Created attachment 53582 [details] [review]
[SIGNED-OFF] Bug 6934 New report Cash Register Statistics

This patch creates a new report that outputs cash register stats for a specific period.

Changes:

koha-tmpl/intranet-tmpl/prog/en/includes/reports-menu.inc               - Added cash register report link
koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash-register_stats.tt  - new file
koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt         - Added cash register report link
reports/cash-register_stats.pl                                          - new file

Testing:

I Apply the patch

0) Go to reports home page (Home › Reports);
1) Validate link << Cash Register >> under Statistics wizards;
2) Click link;
3) Validate page with three fieldsets;
4) Chose a date interval, a transaction type, a library and an output;
5) Validade the result.

Signed-off-by: Nicole <nicole@bywatersolutions.com>
Comment 77 Bouzid Fergani 2016-07-21 15:34:16 UTC
Created attachment 53583 [details] [review]
[SIGNED-OFF]Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils)

I cleaned up the code according to comment #23. I got rid of DHTMLcalendar_dateformat,
beacause it wasn't use in cash_register_stats.tt. C4::Dates is deprecated,
Koha::DateUtils is now used.
Some column names are changed and the pagination for the dataTables is fixed.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 78 Bouzid Fergani 2016-07-21 15:34:20 UTC
Created attachment 53584 [details] [review]
[SIGNED-OFF]Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction

To be more specific, the column note from accountlines is now displayed in the table of the transactions.
The grand total is now calculated for neagtive types like Credit or Payment and isn't counted for Write off types.
Credit (return item) 'CR' has been added to drop down of transaction type

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Comment 79 Bouzid Fergani 2016-07-21 15:34:24 UTC
Created attachment 53585 [details] [review]
[SIGNED-OFF]Bug 6934: QA Follow-up

Tried to address every single thing that QA mentioned :
1)Removed isNull function
2)Changed datepicker properly
3)We have to fill in two dates not to be flooded with old entries if we don't want to.
4)The first two fieldsets are now in one bigger field set
5)Removed the use of $fullreportname variable
6)Removed the use of dt_from_string when it already recieved a DT.
7)Removed comments starting with "###" in cash_register_stats.pl
8)Used Text::CSV::Encoded to generate the CSV file (used guided_reports.pl as an example)
9)I am not sure if using C4::Reports::Guided is the right choice here. I instead used the same column names as the one recieved by the query in cash_register_stats because this report uses columns from 5 different tables in addition of using both firstname and surname twice.

For the ninth point, I would like to know more about C4::Reports::Guided if you still think using it would be a better solution. Thank you!

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 80 Bouzid Fergani 2016-07-21 15:34:27 UTC
Created attachment 53586 [details] [review]
[SIGNED-OFF]Bug 6934 Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 81 Bouzid Fergani 2016-07-21 15:34:31 UTC
Created attachment 53587 [details] [review]
[SIGNED-OFF]Bug 6934: second fix to calendars

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 82 Bouzid Fergani 2016-07-21 15:34:35 UTC
Created attachment 53588 [details] [review]
[SIGNED-OFF]Bug 6934: fixed encoding issue

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Encoding issue fixed
Comment 83 Bouzid Fergani 2016-07-21 15:34:39 UTC Comment hidden (obsolete)
Comment 84 Bouzid Fergani 2016-07-21 15:34:43 UTC Comment hidden (obsolete)
Comment 85 Bouzid Fergani 2016-07-21 15:34:46 UTC Comment hidden (obsolete)
Comment 86 Bouzid Fergani 2016-07-21 16:44:08 UTC
fixed conflicts, patches should apply correctly.
Comment 87 Héctor Eduardo Castro Avalos 2016-07-22 15:13:15 UTC Comment hidden (obsolete)
Comment 88 Héctor Eduardo Castro Avalos 2016-07-22 15:39:20 UTC
Created attachment 53650 [details] [review]
[SIGNED-OFF]Bug 6934: QA follow-up

- fixed tabs
- fixed capitalization on reports start page

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 89 Héctor Eduardo Castro Avalos 2016-07-22 15:39:33 UTC
Created attachment 53651 [details] [review]
[SIGNED-OFF]Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 90 Héctor Eduardo Castro Avalos 2016-07-22 15:39:45 UTC
Created attachment 53652 [details] [review]
[SIGNED-OFF]Bug 6934: Make CSV header column translatable and add total amounts in csv file

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 91 Héctor Eduardo Castro Avalos 2016-07-22 15:39:55 UTC
Created attachment 53653 [details] [review]
[SIGNED-OFF]Bug 6934: Fix the href for datables in link rel stylesheet

Fix the new path for 'datatables.css'

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 92 Héctor Eduardo Castro Avalos 2016-07-22 15:42:10 UTC
Hi Bouzid

For me this is Ok.

Just explain what does "Active transaction" mean? QA comment 59

Regards
Comment 93 Katrin Fischer 2016-07-27 22:00:58 UTC
Miraculously, this still applies :) Can you please answer the question asked in comment#92?
Comment 94 Katrin Fischer 2016-07-27 22:06:34 UTC
Please also fix these errors pointed out by the QA script. Other problems can be ignored for now ("Tie::Hash::FIELDS"...)

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 39)
		forbidden pattern: tab char (line 31)
		forbidden pattern: tab char (line 36)
		forbidden pattern: tab char (line 32)
		forbidden pattern: tab char (line 34)
		forbidden pattern: tab char (line 37)
		forbidden pattern: tab char (line 33)
		forbidden pattern: tab char (line 35)
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	reports/cash_register_stats.pl
   FAIL	  critic
		"$q_errors" is declared but not used at line 140, column 9. Unused variables clutter code and make it harder to read.
		"$content" is declared but not used at line 146, column 9. Unused variables clutter code and make it harder to read.

I will keep an eye on this and try to continue as soon as the follow up is there.
Comment 95 Katrin Fischer 2016-07-27 22:11:04 UTC
Tiny thing: the total amount in the last grey line of the table is not formatted.
Comment 96 Francois Charbonnier 2016-07-28 13:16:35 UTC
(In reply to Katrin Fischer from comment #59)

> - What is the definition of an "Active transaction"? (was not clear to me,
> so asking :) )

The idea behind "Active transactions" is to help balance cash register transactions. In other words, how much goes in or how much goes out the cash register. The active transactions group what patrons paid or gave as credit to the library, which means what went IN the register.

The code works this way : it takes all transactions where account types  are NOT "F" (Fine), "FU" (Fine unpaid), "FOR" (Forgiven), "M" (Sundry), "L" (Lost).
Comment 97 Bouzid Fergani 2016-07-28 13:39:00 UTC Comment hidden (obsolete)
Comment 98 Héctor Eduardo Castro Avalos 2016-07-28 14:31:28 UTC
Hi Bouzid

One comment:

In bug 16964 I fix the capitalization up for "Report Plugins", right now the bug is in Passed QA. I will try to fix it again in your last patch.

Regards
Comment 99 Bouzid Fergani 2016-07-28 14:51:47 UTC
Hi Héctor,
ok, thanks.
Regards
Comment 100 Héctor Eduardo Castro Avalos 2016-07-28 15:28:31 UTC
Passing QA-Test Tool I got:

FAIL	reports/cash_register_stats.pl
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
		0
   FAIL	  valid
		Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 
		Name "Tie::Hash::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo 
		Use of uninitialized value $_[0] in lc

But I think this is a problem caused for bug 11998 and explained for Jonathan in bug 16104

if you test with: $ perl -cw reports/cash_register_stats.pl
you obtain:
Use of uninitialized value $_[0] in lc at /usr/share/perl5/Cache.pm line 485.
Use of uninitialized value $_[0] in lc at /usr/share/perl5/Cache.pm line 485.
Use of uninitialized value $_[0] in lc at /usr/share/perl5/Cache.pm line 485.
Name "Tie::Hash::FIELDS" used only once: possible typo at /usr/share/perl/5.14/fields.pm line 135.
Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo at /usr/share/perl/5.14/fields.pm line 135.
Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo at /usr/share/perl/5.14/fields.pm line 135.
reports/cash_register_stats.pl syntax OK

But all this errors are from bug 11998

Still
FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 36)
		forbidden pattern: tab char (line 37)
		forbidden pattern: tab char (line 35)
		forbidden pattern: tab char (line 34)
		forbidden pattern: tab char (line 31)
		forbidden pattern: tab char (line 33)
		forbidden pattern: tab char (line 39)
		forbidden pattern: tab char (line 32)

But I will fix it up in the last patch
Comment 101 Héctor Eduardo Castro Avalos 2016-07-28 15:35:08 UTC
Created attachment 53780 [details] [review]
[SIGNED-OFF]Bug6934: fix errors pointed out by the QA script

Also fix forbidden patterns in koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 102 Katrin Fischer 2016-08-01 06:04:24 UTC
(In reply to Francois Charbonnier from comment #96)
> (In reply to Katrin Fischer from comment #59)
> 
> > - What is the definition of an "Active transaction"? (was not clear to me,
> > so asking :) )
> 
> The idea behind "Active transactions" is to help balance cash register
> transactions. In other words, how much goes in or how much goes out the cash
> register. The active transactions group what patrons paid or gave as credit
> to the library, which means what went IN the register.
> 
> The code works this way : it takes all transactions where account types  are
> NOT "F" (Fine), "FU" (Fine unpaid), "FOR" (Forgiven), "M" (Sundry), "L"
> (Lost).

Sorry, I still don't get it :( The list of excluded fine types looks a bit arbitrary to me - what about custom MANUAL_INV values? And what is left by excluding those?
Comment 103 Francois Charbonnier 2016-08-02 14:18:48 UTC
(In reply to Katrin Fischer from comment #102)
> (In reply to Francois Charbonnier from comment #96)
> > (In reply to Katrin Fischer from comment #59)
> > 
> > > - What is the definition of an "Active transaction"? (was not clear to me,
> > > so asking :) )
> > 
> > The idea behind "Active transactions" is to help balance cash register
> > transactions. In other words, how much goes in or how much goes out the cash
> > register. The active transactions group what patrons paid or gave as credit
> > to the library, which means what went IN the register.
> > 
> > The code works this way : it takes all transactions where account types  are
> > NOT "F" (Fine), "FU" (Fine unpaid), "FOR" (Forgiven), "M" (Sundry), "L"
> > (Lost).
> 
> Sorry, I still don't get it :( The list of excluded fine types looks a bit
> arbitrary to me - what about custom MANUAL_INV values? And what is left by
> excluding those?

Hey Katrin,

But you do get that "active transaction" groups all transactions where patrons give money to the library, right ?

The logic should be different. What do you think of grouping the relevant account types, "Pay" and "C" (are there other?) instead of excluding this list?
Comment 104 Katrin Fischer 2016-08-02 15:25:28 UTC
I think that sounds more logical! Also safer with the option of new types being added - not so likely we change the types for paying and credit.

Maybe we could also think about a more 'speaking' name.
Comment 105 Owen Leonard 2016-08-02 15:40:55 UTC
> But you do get that "active transaction" groups all transactions where
> patrons give money to the library, right ?

I don't think "active transaction" has any inherent meaning. What about something like "All payments to the library" ?

Some other things I notice:

- The page is looking for datatables.js in the wrong place. It's now at intranet-tmpl/prog/js/datatables.js.

- The table header "Patron cardnumber" should be "Patron card number."
Comment 106 Katrin Fischer 2016-08-02 21:00:21 UTC
Please take a look at comment#103 - comment#105 for a hopefully last follow-up.
Comment 107 Bouzid Fergani 2016-08-04 19:05:40 UTC
Created attachment 54010 [details] [review]
Bug6934: rename active transaction with  All payments to the library and grouping in it the relevant account types 'Pay' 'C'

    rename Patron carnumber on Patron card number
    fix src databales.js
Comment 108 Katrin Fischer 2016-08-04 21:03:15 UTC
Treating the last as qa follow-up.
Comment 109 Katrin Fischer 2016-08-07 08:22:44 UTC
Created attachment 54088 [details] [review]
[PASSED QA] Bug 6934: New report Cash Register Statistics

This patch creates a new report that outputs cash register stats for a specific period.

Changes:

koha-tmpl/intranet-tmpl/prog/en/includes/reports-menu.inc               - Added cash register report link
koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash-register_stats.tt  - new file
koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt         - Added cash register report link
reports/cash-register_stats.pl                                          - new file

Testing:

I Apply the patch

0) Go to reports home page (Home › Reports);
1) Validate link << Cash Register >> under Statistics wizards;
2) Click link;
3) Validate page with three fieldsets;
4) Chose a date interval, a transaction type, a library and an output;
5) Validade the result.

Signed-off-by: Nicole <nicole@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 110 Katrin Fischer 2016-08-07 08:22:50 UTC
Created attachment 54089 [details] [review]
[PASSED QA] Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils)

I cleaned up the code according to comment #23. I got rid of DHTMLcalendar_dateformat,
beacause it wasn't use in cash_register_stats.tt. C4::Dates is deprecated,
Koha::DateUtils is now used.
Some column names are changed and the pagination for the dataTables is fixed.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 111 Katrin Fischer 2016-08-07 08:22:55 UTC
Created attachment 54090 [details] [review]
[PASSED QA] Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction

To be more specific, the column note from accountlines is now displayed in the table of the transactions.
The grand total is now calculated for neagtive types like Credit or Payment and isn't counted for Write off types.
Credit (return item) 'CR' has been added to drop down of transaction type

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 112 Katrin Fischer 2016-08-07 08:23:00 UTC
Created attachment 54091 [details] [review]
[PASSED QA] Bug 6934: QA Follow-up

Tried to address every single thing that QA mentioned :
1)Removed isNull function
2)Changed datepicker properly
3)We have to fill in two dates not to be flooded with old entries if we don't want to.
4)The first two fieldsets are now in one bigger field set
5)Removed the use of $fullreportname variable
6)Removed the use of dt_from_string when it already recieved a DT.
7)Removed comments starting with "###" in cash_register_stats.pl
8)Used Text::CSV::Encoded to generate the CSV file (used guided_reports.pl as an example)
9)I am not sure if using C4::Reports::Guided is the right choice here. I instead used the same column names as the one recieved by the query in cash_register_stats because this report uses columns from 5 different tables in addition of using both firstname and surname twice.

For the ninth point, I would like to know more about C4::Reports::Guided if you still think using it would be a better solution. Thank you!

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 113 Katrin Fischer 2016-08-07 08:23:05 UTC
Created attachment 54092 [details] [review]
[PASSED QA] Bug 6934: Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 114 Katrin Fischer 2016-08-07 08:23:10 UTC
Created attachment 54093 [details] [review]
[PASSED QA] Bug 6934: second fix to calendars

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 115 Katrin Fischer 2016-08-07 08:23:15 UTC
Created attachment 54094 [details] [review]
[PASSED QA] Bug 6934: fixed encoding issue

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Encoding issue fixed

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 116 Katrin Fischer 2016-08-07 08:23:19 UTC
Created attachment 54095 [details] [review]
[PASSED QA] Bug 6934: QA follow-up

- fixed tabs
- fixed capitalization on reports start page

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 117 Katrin Fischer 2016-08-07 08:23:24 UTC
Created attachment 54096 [details] [review]
[PASSED QA] Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 118 Katrin Fischer 2016-08-07 08:23:29 UTC
Created attachment 54097 [details] [review]
[PASSED QA] Bug 6934: Make CSV header column translatable and add total amounts in csv file

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 119 Katrin Fischer 2016-08-07 08:23:34 UTC
Created attachment 54098 [details] [review]
[PASSED QA] Bug 6934: Fix the href for datables in link rel stylesheet

Fix the new path for 'datatables.css'

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 120 Katrin Fischer 2016-08-07 08:23:39 UTC
Created attachment 54099 [details] [review]
[PASSED QA] Bug 6934: fix errors pointed out by the QA script

Also fix forbidden patterns in koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 121 Katrin Fischer 2016-08-07 08:23:43 UTC
Created attachment 54100 [details] [review]
[PASSED QA] Bug 6934: rename active transaction with All payments to the library and grouping in it the relevant account types 'Pay' 'C'

    rename Patron carnumber on Patron card number
    fix src databales.js

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 122 Kyle M Hall 2016-09-02 16:58:36 UTC
Can someone clean up these patches a bit before I push? I see half are the signed off ones, half are passed qa, and one in the middle that is neither ; )
Comment 123 Bouzid Fergani 2016-09-29 17:32:29 UTC
Created attachment 55939 [details] [review]
Bug 6934 - Fix problem  C4::Branch::GetBranchesLoop not exist
Comment 124 Bouzid Fergani 2016-09-29 17:36:26 UTC
I clean all patches and fix a new minor problem, GetBranchesLoop not exist.
Comment 125 Héctor Eduardo Castro Avalos 2016-10-03 21:28:27 UTC
Created attachment 55988 [details] [review]
[PASSED QA] Bug 6934: New report Cash Register Statistics

This patch creates a new report that outputs cash register stats for a specific period.

Changes:

koha-tmpl/intranet-tmpl/prog/en/includes/reports-menu.inc               - Added cash register report link
koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash-register_stats.tt  - new file
koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt         - Added cash register report link
reports/cash-register_stats.pl                                          - new file

Testing:

I Apply the patch

0) Go to reports home page (Home › Reports);
1) Validate link << Cash Register >> under Statistics wizards;
2) Click link;
3) Validate page with three fieldsets;
4) Chose a date interval, a transaction type, a library and an output;
5) Validade the result.

Signed-off-by: Nicole <nicole@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 126 Héctor Eduardo Castro Avalos 2016-10-03 21:28:39 UTC
Created attachment 55989 [details] [review]
[PASSED QA] Bug 6934: Fix code in CashRegisterStats (dataTables pagination, more accurate descriptions, add a delimiter pull down, change C4::Dates to Koha::DateUtils)

I cleaned up the code according to comment #23. I got rid of DHTMLcalendar_dateformat,
beacause it wasn't use in cash_register_stats.tt. C4::Dates is deprecated,
Koha::DateUtils is now used.
Some column names are changed and the pagination for the dataTables is fixed.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 127 Héctor Eduardo Castro Avalos 2016-10-03 21:28:52 UTC
Created attachment 55990 [details] [review]
[PASSED QA] Bug 6934: New features, note added to transaction information, total caculated for negative types of transaction

To be more specific, the column note from accountlines is now displayed in the table of the transactions.
The grand total is now calculated for neagtive types like Credit or Payment and isn't counted for Write off types.
Credit (return item) 'CR' has been added to drop down of transaction type

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 128 Héctor Eduardo Castro Avalos 2016-10-03 21:29:12 UTC
Created attachment 55991 [details] [review]
[PASSED QA] Bug 6934: QA Follow-up

Tried to address every single thing that QA mentioned :
1)Removed isNull function
2)Changed datepicker properly
3)We have to fill in two dates not to be flooded with old entries if we don't want to.
4)The first two fieldsets are now in one bigger field set
5)Removed the use of $fullreportname variable
6)Removed the use of dt_from_string when it already recieved a DT.
7)Removed comments starting with "###" in cash_register_stats.pl
8)Used Text::CSV::Encoded to generate the CSV file (used guided_reports.pl as an example)
9)I am not sure if using C4::Reports::Guided is the right choice here. I instead used the same column names as the one recieved by the query in cash_register_stats because this report uses columns from 5 different tables in addition of using both firstname and surname twice.

For the ninth point, I would like to know more about C4::Reports::Guided if you still think using it would be a better solution. Thank you!

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 129 Héctor Eduardo Castro Avalos 2016-10-03 21:29:27 UTC
Created attachment 55992 [details] [review]
[PASSED QA] Bug 6934: Fixed Calendar and removed used of unexisting and unused C4::Budgets getCurrency

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 130 Héctor Eduardo Castro Avalos 2016-10-03 21:29:44 UTC
Created attachment 55993 [details] [review]
[PASSED QA] Bug 6934: second fix to calendars

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 131 Héctor Eduardo Castro Avalos 2016-10-03 21:30:03 UTC
Created attachment 55994 [details] [review]
[PASSED QA] Bug 6934: fixed encoding issue

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Encoding issue fixed

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 132 Héctor Eduardo Castro Avalos 2016-10-03 21:30:21 UTC
Created attachment 55995 [details] [review]
[PASSED QA] Bug 6934: QA follow-up

- fixed tabs
- fixed capitalization on reports start page

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 133 Héctor Eduardo Castro Avalos 2016-10-03 21:30:35 UTC
Created attachment 55996 [details] [review]
[PASSED QA] Bug 6934: fix problem when export it open in LibreOffice - show values of itype and use TT plugin

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 134 Héctor Eduardo Castro Avalos 2016-10-03 21:30:51 UTC
Created attachment 55997 [details] [review]
[PASSED QA] Bug 6934: Make CSV header column translatable and add total amounts in csv file

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 135 Héctor Eduardo Castro Avalos 2016-10-03 21:31:09 UTC
Created attachment 55998 [details] [review]
[PASSED QA] Bug 6934: Fix the href for datables in link rel stylesheet

Fix the new path for 'datatables.css'

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 136 Héctor Eduardo Castro Avalos 2016-10-03 21:31:26 UTC
Created attachment 55999 [details] [review]
[PASSED QA] Bug 6934: fix errors pointed out by the QA script

Also fix forbidden patterns in koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 137 Héctor Eduardo Castro Avalos 2016-10-03 21:31:43 UTC
Created attachment 56000 [details] [review]
[PASSED QA] Bug 6934: rename active transaction with All payments to the library and grouping in it the relevant account types 'Pay' 'C'

rename Patron carnumber on Patron card number
    fix src databales.js

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 138 Héctor Eduardo Castro Avalos 2016-10-03 21:32:15 UTC
Created attachment 56001 [details] [review]
[SIGNED-OFF]Bug 6934 - Fix problem C4::Branch::GetBranchesLoop not exist

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Comment 139 Héctor Eduardo Castro Avalos 2016-10-03 21:33:40 UTC
Rebase all patches to avoid merge conflicts and signed-off last patch
Comment 140 Katrin Fischer 2016-10-24 12:47:06 UTC
Created attachment 56763 [details] [review]
[PASSED QA] Bug 6934 - Fix problem C4::Branch::GetBranchesLoop not exist

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 141 Katrin Fischer 2016-10-24 12:48:07 UTC
Created attachment 56764 [details] [review]
Bug 6934: Fix tiny capitalization issue missed before

Account Management Fee
Comment 142 Katrin Fischer 2016-10-24 12:48:36 UTC
Marked the last patch PQA and added another follow-up... let's hope it's the last one!
Comment 143 Kyle M Hall 2016-10-28 11:51:42 UTC
Pushed to master for for 16.11, thanks Simith, Genevieve, Remi, Katrin, Bouzid, and Hector!
Comment 144 Mason James 2016-12-14 12:01:29 UTC
Enhancement, skipping for 16.05.x
Comment 145 Marc Véron 2017-06-07 16:47:12 UTC
For the records (from Bug 18734):
- Export to CSV leads to an Internal Server error, see Bug 18734
- It can be fixed by changing line 174 from exit(1); to exit(0);
  But then a new issue appears: The export has an empty line above the header line, and the header line is mixed with and without double quotes:

mfirstname;card number;firstname;branchname;date;accounttype;amount;title;barcode;"itype"

Noting it here to make the original authors of this feature aware.