Attachment 106648 Details for Bug 24412 – Bug 24412: (follow-up) QA

[patch] Bug 24412: (follow-up) QA

Bug-24412-follow-up-QA.patch (text/plain), 8.05 KB, created by Nicolas Legrand on 2020-07-08 08:02:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nicolas Legrand

Created: 2020-07-08 08:02:48 UTC

Size: 8.05 KB

patch

obsolete

>From beeebdbf821e5ced39ed7871d81cfa79b4aa9917 Mon Sep 17 00:00:00 2001
>From: Nicolas Legrand <nicolas.legrand@bulac.fr>
>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 %]
> <table>
>     <tr>
>         [% 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 %]
> <table id="[% table_name | html %]">
>     <thead>
>@@ -39,7 +38,7 @@
>                     [% END %]
>                 </td>
>                 <td>[% Branches.GetName( reserveloo.item.homebranch ) | html %]</td>
>-                <td>[% Branches.GetName( reserveloo.item.holdingbranch ) | html %][% IF (reserveloo.desk_id  ) %], [% Desks.GetName (reserveloo.desk_id) | html %][% END %]</td>
>+                <td>[% Branches.GetName( reserveloo.item.holdingbranch ) | html %][% IF (reserveloo.desk_id  ) %], [% reserveloo.desk.desk_name | html %][% END %]</td>
>                 <td>[% AuthorisedValues.GetByCode('LOC', reserveloo.item.location) | html %]</td>
>                 <td>[% reserveloo.item.itemcallnumber | html %]</td>
>                 <td>[% reserveloo.item.copynumber | html %]</td>
>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 %]
>-                                <span class="waitingat">Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% Desks.GetName ( hold.desk_id ) | html %][% END %] since [% hold.waitingdate | $KohaDates %].</span>
>+                                <span class="waitingat">Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% hold.desk.desk_name | html %][% END %] since [% hold.waitingdate | $KohaDates %].</span>
>                                 [% IF canreservefromotherbranches AND ( hold.waitingdate OR hold.priority == 1 ) %]
>                                     <span class="heldfor">Hold for:</span>
>                                     [% 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 <strong> [% HOLD.branch.branchname | html %]</strong>[% IF ( HOLD.desk_id ) %], [% Desks.GetName ( HOLD.desk_id ) | html %],[% END %]
>+                                        Item waiting at <strong> [% HOLD.branch.branchname | html %]</strong>[% IF ( HOLD.desk_id ) %], [% HOLD.desk.desk_name | html %],[% END %]
> 
>                                         [% IF ( HOLD.waitingdate ) %]
>                                             since [% HOLD.waitingdate | $KohaDates %]
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 24412: 97328 | 97329 | 97330 | 101625 | 101626 | 101627 | 101628 | 101629 | 101630 | 101721 | 101722 | 101723 | 101825 | 101884 | 101885 | 101886 | 101898 | 101899 | 101900 | 101955 | 101956 | 101957 | 106586 | 106648 | 110067 | 110068 | 110069 | 110070 | 110450 | 110451 | 110452 | 110453 | 110454 | 110455 | 110582 | 110583 | 110584 | 110585 | 110586 | 112046 | 112047 | 112048 | 112049 | 112050 | 112092 | 112099 | 112165 | 112166 | 112167 | 112168 | 112169 | 112170 | 112171 | 112172 | 112201 | 112202 | 112203 | 112204 | 112205 | 112206 | 112207 | 112208 | 112209 | 112210 | 112211 | 112212 | 112213 | 112214 | 112215 | 112216 | 112217 | 112218 | 112219 | 112264 | 112265 | 112266 | 112267 | 112268 | 112269 | 112270 | 112271 | 112272 | 112292 | 112293 | 113108 | 113109 | 113116 | 113228 | 113229 | 113230 | 113231 | 113232 | 113233 | 113234 | 113235 | 113236