Attachment 53585 Details for Bug 6934 – [SIGNED-OFF]Bug 6934: QA Follow-up

[patch] [SIGNED-OFF]Bug 6934: QA Follow-up

SIGNED-OFFBug-6934-QA-Follow-up.patch (text/plain), 11.10 KB, created by Bouzid Fergani on 2016-07-21 15:34:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bouzid Fergani

Created: 2016-07-21 15:34:24 UTC

Size: 11.10 KB

patch

obsolete

>From 184381f8ffd148eba9d3b277e0917018b7486abf Mon Sep 17 00:00:00 2001
>From: remi <remi.mayrand-provencher@inLibro.com>
>Date: Thu, 3 Mar 2016 11:14:12 -0500
>Subject: [PATCH] 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>
>---
> .../prog/en/modules/reports/cash_register_stats.tt | 50 +++--------
> reports/cash_register_stats.pl                     | 98 +++++++++++++---------
> 2 files changed, 70 insertions(+), 78 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt
>index 26778c0..425d333 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt
>@@ -19,12 +19,6 @@
> //<![CDATA[
> actTotal = "";
> 
>-function isNull(f,noalert) {
>-    if (f.value.length > 0) {
>-        return false;
>-    }
>-    return true;
>-}
> 
> $(document).ready(function() {
>     // http://jqueryui.com/demos/datepicker/#date-range
>@@ -87,28 +81,20 @@ $(document).ready(function() {
>     <h1>Cash register statistics</h1>
>     <form method="post" action="/cgi-bin/koha/reports/cash_register_stats.pl" id="frmCashRegister">
>         <fieldset class="rows">
>-        <legend>Cash register statistics [% beginDate | $KohaDates %] to [% endDate | $KohaDates %]</legend>
>-        <table>
>-            <thead>
>-                <tr>
>-                    <th>Interval</th>
>-                </tr>
>-            </thead>
>-            <tbody>
>-                <tr>
>-                    <td>
>-                        <label for="filter_date_begin">From</label>
>-                        <input type="text" size="10" id="filter_date_begin" name="filter_date_begin" value="[% beginDate | $KohaDates %]" />
>-                        <label for="filter_date_end">To</label>
>-                        <input type="text" size="10" id="filter_date_end" name="filter_date_end" value="[% endDate | $KohaDates %]" />
>-                    </td>
>-                </tr>
>-            </tbody>
>-        </table><br />
>-        </fieldset>
>-
>-        <fieldset class="rows">
>+            <legend>Cash register statistics [% beginDate | $KohaDates %] to [% endDate | $KohaDates %]</legend>
>             <ol>
>+                <br>
>+                <li>
>+                    <label for="filter_date_begin">From: </label>
>+                    <input type="text" size="10" id="filter_date_begin" name="filter_date_begin" value="[% beginDate | $KohaDates %]" class="datepickerfrom" />
>+                    <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
>+                </li>
>+                <li>
>+                    <label for="filter_date_end">To: </label>
>+                    <input type="text" size="10" id="filter_date_end" name="filter_date_end" value="[% endDate | $KohaDates %]" class="datepickerto" />
>+                    <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
>+                </li>
>+
>                 <li>
>                     <label for="">Transaction type:</label>
>                     <select name="transaction_type" id="transaction_type">
>@@ -223,16 +209,6 @@ $(document).ready(function() {
>                     <label for="outputfile">To a file:</label>
>                     <input type="radio" name="output" value="file" id="outputfile" />
>                     <label class="inline" for="basename">Named: </label><input type="text" name="basename" id="basename" value="Export" />
>-                    <label class="inline" for="sep">Delimiter: </label>
>-                    <select name="sep" id="sep" size="1">
>-                        [% FOREACH value IN CGIsepChoice.values.sort() %]
>-                           [% IF ( value == CGIsepChoice.default ) %]
>-                               <option value="[% value %]" selected="selected">[% value %]</option>
>-                           [% ELSE %]
>-                              <option value="[% value %]">[% value %]</option>
>-                           [% END %]
>-                        [% END %]
>-                    </select>
> 
>                 </li>
>             </ol>
>diff --git a/reports/cash_register_stats.pl b/reports/cash_register_stats.pl
>index 8b09661..7d89e65 100755
>--- a/reports/cash_register_stats.pl
>+++ b/reports/cash_register_stats.pl
>@@ -25,15 +25,15 @@ use C4::Circulation;
> use DateTime;
> use Koha::DateUtils;
> use C4::Budgets qw/GetCurrency GetCurrencies/;
>+use Text::CSV::Encoded;
> #use Data::Dumper;
> #use Smart::Comments;
> 
> my $input            = new CGI;
> my $dbh              = C4::Context->dbh;
>-my $fullreportname   = "reports/cash_register_stats.tt";
> 
> my ($template, $borrowernumber, $cookie) = get_template_and_user({
>-    template_name => $fullreportname,
>+    template_name => "reports/cash_register_stats.tt",
>     query => $input,
>     type => "intranet",
>     authnotrequired => 0,
>@@ -46,8 +46,6 @@ my $output           = $input->param("output");
> my $basename         = $input->param("basename");
> my $transaction_type = $input->param("transaction_type") || 'ACT';
> my $manager_branchcode       = $input->param("branch") || C4::Context->userenv->{'branch'};
>-our $sep = $input->param("sep") // ',';
>-$sep = "\t" if ($sep eq 'tabulation');
> 
> $template->param(
>     do_it => $do_it,
>@@ -57,14 +55,12 @@ $template->param(
> #Initialize date pickers to today
> my $fromDate = dt_from_string;
> my $toDate   = dt_from_string;
>-### fromdate today: $fromDate
> 
> my $query_manualinv = "SELECT id, authorised_value FROM authorised_values WHERE category = 'MANUAL_INV'";
> my $sth_manualinv = $dbh->prepare($query_manualinv) or die "Unable to prepare query" . $dbh->errstr;
> $sth_manualinv->execute() or die "Unable to execute query " . $sth_manualinv->errstr;
> my $manualinv_types = $sth_manualinv->fetchall_arrayref({});
> 
>-### $manualinv_types
> 
> if ($do_it) {
> 
>@@ -92,7 +88,6 @@ if ($do_it) {
>         $whereBranchCode = "AND m.branchcode = '$manager_branchcode'";
>     }
> 
>-    ### $transaction_type;
> 
>     my $query = "
>     SELECT round(amount,2) AS amount, description,
>@@ -121,7 +116,6 @@ if ($do_it) {
>         #if ((abs($row->{amount}) - $row->{amountoutstanding}) > 0) {
>             $row->{amount} = sprintf("%.2f", abs ($row->{amount}));
>             $row->{date} = dt_from_string($row->{date}, 'sql');
>-            ### date : $row->{date}
> 
>             push (@loopresult, $row);
>             if($transaction_type eq 'ACT' && ($row->{accounttype} !~ /^C$|^CR$|^LR$|^Pay$/)){
>@@ -148,49 +142,71 @@ if ($do_it) {
>         );
>     } else{
>         binmode STDOUT, ':encoding(UTF-8)';
>+        my $q_errors;
>+        my $format = 'csv';
>+        my $reportname = $input->param('basename');
>+        my $reportfilename = $reportname ? "$reportname.$format" : "reportresults.$format" ;
>+        #my $reportfilename = "$reportname.html" ;
>+        my $delimiter = C4::Context->preference('delimiter') || ',';
>+        my ( $type, $content );
>+        if ( $format eq 'csv' ) {
>+            my $type = 'application/csv';
>+            my $csv = Text::CSV::Encoded->new({ encoding_out => 'UTF-8', sep_char => $delimiter});
>+            $csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag();
>+            my @headers = ();
>+            push @headers, "mfirstname",
>+			            "cardnumber",
>+			            "bfirstname",
>+			            "branchname",
>+			            "date",
>+			            "accounttype",
>+			            "amount",
>+			            "title",
>+			            "barcode",
>+			            "itype";
>+            if ($csv->combine(@headers)) {
>+                $content .= Encode::decode('UTF-8', $csv->string()) . "\n";
>+            } else {
>+                push @$q_errors, { combine => 'HEADER ROW: ' . $csv->error_diag() } ;
>+            }
>+            foreach my $row (@loopresult) {
>+				my @rowValues = ();
>+                push @rowValues, $row->{mfirstname},
>+                        $row->{cardnumber},
>+						$row->{bfirstname},
>+						$row->{branchname},
>+						$row->{date},
>+						$row->{accounttype},
>+						$row->{amount},
>+						$row->{title},
>+						$row->{barcode};
>+						$row->{itype};
>+                if ($csv->combine(@rowValues)) {
>+                    $content .= $csv->string() . "\n";
>+                } else {
>+                    push @$q_errors, { combine => $csv->error_diag() } ;
>+                }
>+            }
>+        }
>         print $input->header(
>-            -type => 'application/vnd.sun.xml.calc',
>-            -encoding => 'utf-8',
>-            -name => "$basename.csv",
>-            -attachment => "$basename.csv"
>+            -type => $type,
>+            -attachment=> $reportfilename
>         );
>+        print $content;
> 
>-        print "Manager name".$sep;
>-        print "Borrower cardnumber".$sep;
>-        print "Borrower name".$sep;
>-        print "Branch".$sep;
>-        print "Transaction date".$sep;
>-        print "Transaction type".$sep;
>-        print "Amount".$sep;
>-        print "Biblio title".$sep;
>-        print "Barcode".$sep;
>-        print "Document type"."\n";
>-
>-        foreach my $item (@loopresult){
>-            print $item->{mfirstname}. ' ' . $item->{msurname} . $sep;
>-            print $item->{cardnumber}.$sep;
>-            print $item->{bfirstname}. ' ' . $item->{bsurname} . $sep;
>-            print $item->{branchname}.$sep;
>-            print $item->{date}.$sep;
>-            print $item->{accounttype}.$sep;
>-            print $item->{amount}.$sep;
>-            print $item->{title}.$sep;
>-            print $item->{barcode}.$sep;
>-            print $item->{itype}."\n";
>-        }
>-
>-        print $sep x 6;
>+        print $delimiter x 6;
>         print $grantotal."\n";
>+        foreach my $err (@$q_errors) {
>+            print "# ERROR: " . (map {$_ . ": " . $err->{$_}} keys %$err) . "\n";
>+        }   # here we print all the non-fatal errors at the end.  Not super smooth, but better than nothing.
>         exit(1);
>     }
> 
> }
> 
>-### fromdate final: $fromDate
>-### toDate final: $toDate
> $template->param(
>-    beginDate        => dt_from_string($fromDate),
>-    endDate          => dt_from_string($toDate),
>+    beginDate        => $fromDate,
>+    endDate          => $toDate,
>     transaction_type => $transaction_type,
>     branchloop       => C4::Branch::GetBranchesLoop($manager_branchcode),
>     manualinv_types  => $manualinv_types,
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 6934: 5634 | 5635 | 5640 | 5641 | 9379 | 9419 | 12602 | 12660 | 31210 | 36476 | 42564 | 42565 | 44847 | 44853 | 45972 | 45973 | 45974 | 48634 | 48642 | 48704 | 48705 | 48706 | 48707 | 48708 | 50429 | 50437 | 50938 | 53496 | 53500 | 53570 | 53571 | 53572 | 53573 | 53574 | 53575 | 53576 | 53577 | 53578 | 53579 | 53581 | 53582 | 53583 | 53584 | 53585 | 53586 | 53587 | 53588 | 53589 | 53590 | 53591 | 53648 | 53650 | 53651 | 53652 | 53653 | 53778 | 53780 | 54010 | 54088 | 54089 | 54090 | 54091 | 54092 | 54093 | 54094 | 54095 | 54096 | 54097 | 54098 | 54099 | 54100 | 55939 | 55988 | 55989 | 55990 | 55991 | 55992 | 55993 | 55994 | 55995 | 55996 | 55997 | 55998 | 55999 | 56000 | 56001 | 56763 | 56764