Attachment 129945 Details for Bug 12285 – Bug 12285: Fix QA issues

[patch] Bug 12285: Fix QA issues

Bug-12285-Fix-QA-issues.patch (text/plain), 8.81 KB, created by Owen Leonard on 2022-01-28 14:05:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2022-01-28 14:05:03 UTC

Size: 8.81 KB

patch

obsolete

>From 0e7f78163a5c8519eed99e64f75a28ee21c37aa3 Mon Sep 17 00:00:00 2001
>From: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
>Date: Mon, 26 Jul 2021 10:08:48 +0300
>Subject: [PATCH] Bug 12285: Fix QA issues
>
>This patch:
>- removes GetBorrowerFines and reworks Fineslip to use Koha objects
>- adds the new notice template in the translated sample_notices.sql
>files
>- adds unit tests
>
>Sponsored-by: Koha-Suomi Oy
>
>Signed-off-by: Owen Leonard <oleonard@myacpl.org>
>---
> C4/Members.pm                                 |  52 +++----
> .../fr-FR/1-Obligatoire/sample_notices.sql    |  15 ++
> t/db_dependent/Members/FineSlip.t             | 135 ++++++++++++++++++
> 3 files changed, 177 insertions(+), 25 deletions(-)
> create mode 100755 t/db_dependent/Members/FineSlip.t
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index e3f58ee139..8e2f751cf0 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -29,11 +29,15 @@ use C4::Reserves;
> use C4::Accounts;
> use C4::Letters qw( GetPreparedLetter );
> use DateTime;
>+use Koha::Account::Lines;
>+use Koha::Checkouts;
>+use Koha::Old::Checkouts;
> use Koha::Database;
> use Koha::DateUtils qw( dt_from_string output_pref );
> use Koha::Database;
> use Koha::Holds;
> use Koha::AdditionalContents;
>+use Koha::Items;
> use Koha::Patrons;
> use Koha::Patron::Categories;
> 
>@@ -684,39 +688,37 @@ WHERE borrowernumber = 0 AND DATEDIFF( NOW(), timestamp ) > ?|;
>     return $cnt eq '0E0'? 0: $cnt;
> }
> 
>-sub GetBorrowerFines {
>-    my ($borrowernumber) = @_;
>-    my $dbh       = C4::Context->dbh;
>-    my $query = qq{
>-        SELECT * FROM accountlines
>-        LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber
>-        LEFT JOIN items ON accountlines.itemnumber = items.itemnumber
>-        WHERE accountlines.borrowernumber = ?
>-        ORDER BY accountlines.timestamp
>-    };
>-    my $sth = $dbh->prepare( $query );
>-    $sth->execute( $borrowernumber );
>-    my $data = $sth->fetchall_arrayref({});
>-    return $data;
>-}
>+=head2 FineSlip
>+
>+    Returns letter hash and populates FINESLIP.
>+
>+=cut
> 
> sub FineSlip {
>     my ($borrowernumber, $branch) = @_;
> 
>-    #my $now       = POSIX::strftime("%Y-%m-%d", localtime);
>+    my $lines = Koha::Account::Lines->search({ borrowernumber => $borrowernumber });
>+    my $total = $lines->total_outstanding;
>+    my @issueslist;
>+
>+    while (my $line = $lines->next) {
>+        next if ($line->amountoutstanding =~ /^0.0+$/);
> 
>-    my $issueslist = GetBorrowerFines($borrowernumber);
>+        my $item = Koha::Items->find({itemnumber => $line->itemnumber});
>+        $item = $item ? $item->unblessed : undef;
>+        my $issue = Koha::Checkouts->find({issue_id => $line->issue_id})
>+        || Koha::Old::Checkouts->find({issue_id => $line->issue_id});
>+        my $dt = dt_from_string( $issue->date_due );
> 
>-    my $total = 0.0;
>+        $item->{'date_due'} = output_pref({ dt => $dt, dateonly => 1 });
>+        $item->{'amount'} = sprintf('%.2f', $line->amount);
>+        $item->{'barcode'} = $item->{barcode} ? $item->{barcode} : '-';
>+        $item->{'timestamp'} = $line->timestamp;
>+        $item->{'description'} = $line->description;
> 
>-    foreach my $it (@$issueslist){
>-        my $dt = dt_from_string( $it->{'date'} );
>-        $it->{'date_due'} = output_pref({ dt => $dt, dateonly => 1 });
>-        $it->{'amount'} = sprintf('%.2f', $it->{'amount'});
>-        $total = $total + $it->{'amount'};
>-        $it->{'barcode'} = '-' if (!defined($it->{'barcode'}));
>+        push @issueslist, $item;
>     }
>-    my @issues = sort { $b->{'timestamp'} <=> $a->{'timestamp'} } @$issueslist;
>+    my @issues = sort { $b->{'timestamp'} <=> $a->{'timestamp'} } @issueslist;
> 
>     my %repeat = (
>         'fines' => [ map {
>diff --git a/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql b/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>index 7712ab46ac..840aa05afd 100644
>--- a/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>+++ b/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>@@ -475,3 +475,18 @@ INSERT IGNORE INTO letter (module, code, name, title, content, message_transport
>     [% END %]
> [% END %]
> ", 'email');
>+
>+INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
>+VALUES ( 'circulation', 'FINESLIP', '', 'Patron fines -slip', '1', 'Fines', '<<borrowers.firstname>> <<borrowers.surname>><br>
>+<<borrowers.cardnumber>><br>
>+Fines: <<total.fines>>
>+<ul>
>+<fines>
>+<li><<fines.date_due>>, <<fines.amount>><br>
>+Bar code: <<items.barcode>><br>
>+<<fines.description>></li>
>+</fines>
>+</ul>
>+Total: <<total.amount>>
>+', 'print'
>+);
>\ No newline at end of file
>diff --git a/t/db_dependent/Members/FineSlip.t b/t/db_dependent/Members/FineSlip.t
>new file mode 100755
>index 0000000000..215bef8e68
>--- /dev/null
>+++ b/t/db_dependent/Members/FineSlip.t
>@@ -0,0 +1,135 @@
>+#!/usr/bin/perl
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 1;
>+use Test::MockModule;
>+use Test::MockTime qw( set_fixed_time );
>+use t::lib::Mocks;
>+use t::lib::TestBuilder;
>+
>+use C4::Circulation qw( AddIssue AddReturn );
>+use C4::Members qw( FineSlip );
>+use C4::Overdues qw( UpdateFine );
>+
>+use Koha::CirculationRules;
>+use Koha::DateUtils qw( dt_from_string output_pref );
>+
>+my $schema = Koha::Database->schema;
>+$schema->storage->txn_begin;
>+my $dbh = C4::Context->dbh;
>+
>+$dbh->do(q|DELETE FROM letter|);
>+$dbh->do(q|DELETE FROM circulation_rules|);
>+
>+my $builder = t::lib::TestBuilder->new;
>+set_fixed_time(CORE::time());
>+
>+my $branchcode   = $builder->build({ source => 'Branch' })->{ branchcode };
>+my $itemtype     = $builder->build({ source => 'Itemtype' })->{ itemtype };
>+
>+my $item  = $builder->build_sample_item({
>+    homebranch => $branchcode,
>+    itype      => $itemtype
>+});
>+
>+my $issuingrule = Koha::CirculationRules->set_rules(
>+    {
>+        categorycode => undef,
>+        itemtype     => undef,
>+        branchcode   => undef,
>+        rules        => {
>+            fine                   => 1,
>+            finedays               => 0,
>+            chargeperiod           => 7,
>+            chargeperiod_charge_at => 0,
>+            lengthunit             => 'days',
>+            issuelength            => 1,
>+        }
>+    }
>+);
>+
>+my $module = Test::MockModule->new('C4::Context');
>+$module->mock( 'userenv', sub { { branch => $branchcode } } );
>+
>+my $slip_content = <<EOS;
>+<<borrowers.firstname>> <<borrowers.surname>>
>+<<borrowers.cardnumber>>
>+Fines and fees: <<total.fines>>
>+<fines>
>+<<fines.date_due>>, <<fines.amount>>
>+Barcode: <<items.barcode>>
>+<<fines.description>>
>+</fines>
>+Total: <<total.amount>>
>+EOS
>+
>+$dbh->do(q|
>+    INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type) VALUES ( 'circulation', 'FINESLIP', '', 'Patron fines -slip', '1', 'Fines and fees slip', ?, 'print')
>+|, {}, $slip_content);
>+
>+my $patron = $builder->build(
>+    {
>+        source => 'Borrower',
>+        value  => {
>+            surname    => 'Patron',
>+            firstname  => 'New',
>+            cardnumber => '0011223344',
>+            branchcode => $branchcode
>+        },
>+    }
>+);
>+
>+my $today = dt_from_string();
>+my $date_due = dt_from_string->subtract_duration( DateTime::Duration->new( days => 13 ) );
>+my $issue_date = dt_from_string->subtract_duration( DateTime::Duration->new( days => 14 ) );
>+my $barcode = $item->barcode;
>+
>+my $issue = AddIssue($patron, $barcode, $date_due, undef, $issue_date);
>+t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1);
>+AddReturn( $barcode, $branchcode);
>+
>+my $lines = Koha::Account::Lines->search({ borrowernumber => $patron->{borrowernumber} });
>+my $total_fines = $lines->count;
>+my $fine_amount = 0;
>+my $description = '';
>+while (my $line = $lines->next){
>+    $fine_amount = sprintf('%.2f', $line->amount);
>+    $description = $line->description;
>+}
>+
>+my $total = sprintf('%.2f', $lines->total_outstanding);
>+
>+$date_due = output_pref({ dt => $date_due, dateonly => 1 });
>+
>+my $fineslip = FineSlip( $patron->{borrowernumber}, $branchcode );
>+my $expected_slip = <<EOS;
>+New Patron
>+0011223344
>+Fines and fees: $total_fines
>+
>+$date_due, $fine_amount
>+Barcode: $barcode
>+$description
>+
>+Total: $total
>+EOS
>+
>+is( $fineslip->{content}, $expected_slip, 'Fineslip returns slip with 1 item' );
>+
>+$schema->storage->txn_rollback;
>\ No newline at end of file
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 12285: 28327 | 29144 | 29692 | 123202 | 123203 | 123243 | 129723 | 129724 | 129922 | 129923 | 129937 | 129943 | 129944 | 129945 | 129946 | 129947 | 133112 | 133147 | 139037 | 139038 | 139039 | 139040 | 139041 | 139042 | 140795 | 140796 | 140797 | 140798 | 140799 | 140800