From 3e6b103371d933d48cdb1fa9a10a7aba06f9fa80 Mon Sep 17 00:00:00 2001 From: Josef Moravec Date: Tue, 22 Dec 2020 13:32:03 +0000 Subject: [PATCH] Bug 15516: (follow-up) Move to Koha::Object[s] Test plan: prove t/db_dependent/Koha/HoldGroup.t prove t/db_dependent/Koha/Holds.t Signed-off-by: Michal Denar --- C4/Reserves.pm | 79 +------------- Koha/HoldGroup.pm | 63 +++++++++++ Koha/HoldGroups.pm | 60 +++++++++++ Koha/Holds.pm | 15 +++ Koha/Template/Plugin/Biblio.pm | 8 -- .../prog/en/modules/reserve/hold-group.tt | 8 +- opac/opac-reserve.pl | 3 +- reserve/hold-group.pl | 8 +- reserve/placerequest.pl | 7 +- t/db_dependent/Koha/HoldGroup.t | 53 ++++++++++ t/db_dependent/Koha/Holds.t | 73 ++++++++++++- t/db_dependent/Reserves/HoldCount.t | 116 --------------------- 12 files changed, 277 insertions(+), 216 deletions(-) create mode 100644 Koha/HoldGroup.pm create mode 100644 Koha/HoldGroups.pm create mode 100755 t/db_dependent/Koha/HoldGroup.t delete mode 100755 t/db_dependent/Reserves/HoldCount.t diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 9b5ea084d1..3b9daec0e9 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -38,8 +38,8 @@ use Koha::Calendar; use Koha::CirculationRules; use Koha::Database; use Koha::DateUtils; -use Koha::Hold; use Koha::Holds; +use Koha::HoldGroups; use Koha::ItemTypes; use Koha::Items; use Koha::Libraries; @@ -141,8 +141,6 @@ BEGIN { GetMaxPatronHoldsForRecord - &AddHoldGroup - &DeleteHoldGroup &GetHoldGroupHolds ); @EXPORT_OK = qw( MergeHolds ); @@ -603,48 +601,6 @@ sub CanReserveBeCanceledFromOpac { return $reserve->is_cancelable_from_opac; } -# TODO: Should be moved to Koha::Objects - -=head2 GetHoldCount - - $number = &GetHoldCount($borrowernumber); - -this function returns the number of reservation for a borrower given on input arg. - -=cut - -sub GetHoldCount { - my ($borrowernumber) = @_; - - my $dbh = C4::Context->dbh; - - my $count = 0; #added by 15516 - my $query = " - SELECT COUNT(*) AS counter - FROM reserves - WHERE borrowernumber = ? - AND hold_group_id is NULL"; #added by 15516 - my $sth = $dbh->prepare($query); - $sth->execute($borrowernumber); - my $row = $sth->fetchrow_hashref; - - # section added by 15516 - $count += $row->{counter}; - - $query = " - SELECT COUNT(DISTINCT hold_group_id) AS counter - FROM reserves - WHERE borrowernumber = ? - AND hold_group_id is not NULL - "; - $sth = $dbh->prepare($query); - $sth->execute($borrowernumber); - $row = $sth->fetchrow_hashref; - $count += $row->{counter}; - # end of section added by 15516 - return $count; -} - =head2 GetOtherReserves ($messages,$nextreservinfo)=$GetOtherReserves(itemnumber); @@ -1264,7 +1220,7 @@ sub ModReserveAffect { WHERE hold_group_id = ? AND reserve_id != ? }, undef, $hold->hold_group_id, $hold->reserve_id); - DeleteHoldGroup($hold->hold_group_id); + Koha::HoldGroups->find($hold->hold_group_id)->delete; } _FixPriority( { biblionumber => $biblionumber } ); @@ -2325,37 +2281,6 @@ sub GetMaxPatronHoldsForRecord { return $max; } -=head2 AddHoldGroup - - my $hold_group_id = AddHoldGroup(); - -Creates a new hold group and returns its ID - -=cut - -sub AddHoldGroup { - my $dbh = C4::Context->dbh; - - $dbh->do('INSERT INTO hold_group VALUES ()'); - return $dbh->last_insert_id(undef, undef, 'hold_group', undef); -} - -=head2 DeleteHoldGroup - - DeleteHoldGroup($hold_group_id); - -Delete a hold group. Reserves that belong to this group are not removed. - -=cut - -sub DeleteHoldGroup { - my ($hold_group_id) = @_; - - my $dbh = C4::Context->dbh; - my $query = 'DELETE FROM hold_group WHERE hold_group_id = ?'; - $dbh->do($query, undef, $hold_group_id); -} - =head2 GetHoldGroupHolds my @reserves = GetHoldGroupHolds($hold_group_id); diff --git a/Koha/HoldGroup.pm b/Koha/HoldGroup.pm new file mode 100644 index 0000000000..70c2f9e320 --- /dev/null +++ b/Koha/HoldGroup.pm @@ -0,0 +1,63 @@ +package Koha::HoldGroup; + +# Copyright 2020 Koha Development team +# +# 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 . + +use Modern::Perl; + +use base qw(Koha::Object); + +=head1 NAME + +Koha::HoldGroup - Koha Hold Group object class + +=head1 API + +=head2 Class Methods + +=cut + +=head3 holds + +$holds = $hold_group->holds + +Return all holds associated with this group + +=cut + +sub holds { + my ($self) = @_; + + my $holds_rs = $self->_result->reserves; + return Koha::Holds->_new_from_dbic($holds_rs); +} + +=head3 _type + +=cut + +sub _type { + return 'HoldGroup'; +} + +=head1 AUTHORS + +Josef Moravec + +=cut + +1; diff --git a/Koha/HoldGroups.pm b/Koha/HoldGroups.pm new file mode 100644 index 0000000000..ad96b60704 --- /dev/null +++ b/Koha/HoldGroups.pm @@ -0,0 +1,60 @@ +package Koha::HoldGroups; + +# Copyright Koha Development team 2020 +# +# 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 . + +use Modern::Perl; + +use Koha::Database; + +use Koha::HoldGroup; + +use base qw(Koha::Objects); + +=head1 NAME + +Koha::HoldGroups - Koha Hold Group object set class + +=head1 API + +=head2 Class Methods + +=cut + +=head3 _type + +=cut + +sub _type { + return 'HoldGroup'; +} + +=head3 object_class + +=cut + +sub object_class { + return 'Koha::HoldGroup'; +} + +=head1 AUTHOR + +Josef Moravec + +=cut + +1; diff --git a/Koha/Holds.pm b/Koha/Holds.pm index 604953d4c9..e220533064 100644 --- a/Koha/Holds.pm +++ b/Koha/Holds.pm @@ -92,6 +92,21 @@ sub forced_hold_level { return; } +=head3 count_grouped + +$holds->count_grouped(); + +Return number of hold, where hold group is counted as one hold + +=cut + +sub count_grouped { + my ($self) = @_; + my $holds_without_group_count = $self->search({ hold_group_id => undef })->count(); + my $hold_groups_count = $self->search({ hold_group_id => { '!=' => undef }}, { group_by => 'me.hold_group_id' })->count(); + return $holds_without_group_count + $hold_groups_count; +} + =head3 type =cut diff --git a/Koha/Template/Plugin/Biblio.pm b/Koha/Template/Plugin/Biblio.pm index ab6a716a0e..e9d7a73009 100644 --- a/Koha/Template/Plugin/Biblio.pm +++ b/Koha/Template/Plugin/Biblio.pm @@ -63,12 +63,4 @@ sub CanArticleRequest { return $biblio ? $biblio->can_article_request( $borrower ) : 0; } -sub Get { - my ($self, $biblionumber) = @_; - - my $biblio = Koha::Biblios->find($biblionumber); - - return $biblio; -} - 1; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt index eee53f763e..98ad470b45 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt @@ -1,11 +1,11 @@ -[% USE Biblio %] [% INCLUDE 'doc-head-open.inc' %] - Koha › Circulation › Holds › Holds group ([% hold_group_id | html %]) + Koha › Circulation › Holds › Holds group ([% hold_group.id | html %]) [% INCLUDE 'doc-head-close.inc' %] -

