Bugzilla – Attachment 110073 Details for
Bug 25039
Move new due calculation to Koha::Checkout
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25039: Move new due date calculation to Koha::Checkout
Bug-25039-Move-new-due-date-calculation-to-KohaChe.patch (text/plain), 13.51 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-09-14 17:54:41 UTC
(
hide
)
Description:
Bug 25039: Move new due date calculation to Koha::Checkout
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-09-14 17:54:41 UTC
Size:
13.51 KB
patch
obsolete
>From 226082f8abdc05842e8aa9045c5bb23a0e74dd34 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 1 Apr 2020 16:48:22 -0300 >Subject: [PATCH] Bug 25039: Move new due date calculation to Koha::Checkout > >This is a follow-up patch of bug 25020. > >This patch adds a new method Koha::Checkout->shift_due_date that accepts >the same parameters we provide in the form. It catches bad scenarios >(type errors, passing both parameters when only one is accepted, and so > on). Date manipulation is tested so time is kept and resulting dates >are correct. > >The controller script is cleaned a bit to use the introduced method. > >I do this because: >- We really need tests for this and doing it with selenium is no-end >- I see a use for this new method for encapsulating behaviours, for > example we might want to add Calendar support for the 'days' use case, > and having the method here assures we will have tests, etc. > >To test: >1. Apply this patches >2. Repeat the original test plan >=> SUCCESS: Everything works as expected >3. Run: > $ kshell > k$ prove t/db_dependent/Koha/Checkout.t >=> SUCCESS: Tests pass! >4. Sign off :-D > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/Checkout.pm | 48 +++++++++- > .../modules/tools/batch_extend_due_dates.tt | 18 ++-- > t/db_dependent/Koha/Checkout.t | 88 +++++++++++++++++++ > tools/batch_extend_due_dates.pl | 75 ++++++---------- > 4 files changed, 170 insertions(+), 59 deletions(-) > create mode 100644 t/db_dependent/Koha/Checkout.t > >diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm >index 9180bf4df7..f10acc8177 100644 >--- a/Koha/Checkout.pm >+++ b/Koha/Checkout.pm >@@ -22,11 +22,13 @@ use Modern::Perl; > > use Carp; > use DateTime; >+use Scalar::Util qw( looks_like_number ); > use Try::Tiny; > > use Koha::Checkouts::ReturnClaims; > use Koha::Database; >-use Koha::DateUtils; >+use Koha::DateUtils qw(dt_from_string); >+use Koha::Exceptions; > use Koha::Items; > > use base qw(Koha::Object); >@@ -162,6 +164,50 @@ sub claim_returned { > }; > } > >+=head3 shift_due_date >+ >+ $checkout->shift_due_date({ hard_due_date => $dt }); >+ $checkout->shift_due_date({ days => $days_count }); >+ >+=cut >+ >+sub shift_due_date { >+ my ($self, $params) = @_; >+ >+ my $hard_due_date = $params->{hard_due_date}; >+ my $days = $params->{days}; >+ >+ if ( $hard_due_date and $days ) { >+ Koha::Exceptions::BadParameter->throw( "Called with both 'days' and 'hard_due_date'. Only one can be used." ); >+ } >+ >+ if ( !( $days or $hard_due_date ) ) { >+ Koha::Exceptions::BadParameter->throw( "At least one parameter needs to be passed" ); >+ } >+ >+ my $due_date = dt_from_string($self->date_due); >+ if ( $hard_due_date ) { >+ # We require a DateTime >+ Koha::Exceptions::WrongParameter->throw( >+ "'hard_due_date' must be a DateTime object" >+ ) unless $hard_due_date->isa('DateTime'); >+ >+ $due_date = $hard_due_date->clone->set( >+ hour => $due_date->hour, >+ minute => $due_date->minute, >+ second => $due_date->second >+ ); >+ } >+ else { >+ Koha::Exceptions::WrongParameter->throw( >+ "'days' must be an integer" >+ ) unless looks_like_number($days); >+ $due_date->add( days => $days ); >+ } >+ >+ return $self->date_due( $due_date ); >+} >+ > =head2 Internal methods > > =head3 _type >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_extend_due_dates.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_extend_due_dates.tt >index f3d8f4ad6a..b35140a5e1 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_extend_due_dates.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_extend_due_dates.tt >@@ -114,7 +114,7 @@ > </fieldset> > </form> <!-- /#extend_due_dates_form --> > [% ELSIF view == 'list' %] >- [% IF checkouts.count %] >+ [% IF checkouts.size > 0 %] > <form action="/cgi-bin/koha/tools/batch_extend_due_dates.pl" method="post" id="process"> > <div id="toolbar"> > <a id="selectall" href="#"><i class="fa fa-check"></i> Select all</a> >@@ -136,15 +136,15 @@ > <tbody> > [% FOR checkout IN checkouts %] > <tr> >- <td><input type="checkbox" name="issue_id" value="[% checkout.issue_id | html %]" /></td> >- <td>[% checkout.date_due | $KohaDates as_due_date => 1 %]</td> >- <td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% checkout.item.biblio.biblionumber | uri %]">[% checkout.item.biblio.title | html %]</a></td> >- <td>[% ItemTypes.GetDescription( checkout.item.effective_itemtype ) | html %]</td> >- <td>[% checkout.item.home_branch.branchname | html %]</td> >- <td>[% checkout.issuedate | $KohaDates %]</td> >- <td>[% Branches.GetName( checkout.branchcode ) | html %]</td> >+ <td><input type="checkbox" name="issue_id" value="[% checkout.current.issue_id | html %]" /></td> >+ <td>[% checkout.current.date_due | $KohaDates as_due_date => 1 %]</td> >+ <td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% checkout.current.item.biblio.biblionumber | uri %]">[% checkout.current.item.biblio.title | html %]</a></td> >+ <td>[% ItemTypes.GetDescription( checkout.current.item.effective_itemtype ) | html %]</td> >+ <td>[% checkout.current.item.home_branch.branchname | html %]</td> >+ <td>[% checkout.current.issuedate | $KohaDates %]</td> >+ <td>[% Branches.GetName( checkout.current.branchcode ) | html %]</td> > <td> >- [% new_due_dates.shift | $KohaDates as_due_date => 1 %] >+ [% checkout.updated.date_due | $KohaDates as_due_date => 1 %] > </td> > </tr> > [% END %] >diff --git a/t/db_dependent/Koha/Checkout.t b/t/db_dependent/Koha/Checkout.t >new file mode 100644 >index 0000000000..62f532cc95 >--- /dev/null >+++ b/t/db_dependent/Koha/Checkout.t >@@ -0,0 +1,88 @@ >+#!/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::Exception; >+ >+use t::lib::TestBuilder; >+ >+use Koha::DateUtils qw(dt_from_string output_pref); >+ >+my $schema = Koha::Database->new->schema; >+my $builder = t::lib::TestBuilder->new; >+ >+subtest 'shift_due_date() tests' => sub { >+ >+ plan tests => 10; >+ >+ $schema->storage->txn_begin; >+ >+ my $checkout = $builder->build_object( >+ { >+ class => 'Koha::Checkouts', >+ value => { >+ date_due => '2020-04-01 13:06:16' >+ } >+ } >+ ); >+ >+ throws_ok >+ { $checkout->shift_due_date({ hard_due_date => 'cat', days => 'dog' }) } >+ 'Koha::Exceptions::BadParameter', >+ 'Passing both params makes it raise an exception'; >+ >+ is( "$@", "Called with both 'days' and 'hard_due_date'. Only one can be used.", 'Exception stringified correctly' ); >+ >+ throws_ok >+ { $checkout->shift_due_date({ }) } >+ 'Koha::Exceptions::BadParameter', >+ 'Passing no params makes it raise an exception'; >+ >+ is( "$@", "At least one parameter needs to be passed", 'Exception stringified correctly' ); >+ >+ throws_ok >+ { $checkout->shift_due_date({ hard_due_date => 'cat' }) } >+ 'Koha::Exceptions::WrongParameter', >+ 'Passing an invalid param type makes it raise an exception'; >+ >+ is( "$@", "'hard_due_date' must be a DateTime object", 'Exception stringified correctly' ); >+ >+ throws_ok >+ { $checkout->shift_due_date({ days => 'dog' }) } >+ 'Koha::Exceptions::WrongParameter', >+ 'Passing an invalid param type makes it raise an exception'; >+ >+ is( "$@", "'days' must be an integer", 'Exception stringified correctly' ); >+ >+ my $hard_due_date = dt_from_string( '2020-04-13' ); >+ $checkout->shift_due_date({ hard_due_date => $hard_due_date }); >+ >+ is( >+ output_pref({ dt => dt_from_string($checkout->date_due) }), >+ output_pref({ dt => dt_from_string('2020-04-13 13:06:16') }) >+ ); >+ >+ $checkout->shift_due_date({ days => 3 }); >+ is( >+ output_pref({ dt => dt_from_string($checkout->date_due) }), >+ output_pref({ dt => dt_from_string('2020-04-16 13:06:16') }) >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >diff --git a/tools/batch_extend_due_dates.pl b/tools/batch_extend_due_dates.pl >index cae338c07e..218c592041 100755 >--- a/tools/batch_extend_due_dates.pl >+++ b/tools/batch_extend_due_dates.pl >@@ -103,37 +103,31 @@ elsif ( $op eq 'list' ) { > ); > > my @new_due_dates; >+ my @checkouts; >+ > while ( my $checkout = $checkouts->next ) { >+ my $updated_checkout = $checkout->get_from_storage; > if ($preview_results) { >- push( >- @new_due_dates, >- output_pref( >- { >- dt => calc_new_due_date( >- { >- due_date => >- dt_from_string( $checkout->date_due ), >- new_hard_due_date => $new_hard_due_date, >- add_days => $due_date_days >- } >- ), >- dateformat => 'iso' >- } >- ) >- ); >+ if ( $new_hard_due_date ) { >+ $updated_checkout->shift_due_date({ hard_due_date => $new_hard_due_date }); >+ } >+ else { >+ $updated_checkout->shift_due_date({ days => $due_date_days }); >+ } > } else { > push( @issue_ids, $checkout->id ); > } >+ >+ push @checkouts, { current => $checkout, updated => $updated_checkout }; > } > > if ( $preview_results ) { > $template->param( >- checkouts => $checkouts, >+ checkouts => \@checkouts, > new_hard_due_date => $new_hard_due_date > ? dt_from_string($new_hard_due_date) > : undef, > due_date_days => $due_date_days, >- new_due_dates => \@new_due_dates, > view => 'list', > ); > } else { >@@ -151,47 +145,30 @@ if ( $op eq 'modify' ) { > @issue_ids = $input->multi_param('issue_id') unless @issue_ids; > > $new_hard_due_date &&= dt_from_string($new_hard_due_date); >- my $checkouts = >- Koha::Checkouts->search( { issue_id => { -in => \@issue_ids } } ); >+ my @checkouts; >+ my $checkouts = Koha::Checkouts->search( { issue_id => { -in => \@issue_ids } } ); > while ( my $checkout = $checkouts->next ) { >- my $new_due_date = calc_new_due_date( >- { >- due_date => dt_from_string($checkout->date_due), >- new_hard_due_date => $new_hard_due_date, >- add_days => $due_date_days >- } >- ); > > # Update checkout's due date >- $checkout->date_due($new_due_date)->store; >+ if ( $new_hard_due_date ) { >+ $checkout->shift_due_date({ hard_due_date => $new_hard_due_date }); >+ } >+ else { >+ $checkout->shift_due_date({ days => $due_date_days }); >+ } > > # Update items.onloan >- $checkout->item->onloan($new_due_date)->store; >+ $checkout->item->onloan($checkout->date_due)->store; >+ >+ push @checkouts, $checkout; > } > > $template->param( > view => 'report', >- checkouts => $checkouts, >+ checkouts => \@checkouts, > ); > } > >-sub calc_new_due_date { >- my ($params) = @_; >- my $due_date = $params->{due_date}; >- my $new_hard_due_date = $params->{new_hard_due_date}; >- my $add_days = $params->{add_days}; >- >- my $new; >- if ( $new_hard_due_date ) { >- $new = $new_hard_due_date->clone->set( >- hour => $due_date->hour, >- minute => $due_date->minute, >- second => $due_date->second, >- ) >- } else { >- $new = $due_date->clone->add( days => $add_days ); >- } >- return $new; >-} >- > output_html_with_http_headers $input, $cookie, $template->output; >+ >+1; >-- >2.28.0
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 25039
:
102273
|
110073
|
110460
|
110483
|
110484
|
110795
|
110796
|
110905