From 754a907646c7219280274c31e60ac85804bb1c6a Mon Sep 17 00:00:00 2001 From: Nicolas Legrand Date: Wed, 8 Jul 2020 09:52:51 +0200 Subject: [PATCH] Bug 24412: (follow-up) QA Following Josef Moravec QA comments : - rewrite Koha::Hold->desk according to Object Oriented Koha Guidelines and use it to fetch desk name in various templates - remove unused Desks.GetName - Check for columns existence in db update Signed-off-by: Martin Renvoize --- Koha/Hold.pm | 11 +++----- Koha/Template/Plugin/Desks.pm | 14 ----------- ..._24412_Attach_waiting_reserve_to_desk.perl | 25 ++++++++++--------- .../prog/en/includes/waiting_holds.inc | 3 +-- .../prog/en/modules/catalogue/detail.tt | 3 +-- .../bootstrap/en/includes/holds-table.inc | 3 +-- 6 files changed, 20 insertions(+), 39 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index fe1745bb61..81dfeab24e 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -174,8 +174,6 @@ sub set_waiting { desk_id => $desk_id, }; - if (defined $desk_id) { $values->{'desk_id'} = $desk_id }; - my $requested_expiration; if ($self->expirationdate) { $requested_expiration = dt_from_string($self->expirationdate); @@ -331,11 +329,10 @@ Returns the related Koha::Library object for this Hold =cut sub desk { - my ($self) = @_; - - $self->{_desk} ||= Koha::Desks->find( $self->desk_id() ); - - return $self->{_desk}; + my $self = shift; + my $desk_rs = $self->_result->desk; + return unless $desk_rs; + return Koha::Desk->_new_from_dbic($desk_rs); } =head3 borrower diff --git a/Koha/Template/Plugin/Desks.pm b/Koha/Template/Plugin/Desks.pm index 0817948fcd..58e100163d 100644 --- a/Koha/Template/Plugin/Desks.pm +++ b/Koha/Template/Plugin/Desks.pm @@ -36,20 +36,6 @@ got or the current one. =head2 Methods -=head3 GetName - -[% Desk.GetName(desk_id) %] - -return desk name or empty string - -=cut - -sub GetName { - my ( $self, $desk_id ) = @_; - my $d = Koha::Desks->find( $desk_id ); - return (defined $d) ? $d->unblessed->{'desk_name'} : q{}; -} - =head3 GetLoggedInDeskId [% Desks.GetLoggedInDeskId %] diff --git a/installer/data/mysql/atomicupdate/bug_24412_Attach_waiting_reserve_to_desk.perl b/installer/data/mysql/atomicupdate/bug_24412_Attach_waiting_reserve_to_desk.perl index c743eab7f3..9242ffbd3d 100644 --- a/installer/data/mysql/atomicupdate/bug_24412_Attach_waiting_reserve_to_desk.perl +++ b/installer/data/mysql/atomicupdate/bug_24412_Attach_waiting_reserve_to_desk.perl @@ -1,16 +1,17 @@ $DBversion = 'XXX'; # will be replaced by the RM if( CheckVersion( $DBversion ) ) { - # you can use $dbh here like: - $dbh->do(q{ -ALTER TABLE reserves ADD COLUMN desk_id INT(11) DEFAULT NULL AFTER branchcode, - ADD KEY desk_id (`desk_id`), - ADD CONSTRAINT `reserves_ibfk_6` FOREIGN KEY (`desk_id`) REFERENCES `desks` (`desk_id`) ON DELETE CASCADE ON UPDATE CASCADE ; - }); - $dbh->do(q{ -ALTER TABLE old_reserves ADD COLUMN desk_id INT(11) DEFAULT NULL AFTER branchcode, - ADD KEY `old_desk_id` (`desk_id`); - }); + if ( !column_exists( 'reserves', 'desk_id' ) ) { + $dbh->do(q{ + ALTER TABLE reserves ADD COLUMN desk_id INT(11) DEFAULT NULL AFTER branchcode, + ADD KEY desk_id (`desk_id`), + ADD CONSTRAINT `reserves_ibfk_6` FOREIGN KEY (`desk_id`) REFERENCES `desks` (`desk_id`) ON DELETE CASCADE ON UPDATE CASCADE ; + }); + $dbh->do(q{ + ALTER TABLE old_reserves ADD COLUMN desk_id INT(11) DEFAULT NULL AFTER branchcode, + ADD KEY `old_desk_id` (`desk_id`); + }); - SetVersion( $DBversion ); - print "Upgrade to $DBversion done (Bug XXXXX - Attach waiting reserve to desk)\n"; + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug XXXXX - Attach waiting reserve to desk)\n"; + } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc index 850368a289..b5b1a1863a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -1,5 +1,4 @@ [% USE ItemTypes %] -[% USE Desks %] [% USE AuthorisedValues %] @@ -39,7 +38,7 @@ [% END %] - + diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index d16e1ac1e0..8f7c52f708 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -4,7 +4,6 @@ [% USE KohaDates %] [% USE AuthorisedValues %] [% USE Branches %] -[% USE Desks %] [% USE Biblio %] [% USE TablesSettings %] [% PROCESS 'i18n.inc' %] @@ -444,7 +443,7 @@ Note that permanent location is a code, and location may be an authval. [% SET hold = item.first_hold %] [% IF hold %] [% IF hold.waitingdate %] - Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% Desks.GetName ( hold.desk_id ) | html %][% END %] since [% hold.waitingdate | $KohaDates %]. + Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% hold.desk.desk_name | html %][% END %] since [% hold.waitingdate | $KohaDates %]. [% IF canreservefromotherbranches AND ( hold.waitingdate OR hold.priority == 1 ) %] Hold for: [% INCLUDE 'patron-title.inc' patron=hold.borrower hide_patron_infos_if_needed=1 %] 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 20c3a56922..5ebdf82da9 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc @@ -1,5 +1,4 @@ [% USE Branches %] -[% USE Desks %] [% USE ItemTypes %] [% USE KohaDates %] @@ -107,7 +106,7 @@ [% IF ( HOLD.is_at_destination ) %] [% IF ( HOLD.found ) %] - Item waiting at [% HOLD.branch.branchname | html %][% IF ( HOLD.desk_id ) %], [% Desks.GetName ( HOLD.desk_id ) | html %],[% END %] + Item waiting at [% HOLD.branch.branchname | html %][% IF ( HOLD.desk_id ) %], [% HOLD.desk.desk_name | html %],[% END %] [% IF ( HOLD.waitingdate ) %] since [% HOLD.waitingdate | $KohaDates %] -- 2.20.1
[% Branches.GetName( reserveloo.item.homebranch ) | html %][% Branches.GetName( reserveloo.item.holdingbranch ) | html %][% IF (reserveloo.desk_id ) %], [% Desks.GetName (reserveloo.desk_id) | html %][% END %][% Branches.GetName( reserveloo.item.holdingbranch ) | html %][% IF (reserveloo.desk_id ) %], [% reserveloo.desk.desk_name | html %][% END %] [% AuthorisedValues.GetByCode('LOC', reserveloo.item.location) | html %] [% reserveloo.item.itemcallnumber | html %] [% reserveloo.item.copynumber | html %]