From beeebdbf821e5ced39ed7871d81cfa79b4aa9917 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 --- Koha/Hold.pm | 11 ++++------ Koha/Template/Plugin/Desks.pm | 14 ------------ .../bug_24412_Attach_waiting_reserve_to_desk.perl | 25 +++++++++++----------- .../intranet-tmpl/prog/en/includes/holds_table.inc | 1 - .../prog/en/includes/waiting_holds.inc | 3 +-- .../prog/en/modules/catalogue/detail.tt | 3 +-- .../bootstrap/en/includes/holds-table.inc | 3 +-- 7 files changed, 20 insertions(+), 40 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 1f79e29..617ca42 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -172,8 +172,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); @@ -329,11 +327,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 02af1b3..4216497 100644 --- a/Koha/Template/Plugin/Desks.pm +++ b/Koha/Template/Plugin/Desks.pm @@ -37,20 +37,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/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index de74f6c..53974e9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -1,5 +1,4 @@ [% USE Koha %] -[% USE Desks %] [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] 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 7f68262..eacacd9 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 a998750..b3d4c12 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' %] @@ -385,7 +384,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 e24d888..7b3c3ab 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.1.4
[% 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 %]