From a447df209bce6e062a7d406de48f1f0e62eb4662 Mon Sep 17 00:00:00 2001 From: Nicolas Legrand Date: Wed, 8 Jul 2020 11:02:16 +0200 Subject: [PATCH] Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl. Plan test: 1. run `prove t/db_dependent/Koha/Item/RelatedObjects.t ; prove t/db_dependent/Koha/Holds.pm`, all should be ok 2. apply patch 3. rerun `prove t/db_dependent/Koha/Item/RelatedObjects.t ; prove t/db_dependent/Koha/Holds.pm`, all should be ok 4. make a hold on a checkouted item and check it in. It should be in waiting reserves now. 5. go have a look to /reserve/request.pl, circ/waitingreserves.pl, koha/opac-user.pl (while logged as the user having the hold) pages. 6. it should be errorless, you should see the library names and the forms should prompt the correct branchcode. --- Koha/Hold.pm | 28 +++++------ Koha/Item.pm | 18 ++++--- .../intranet-tmpl/prog/en/includes/holds_table.inc | 2 +- .../prog/en/includes/waiting_holds.inc | 6 +-- .../bootstrap/en/includes/holds-table.inc | 6 +-- t/db_dependent/Koha/Holds.t | 15 +++++- t/db_dependent/Koha/Item/RelatedObjects.t | 55 ++++++++++++++++++++++ 7 files changed, 96 insertions(+), 34 deletions(-) create mode 100644 t/db_dependent/Koha/Item/RelatedObjects.t diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 8969e32..70426ef 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -298,11 +298,10 @@ Returns the related Koha::Item object for this Hold =cut sub item { - my ($self) = @_; - - $self->{_item} ||= Koha::Items->find( $self->itemnumber() ); - - return $self->{_item}; + my $self = shift; + my $item_rs = $self->_result->item; + return unless $item_rs; + return Koha::Item->_new_from_dbic($item_rs); } =head3 branch @@ -311,12 +310,12 @@ Returns the related Koha::Library object for this Hold =cut +# FIXME Should be renamed with ->library sub branch { - my ($self) = @_; - - $self->{_branch} ||= Koha::Libraries->find( $self->branchcode() ); - - return $self->{_branch}; + my $self = shift; + my $branch_rs = $self->_result->branchcode; + return unless $branch_rs; + return Koha::Library->_new_from_dbic($branch_rs); } =head3 borrower @@ -327,11 +326,10 @@ Returns the related Koha::Patron object for this Hold # FIXME Should be renamed with ->patron sub borrower { - my ($self) = @_; - - $self->{_borrower} ||= Koha::Patrons->find( $self->borrowernumber() ); - - return $self->{_borrower}; + my $self = shift; + my $patron_rs = $self->_result->borrowernumber; + return unless $patron_rs; + return Koha::Patron->_new_from_dbic($patron_rs); } =head3 is_suspended diff --git a/Koha/Item.pm b/Koha/Item.pm index 9560a56..2ecbb8f 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -301,11 +301,10 @@ sub effective_itemtype { =cut sub home_branch { - my ($self) = @_; - - $self->{_home_branch} ||= Koha::Libraries->find( $self->homebranch() ); - - return $self->{_home_branch}; + my $self = shift; + my $home_branch_rs = $self->_result->homebranch; + return unless $home_branch_rs; + return Koha::Library->_new_from_dbic($home_branch_rs); } =head3 holding_branch @@ -313,11 +312,10 @@ sub home_branch { =cut sub holding_branch { - my ($self) = @_; - - $self->{_holding_branch} ||= Koha::Libraries->find( $self->holdingbranch() ); - - return $self->{_holding_branch}; + my $self = shift; + my $holding_branch_rs = $self->_result->holdingbranch; + return unless $holding_branch_rs; + return Koha::Library->_new_from_dbic($holding_branch_rs); } =head3 biblio diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index 600977a..2046152 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -128,7 +128,7 @@ [% END %] [% ELSE %] [% IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 %] - [% Branches.GetName(hold.branchcode) | html %] + [% hold.branch.branchname | html %] [% ELSE %] [% IF ( reserveloo.item.homebranch != reserveloo.item.holdingbranch ) %] - + [% ELSE %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc index 610c968..d191163 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc @@ -112,12 +112,12 @@ until [% HOLD.expirationdate | $KohaDates %] [% END %] [% END %] - + [% ELSE %] - Item waiting to be pulled from [% Branches.GetName( HOLD.branchcode ) | html %] + Item waiting to be pulled from [% HOLD.branch.branchname | html %] [% END %] [% ELSE %] - Item in transit to [% Branches.GetName( HOLD.branchcode ) | html %] + Item in transit to [% HOLD.branch.branchname | html %] [% END %] [% ELSE %] [% IF ( HOLD.is_in_transit ) %] diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t index 055dd18..6a8a4b4 100644 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -35,7 +35,7 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; subtest 'DB constraints' => sub { - plan tests => 1; + plan tests => 2; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); my $item = $builder->build_sample_item; @@ -55,7 +55,18 @@ subtest 'DB constraints' => sub { eval { $hold->priority(undef)->store } } qr{.*DBD::mysql::st execute failed: Column 'priority' cannot be null.*}, - 'DBD should have raised an error about priority that cannot be null'; + 'DBD should have raised an error about priority that cannot be null'; + + subtest 'Related objects' => sub { + plan tests => 3; + my $item = $hold->item; + is(ref($item), 'Koha::Item', 'Returns a Koha::Item resultset' ); + my $patron = $hold->borrower; + is(ref($patron), 'Koha::Patron', 'Returns a Koha::Patron resultset' ); + my $library = $hold->branch; + is(ref($library), 'Koha::Library', 'Returns a Koha::Branch resultset' ); + }; + }; subtest 'cancel' => sub { diff --git a/t/db_dependent/Koha/Item/RelatedObjects.t b/t/db_dependent/Koha/Item/RelatedObjects.t new file mode 100644 index 0000000..fff4119 --- /dev/null +++ b/t/db_dependent/Koha/Item/RelatedObjects.t @@ -0,0 +1,55 @@ +#!/usr/bin/perl + +# Copyright 2020 BULAC +# +# 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 => 2; + +use C4::Biblio; +use C4::Circulation; + +use Koha::Items; +use Koha::Database; +use Koha::Old::Items; + +use List::MoreUtils qw(all); + +use t::lib::TestBuilder; +use t::lib::Mocks; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + + +$schema->storage->txn_begin; +my $library = $builder->build( { source => 'Branch' } ); +my $nb_of_items = Koha::Items->search->count; +my $biblio = $builder->build_sample_biblio(); +my $new_item = $builder->build_sample_item({ + biblionumber => $biblio->biblionumber, + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + }); +my $home_branch = $new_item->home_branch; +is(ref($home_branch), 'Koha::Library', 'Returns a Koha::Library resultset' ); +my $holding_branch = $new_item->holding_branch; +is(ref($holding_branch), 'Koha::Library', 'Returns a Koha::Library resultset' ); +$schema->storage->txn_rollback; + + -- 2.1.4