Bugzilla – Attachment 84622 Details for
Bug 20912
Rental fees based on time period
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20912: (follow-up) Improve test coverage
Bug-20912-follow-up-Improve-test-coverage.patch (text/plain), 18.02 KB, created by
Kyle M Hall (khall)
on 2019-02-01 13:50:25 UTC
(
hide
)
Description:
Bug 20912: (follow-up) Improve test coverage
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2019-02-01 13:50:25 UTC
Size:
18.02 KB
patch
obsolete
>From a88c1c2b9bb2836110975ed93fb0ecc526d161f9 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Wed, 30 Jan 2019 16:20:23 +0000 >Subject: [PATCH] Bug 20912: (follow-up) Improve test coverage > >Increase test coverage for CanBookBeIssued and fix a introduced during >the refactoring to Koha::Fees. > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Circulation.pm | 22 +++--- > Koha/Charges/Fees.pm | 12 ++-- > Koha/Schema/Result/Itemtype.pm | 4 +- > admin/itemtypes.pl | 6 +- > installer/data/mysql/kohastructure.sql | 2 +- > .../prog/en/modules/admin/itemtypes.tt | 6 +- > .../prog/en/modules/catalogue/moredetail.tt | 2 +- > t/db_dependent/Circulation.t | 71 +++++++++++++++++-- > t/db_dependent/Koha/Charges/Fees.t | 14 ++-- > 9 files changed, 98 insertions(+), 41 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 6c7ae420ee..3641aebc9f 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -994,7 +994,7 @@ sub CanBookBeIssued { > if ( $rentalConfirmation ){ > my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $patron->borrowernumber ); > my $itemtype = Koha::ItemTypes->find( $item->{itype} ); # GetItem sets effective itemtype >- $rentalCharge += $itemtype->calc_rental_charge_daily( { from => dt_from_string(), to => $duedate } ); >+ $rentalCharge += $fees->accumulate_rentalcharge(); > if ( $rentalCharge > 0 ){ > $needsconfirmation{RENTALCHARGE} = $rentalCharge; > } >@@ -1454,7 +1454,7 @@ sub AddIssue { > ); > ModDateLastSeen( $item->{'itemnumber'} ); > >- # If it costs to borrow this book, charge it to the patron's account. >+ # If it costs to borrow this book, charge it to the patron's account. > my ( $charge ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} ); > if ( $charge > 0 ) { > AddIssuingCharge( $issue, $charge ); >@@ -1463,10 +1463,10 @@ sub AddIssue { > > my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); > if ( $itemtype ) { >- my $daily_charge = $fees->rental_charge_daily(); >- if ( $daily_charge > 0 ) { >- AddIssuingCharge( $issue, $daily_charge, 'Daily rental' ) if $daily_charge > 0; >- $charge += $daily_charge; >+ my $accumulate_charge = $fees->accumulate_rentalcharge(); >+ if ( $accumulate_charge > 0 ) { >+ AddIssuingCharge( $issue, $accumulate_charge, 'Daily rental' ) if $accumulate_charge > 0; >+ $charge += $accumulate_charge; > $item->{charge} = $charge; > } > } >@@ -2954,15 +2954,15 @@ sub AddRenewal { > )->store(); > } > >- # Charge a new daily rental fee, if applicable >+ # Charge a new accumulate rental fee, if applicable > my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); > if ( $itemtype ) { >- my $daily_charge = $fees->rental_charge_daily(); >- if ( $daily_charge > 0 ) { >+ my $accumulate_charge = $fees->accumulate_rentalcharge(); >+ if ( $accumulate_charge > 0 ) { > my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item->{'barcode'}"; >- AddIssuingCharge( $issue, $daily_charge, $type_desc ) >+ AddIssuingCharge( $issue, $accumulate_charge, $type_desc ) > } >- $charge += $daily_charge; >+ $charge += $accumulate_charge; > } > > # Send a renewal slip according to checkout alert preferencei >diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm >index 62f534147c..0d948338b3 100644 >--- a/Koha/Charges/Fees.pm >+++ b/Koha/Charges/Fees.pm >@@ -75,22 +75,22 @@ sub new { > return bless( $params, $class ); > } > >-=head3 rental_charge_daily >+=head3 accumulate_rentalcharge > >- my $fee = $self->rental_charge_daily(); >+ my $fee = $self->accumulate_rentalcharge(); > > This method calculates the daily rental fee for a given itemtype for a given > period of time passed in as a pair of DateTime objects. > > =cut > >-sub rental_charge_daily { >+sub accumulate_rentalcharge { > my ( $self, $params ) = @_; > > my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); >- my $rental_charge_daily = $itemtype->rental_charge_daily; >+ my $rentalcharge_daily = $itemtype->rentalcharge_daily; > >- return undef unless $rental_charge_daily && $rental_charge_daily > 0; >+ return undef unless $rentalcharge_daily && $rentalcharge_daily > 0; > > my $duration; > if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) { >@@ -102,7 +102,7 @@ sub rental_charge_daily { > } > my $days = $duration->in_units('days'); > >- my $charge = $rental_charge_daily * $days; >+ my $charge = $rentalcharge_daily * $days; > > return $charge; > } >diff --git a/Koha/Schema/Result/Itemtype.pm b/Koha/Schema/Result/Itemtype.pm >index a7c84f1046..6f022e986f 100644 >--- a/Koha/Schema/Result/Itemtype.pm >+++ b/Koha/Schema/Result/Itemtype.pm >@@ -41,7 +41,7 @@ __PACKAGE__->table("itemtypes"); > is_nullable: 1 > size: [28,6] > >-=head2 rental_charge_daily >+=head2 rentalcharge_daily > > data_type: 'decimal' > is_nullable: 1 >@@ -115,7 +115,7 @@ __PACKAGE__->add_columns( > { data_type => "longtext", is_nullable => 1 }, > "rentalcharge", > { data_type => "decimal", is_nullable => 1, size => [28, 6] }, >- "rental_charge_daily", >+ "rentalcharge_daily", > { data_type => "decimal", is_nullable => 1, size => [28, 6] }, > "defaultreplacecost", > { data_type => "decimal", is_nullable => 1, size => [28, 6] }, >diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl >index 723dfe9c93..3381e21ec3 100755 >--- a/admin/itemtypes.pl >+++ b/admin/itemtypes.pl >@@ -72,7 +72,7 @@ if ( $op eq 'add_form' ) { > my $itemtype = Koha::ItemTypes->find($itemtype_code); > my $description = $input->param('description'); > my $rentalcharge = $input->param('rentalcharge'); >- my $rental_charge_daily = $input->param('rental_charge_daily'); >+ my $rentalcharge_daily = $input->param('rentalcharge_daily'); > my $defaultreplacecost = $input->param('defaultreplacecost'); > my $processfee = $input->param('processfee'); > my $image = $input->param('image') || q||; >@@ -93,7 +93,7 @@ if ( $op eq 'add_form' ) { > if ( $itemtype and $is_a_modif ) { # it's a modification > $itemtype->description($description); > $itemtype->rentalcharge($rentalcharge); >- $itemtype->rental_charge_daily($rental_charge_daily); >+ $itemtype->rentalcharge_daily($rentalcharge_daily); > $itemtype->defaultreplacecost($defaultreplacecost); > $itemtype->processfee($processfee); > $itemtype->notforloan($notforloan); >@@ -118,7 +118,7 @@ if ( $op eq 'add_form' ) { > itemtype => $itemtype_code, > description => $description, > rentalcharge => $rentalcharge, >- rental_charge_daily => $rental_charge_daily, >+ rentalcharge_daily => $rentalcharge_daily, > defaultreplacecost => $defaultreplacecost, > processfee => $processfee, > notforloan => $notforloan, >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 264d4b1bbe..dcb6447c25 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -988,7 +988,7 @@ CREATE TABLE `itemtypes` ( -- defines the item types > itemtype varchar(10) NOT NULL default '', -- unique key, a code associated with the item type > description LONGTEXT, -- a plain text explanation of the item type > rentalcharge decimal(28,6) default NULL, -- the amount charged when this item is checked out/issued >- rental_charge_daily decimal(28,6) default NULL, -- the amount charged for each day between checkout date and due date >+ rentalcharge_daily decimal(28,6) default NULL, -- the amount charged for each increment (day/hour) between checkout date and due date > defaultreplacecost decimal(28,6) default NULL, -- default replacement cost > processfee decimal(28,6) default NULL, -- default text be recorded in the column note when the processing fee is applied > notforloan smallint(6) default NULL, -- 1 if the item is not for loan, 0 if the item is available for loan >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >index 6c8b0e71ba..e692fc7950 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >@@ -235,8 +235,8 @@ Item types administration > <span class="hint">This fee is charged once per checkout/renewal per item</span> > </li> > <li> >- <label for="rental_charge_daily">Daily rental charge: </label> >- <input type="text" id="rental_charge_daily" name="rental_charge_daily" size="10" value="[% itemtype.rental_charge_daily | $Price %]" /> >+ <label for="rentalcharge_daily">Daily rental charge: </label> >+ <input type="text" id="rentalcharge_daily" name="rentalcharge_daily" size="10" value="[% itemtype.rentalcharge_daily | $Price %]" /> > <span class="hint">This fee is charged a checkout/renewal time for each day between the checkout/renewal date and due date.</span> > </li> > <li> >@@ -381,7 +381,7 @@ Item types administration > </td> > <td> > [% UNLESS ( itemtype.notforloan ) %] >- [% itemtype.rental_charge_daily | $Price %] >+ [% itemtype.rentalcharge_daily | $Price %] > [% END %] > </td> > <td>[% itemtype.defaultreplacecost | $Price %]</td> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >index 183e78f949..53225a2ce9 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >@@ -34,7 +34,7 @@ > <li><span class="label">Item type:</span> [% itemtypename | html %] </li> > [% END %] > [% IF ( rentalcharge ) %]<li><span class="label">Rental charge:</span>[% rentalcharge | $Price %] </li>[% END %] >- [% IF ( rental_charge_daily ) %]<li><span class="label">Daily rental charge:</span>[% rental_charge_daily | $Price %] </li>[% END %] >+ [% IF ( rentalcharge_daily ) %]<li><span class="label">Daily rental charge:</span>[% rentalcharge_daily | $Price %] </li>[% END %] > <li><span class="label">ISBN:</span> [% isbn | html %] </li> > <li><span class="label">Publisher:</span>[% place | html %] [% publishercode | html %] [% publicationyear | html %] </li> > [% IF ( volumeddesc ) %]<li><span class="label">Volume:</span> [% volumeddesc | html %]</li>[% END %] >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 9f9b47230b..839e3fd818 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 125; >+use Test::More tests => 126; > use Test::MockModule; > > use Data::Dumper; >@@ -73,7 +73,7 @@ my $itemtype = $builder->build( > value => { > notforloan => undef, > rentalcharge => 0, >- rental_charge_daily => 0, >+ rentalcharge_daily => 0, > defaultreplacecost => undef, > processfee => undef > } >@@ -1951,7 +1951,7 @@ subtest '_FixAccountForLostAndReturned' => sub { > rentalcharge => 0, > defaultreplacecost => undef, > processfee => $processfee_amount, >- rental_charge_daily => 0, >+ rentalcharge_daily => 0, > } > } > ); >@@ -2249,7 +2249,7 @@ subtest '_FixAccountForLostAndReturned' => sub { > rentalcharge => 0, > defaultreplacecost => undef, > processfee => 0, >- rental_charge_daily => 0, >+ rentalcharge_daily => 0, > } > } > ); >@@ -2853,7 +2853,7 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub { > ); > > my $library = $builder->build_object({ class => 'Koha::Libraries' }); >- my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes', value => { rental_charge_daily => 0.00 }}); >+ my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes', value => { rentalcharge_daily => 0.00 }}); > my $patron = $builder->build_object({ > class => 'Koha::Patrons', > value => { branchcode => $library->id } >@@ -3016,7 +3016,7 @@ subtest 'Incremented fee tests' => sub { > value => { > notforloan => undef, > rentalcharge => 0, >- rental_charge_daily => 1.000000 >+ rentalcharge_daily => 1.000000 > } > } > )->store; >@@ -3038,7 +3038,7 @@ subtest 'Incremented fee tests' => sub { > } > )->store; > >- is( $itemtype->rental_charge_daily, '1.000000', 'Daily rental charge stored and retreived correctly' ); >+ is( $itemtype->rentalcharge_daily, '1.000000', 'Daily rental charge stored and retreived correctly' ); > is( $item->effective_itemtype, $itemtype->id, "Itemtype set correctly for item"); > > my $dt_from = dt_from_string(); >@@ -3095,3 +3095,60 @@ subtest 'Incremented fee tests' => sub { > $accountlines->delete(); > $issue->delete(); > }; >+ >+subtest 'CanBookBeIssued & RentalFeesCheckoutConfirmation' => sub { >+ plan tests => 2; >+ >+ t::lib::Mocks::mock_preference('RentalFeesCheckoutConfirmation', 1); >+ t::lib::Mocks::mock_preference('item-level_itypes', 1); >+ >+ my $library = >+ $builder->build_object( { class => 'Koha::Libraries' } )->store; >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ )->store; >+ >+ my $itemtype = $builder->build_object( >+ { >+ class => 'Koha::ItemTypes', >+ value => { >+ notforloan => 0, >+ rentalcharge => 0, >+ rentalcharge_daily => 0 >+ } >+ } >+ ); >+ >+ my $biblioitem = $builder->build( { source => 'Biblioitem' } ); >+ my $item = $builder->build_object( >+ { >+ class => 'Koha::Items', >+ value => { >+ homebranch => $library->id, >+ holdingbranch => $library->id, >+ notforloan => 0, >+ itemlost => 0, >+ withdrawn => 0, >+ itype => $itemtype->id, >+ biblionumber => $biblioitem->{biblionumber}, >+ biblioitemnumber => $biblioitem->{biblioitemnumber}, >+ } >+ } >+ )->store; >+ >+ my ( $issuingimpossible, $needsconfirmation ); >+ my $dt_from = dt_from_string(); >+ my $dt_due = dt_from_string()->add( days => 3 ); >+ >+ $itemtype->rentalcharge('1.000000')->store; >+ ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode, $dt_due, undef, undef, undef ); >+ is_deeply( $needsconfirmation, { RENTALCHARGE => '1' }, 'Item needs rentalcharge confirmation to be issued' ); >+ $itemtype->rentalcharge('0')->store; >+ $itemtype->rentalcharge_daily('1.000000')->store; >+ ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode, $dt_due, undef, undef, undef ); >+ is_deeply( $needsconfirmation, { RENTALCHARGE => '3' }, 'Item needs rentalcharge confirmation to be issued, increment' ); >+ $itemtype->rentalcharge_daily('0')->store; >+}; >diff --git a/t/db_dependent/Koha/Charges/Fees.t b/t/db_dependent/Koha/Charges/Fees.t >index 55c324216b..cc555779b5 100644 >--- a/t/db_dependent/Koha/Charges/Fees.t >+++ b/t/db_dependent/Koha/Charges/Fees.t >@@ -58,7 +58,7 @@ my $itemtype = $builder->build_object( > { > class => 'Koha::ItemTypes', > value => { >- rental_charge_daily => '0.00', >+ rentalcharge_daily => '0.00', > rentalcharge => '0.00', > processfee => '0.00', > defaultreplacecost => '0.00', >@@ -99,20 +99,20 @@ my $fees = Koha::Charges::Fees->new( > } > ); > >-subtest 'Koha::ItemType::rental_charge_daily tests' => sub { >+subtest 'accumulate_rentalcharge tests' => sub { > plan tests => 4; > >- $itemtype->rental_charge_daily(1.00); >+ $itemtype->rentalcharge_daily(1.00); > $itemtype->store(); >- is( $itemtype->rental_charge_daily, >+ is( $itemtype->rentalcharge_daily, > 1.00, 'Daily return charge stored correctly' ); > > t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' ); >- my $charge = $fees->rental_charge_daily(); >+ my $charge = $fees->accumulate_rentalcharge(); > is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' ); > > t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' ); >- $charge = $fees->rental_charge_daily(); >+ $charge = $fees->accumulate_rentalcharge(); > is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' ); > > my $calendar = C4::Calendar->new( branchcode => $library->id ); >@@ -121,6 +121,6 @@ subtest 'Koha::ItemType::rental_charge_daily tests' => sub { > title => 'Test holiday', > description => 'Test holiday' > ); >- $charge = $fees->rental_charge_daily(); >+ $charge = $fees->accumulate_rentalcharge(); > is( $charge, 5.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' ); > }; >-- >2.17.2 (Apple Git-113)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20912
:
76065
|
76066
|
76067
|
76068
|
76069
|
77302
|
77303
|
77304
|
81141
|
81142
|
81143
|
81450
|
81771
|
81772
|
81773
|
81774
|
81775
|
81778
|
82195
|
82196
|
84412
|
84413
|
84414
|
84415
|
84416
|
84417
|
84418
|
84419
|
84420
|
84570
|
84571
|
84572
|
84573
|
84574
|
84575
|
84576
|
84577
|
84578
|
84579
|
84580
|
84581
|
84582
|
84583
|
84584
|
84585
|
84610
|
84611
|
84612
|
84613
|
84614
|
84615
|
84616
|
84617
|
84618
|
84619
|
84620
|
84621
|
84622
|
84623
|
84624
|
84625
|
84707
|
84708
|
84709
|
84710
|
84711
|
84712
|
84713
|
84714
|
84715
|
84716
|
84717
|
84718
|
84719
|
84720
|
84721
|
84722
|
84723
|
84724
|
85291
|
85292
|
85293
|
85294
|
85295
|
85296
|
85297
|
85298
|
85299
|
85300
|
85301
|
85302
|
85303
|
85304
|
85305
|
85306
|
85307
|
85308
|
85316
|
85543
|
85544
|
85545
|
85546
|
85547
|
85548
|
85549
|
85550
|
85551
|
85552
|
85553
|
85554
|
85555
|
85556
|
85557
|
85558
|
85559
|
85560
|
85561
|
85628
|
85631
|
86140
|
86141
|
86142
|
86143
|
86144
|
86145
|
86146
|
86147
|
86148
|
86149
|
86151
|
86152
|
86153
|
86154
|
86155
|
86156
|
86157
|
86158
|
86159
|
86160
|
86244
|
86245
|
86246
|
86247
|
86248
|
86249
|
86250
|
86251
|
86252
|
86253
|
86254
|
86255
|
86256
|
86257
|
86258
|
86259
|
86260
|
86261
|
86262
|
86263
|
86264
|
86282
|
86283
|
86284
|
86285
|
86286
|
86287
|
86288
|
86289
|
86290
|
86291
|
86292
|
86293
|
86294
|
86295
|
86296
|
86297
|
86298
|
86299
|
86300
|
86301
|
86302
|
86306
|
86307
|
86308
|
86309
|
86310
|
86311
|
86312
|
86313
|
86314
|
86315
|
86316
|
86317
|
86318
|
86319
|
86320
|
86321
|
86322
|
86323
|
86324
|
86325
|
86326
|
86327
|
87363
|
87413
|
87703