Holds group ([% hold_group_id | html %])

+

Holds group ([% hold_group.id | html %])

+ [% holds = hold_group.holds %] [% IF holds.size %] @@ -17,7 +17,7 @@ [% FOREACH hold IN holds %] - [% biblio = Biblio.Get(hold.biblionumber) %] + [% biblio = hold.biblio %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 252013d590..b071651d10 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -195,8 +195,7 @@ foreach my $biblioNumber (@biblionumbers) { if ( $query->param('place_reserve') ) { my $reserve_cnt = 0; if ($maxreserves) { - # TODO: Should be: $reserve_cnt = $patron->holds->count; - $reserve_cnt = GetHoldCount( $borrowernumber ); + $reserve_cnt = $patron->holds->count_grouped; } # List is composed of alternating biblio/item/branch diff --git a/reserve/hold-group.pl b/reserve/hold-group.pl index 49d33e4198..fd055ef625 100755 --- a/reserve/hold-group.pl +++ b/reserve/hold-group.pl @@ -20,23 +20,21 @@ use CGI qw( -utf8 ); use C4::Auth; use C4::Output; -use C4::Reserves; +use Koha::HoldGroups; my $cgi = new CGI; my ($template, $loggedinuser, $cookie) = get_template_and_user({ template_name => "reserve/hold-group.tt", query => $cgi, type => "intranet", - authnotrequired => 0, flagsrequired => { circulate => 'circulate_remaining_permissions' }, }); my $hold_group_id = $cgi->param('hold_group_id'); -my @holds = GetHoldGroupHolds($hold_group_id); +my $hold_group = Koha::HoldGroups->find($hold_group_id); $template->param( - hold_group_id => $hold_group_id, - holds => \@holds, + hold_group => $hold_group, ); output_html_with_http_headers $cgi, $cookie, $template->output; diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 896a1b673e..a2a8c5788d 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -32,6 +32,7 @@ use C4::Circulation; use C4::Members; use C4::Auth qw/checkauth/; +use Koha::HoldGroups; use Koha::Items; use Koha::Patrons; @@ -77,9 +78,9 @@ my $found; if ( $type eq 'str8' && $borrower ) { - my $hold_group_id; + my $hold_group; if ((@biblionumbers > 1) && $requestany) { - $hold_group_id = AddHoldGroup(); + $hold_group = Koha::HoldGroups->new->store; } foreach my $biblionumber ( keys %bibinfos ) { @@ -144,7 +145,7 @@ if ( $type eq 'str8' && $borrower ) { found => $found, itemtype => $itemtype, non_priority => $non_priority, - hold_group_id => $hold_group_id, + hold_group_id => $hold_group ? $hold_group->id : undef, } ); } diff --git a/t/db_dependent/Koha/HoldGroup.t b/t/db_dependent/Koha/HoldGroup.t new file mode 100755 index 0000000000..4bc4687c79 --- /dev/null +++ b/t/db_dependent/Koha/HoldGroup.t @@ -0,0 +1,53 @@ +#!/usr/bin/perl + +# Copyright 2020 Koha Development team +# +# 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 . + +use Modern::Perl; + +use Test::More tests => 1; + +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'holds tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $hold_group = $builder->build_object({ class => 'Koha::HoldGroups' }); + my $hold = $builder->build_object( + { + class => 'Koha::Holds', + value => { + borrowernumber => $patron->borrowernumber, + hold_group_id => $hold_group->hold_group_id, + } + } + ); + + my $holds = $hold_group->holds; + is( ref($holds), 'Koha::Holds', 'Right type' ); + my $hold_from_group = $holds->next; + is( $hold_from_group->id, $hold->id, 'Right object' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t index 538e7119de..84fe70758f 100755 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 6; use Test::Warn; use C4::Reserves; @@ -410,4 +410,75 @@ subtest 'Desks' => sub { $schema->storage->txn_rollback; +subtest 'count_grouped' => sub { + plan tests => 3; + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ + class => 'Koha::Patrons', + }); + my $patron_id = $patron->borrowernumber; + + my $hold1 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => undef, + }, + }); + + is($patron->holds->count_grouped, 1, 'Test patron has 1 hold.'); + + my $hold_group = $builder->build_object({ + class => 'Koha::HoldGroups', + }); + + my $hold2 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => $hold_group->hold_group_id, + } + }); + my $hold3 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => $hold_group->hold_group_id, + } + }); + + is($patron->holds->count_grouped, 2, 'Test patron has 2 holds.'); + + my $hold_group2 = $builder->build_object({ + class => 'Koha::HoldGroups', + }); + + my $hold4 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => $hold_group2->hold_group_id, + } + }); + my $hold5 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => $hold_group2->hold_group_id, + } + }); + my $hold6 = $builder->build_object({ + class => 'Koha::Holds', + value => { + borrowernumber => $patron_id, + hold_group_id => $hold_group2->hold_group_id, + } + }); + + is($patron->holds->count_grouped, 3, 'Test patron has 3 holds.'); + + $schema->storage->txn_rollback; +}; + 1; diff --git a/t/db_dependent/Reserves/HoldCount.t b/t/db_dependent/Reserves/HoldCount.t deleted file mode 100755 index 7fc5243621..0000000000 --- a/t/db_dependent/Reserves/HoldCount.t +++ /dev/null @@ -1,116 +0,0 @@ -#!/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 . - -use Modern::Perl; - -use Test::More tests => 3; -use t::lib::TestBuilder; - -use C4::Context; -use C4::Biblio; -use C4::Items; -use C4::Reserves; - -use Koha::Database; - - - -my $dbh = C4::Context->dbh; -$dbh->{AutoCommit} = 0; -$dbh->{RaiseError} = 1; - -my $builder = t::lib::TestBuilder->new(); - -my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; -my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; -my $borrower = $builder->build({ - source => 'Borrower', - value => { - branchcode => $branchcode, - categorycode => $categorycode - } -}); -my $borrowernumber = $borrower->{borrowernumber}; - -my $biblio = $builder->build( { source => 'Biblio', } ); -my $biblionumber = $biblio->{biblionumber}; - -# Use AddReserve instead of TestBuilder because i don't manage -# to make TestBuilder set hold_group_id to null. Idea ? -my $reserve_id = AddReserve($branchcode, $borrowernumber, -$biblionumber, $biblionumber, 1, undef, undef, undef -$biblio->{title}, undef, undef, undef); - -is(GetHoldCount($borrowernumber), 1, 'Test borrower has 1 reserve.'); - -my $reserves_group = $builder->build({ - source => 'HoldGroup', - value => { - hold_group_id => 1 - } -}); - -my $reserve2 = $builder->build({ - source => 'Reserve', - value => { - borrowernumber => $borrowernumber, - hold_group_id => 1 - } -}); -my $reserve3 = $builder->build({ - source => 'Reserve', - value => { - borrowernumber => $borrowernumber, - hold_group_id => 1 - } -}); - -is(GetHoldCount($borrowernumber), 2, 'Test borrower has 2 reserve.'); - -my $reserves_group2 = $builder->build({ - source => 'HoldGroup', - value => { - hold_group_id => 2 - } -}); -my $hold_group_id2 = $reserves_group2->{hold_group_id}; - -my $reserve4 = $builder->build({ - source => 'Reserve', - value => { - borrowernumber => $borrowernumber, - hold_group_id => 2 - } -}); -my $reserve5 = $builder->build({ - source => 'Reserve', - value => { - borrowernumber => $borrowernumber, - hold_group_id => 2 - } -}); -my $reserve6 = $builder->build({ - source => 'Reserve', - value => { - borrowernumber => $borrowernumber, - hold_group_id => 2 - } -}); - -is(GetHoldCount($borrowernumber), 3, 'Test borrower has 2 reserve.'); - -$dbh->rollback; -- 2.11.0
[% biblio.title | html %] [% hold.priority | html %]