From 4038620dfd07dd63a0926bce7a6ac28c398c248d 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Séverine QUEUNE --- Koha/Hold.pm | 11 ++++------ Koha/Template/Plugin/Desks.pm | 14 ------------ .../bug_24412_Attach_waiting_reserve_to_desk.perl | 25 +++++++++++----------- .../prog/en/includes/waiting_holds.inc | 1 - .../prog/en/modules/catalogue/detail.tt | 3 +-- .../bootstrap/en/includes/holds-table.inc | 1 - 6 files changed, 18 insertions(+), 37 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 522bfc1..0472a4e 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -186,8 +186,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); @@ -375,11 +373,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 0817948..58e1001 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 c743eab..9242ffb 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 5c7922d..a7ef638 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 %] 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 45f60f3..09b6ddb 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' %] @@ -461,7 +460,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 1bb3739..b4068f3 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 %] [% PROCESS 'i18n.inc' %] -- 2.1.4