Attachment 109124 Details for Bug 22789 – Bug 22789: (follow-up) Fix atomic update, GUI and more than one hold

[patch] Bug 22789: (follow-up) Fix atomic update, GUI and more than one hold

Bug-22789-follow-up-Fix-atomic-update-GUI-and-more.patch (text/plain), 9.24 KB, created by Agustín Moyano on 2020-08-25 18:12:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Agustín Moyano

Created: 2020-08-25 18:12:54 UTC

Size: 9.24 KB

patch

obsolete

>From 8c69a8bca63f86e01f2bce6608f0f26efdf3dfe6 Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Tue, 25 Aug 2020 14:53:55 -0300
>Subject: [PATCH] Bug 22789: (follow-up) Fix atomic update, GUI and more than
> one hold
>
>This patch
>* sets one check for reserves and another for old_reserves in
>atomic update
>* Adds a message below the checkbox and adds detail when a hold is non
>  priority
>* Fixes issue when there are more than one hold, but the first is non
>  priority
>* Adds test case for this last scenario
>---
> C4/Circulation.pm                             |  9 ++-
> .../data/mysql/atomicupdate/bug_22789.perl    |  3 +
> .../prog/en/includes/holds_table.inc          | 57 ++++++++++++-------
> .../prog/en/modules/reserve/request.tt        |  3 +-
> reserve/request.pl                            |  1 +
> t/db_dependent/Holds.t                        | 26 ++++++++-
> 6 files changed, 75 insertions(+), 24 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 7572bd12cd..3571051e84 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2761,7 +2761,14 @@ sub CanBookBeRenewed {
> 
>     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
> 
>-    $resfound = 0 if $resrec->{non_priority};
>+    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
>+    if ( $resfound && $resrec->{non_priority} ) {
>+        $resfound = Koha::Holds->search(
>+            { biblionumber => $resrec->{biblionumber}, non_priority => 0 } )
>+          ->count > 0;
>+    }
>+
>+
> 
>     # This item can fill one or more unfilled reserve, can those unfilled reserves
>     # all be filled by other available items?
>diff --git a/installer/data/mysql/atomicupdate/bug_22789.perl b/installer/data/mysql/atomicupdate/bug_22789.perl
>index 1986b14ba6..eb75263f66 100644
>--- a/installer/data/mysql/atomicupdate/bug_22789.perl
>+++ b/installer/data/mysql/atomicupdate/bug_22789.perl
>@@ -6,6 +6,9 @@ if( CheckVersion( $DBversion ) ) {
>     # or perform some test and warn
>     if( !column_exists( 'reserves', 'non_priority' ) ) {
>         $dbh->do("ALTER TABLE reserves ADD COLUMN `non_priority` tinyint(1) NOT NULL DEFAULT 0 AFTER `item_level_hold` -- Is this a non priority hold");
>+    }
>+
>+    if( !column_exists( 'old_reserves', 'non_priority' ) ) {
>         $dbh->do("ALTER TABLE old_reserves ADD COLUMN `non_priority` tinyint(1) NOT NULL DEFAULT 0 AFTER `item_level_hold` -- Is this a non priority hold");
>     }
> 
>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 26791f819d..01dfed3171 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>@@ -140,38 +140,53 @@
>             </td>
> 
>             <td>
>+                <ul>
>                 [% IF ( hold.found ) %]
>-                    <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&amp;itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]">
>-                        [% IF ( hold.barcodenumber ) %]
>-                            [% hold.barcodenumber | html %]
>-                            <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" />
>-                        [% ELSE %]
>-                            No barcode
>-                        [% END %]
>-                    </a>
>+                    <li>
>+                        <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&amp;itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]">
>+                            [% IF ( hold.barcodenumber ) %]
>+                                [% hold.barcodenumber | html %]
>+                                <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" />
>+                            [% ELSE %]
>+                                No barcode
>+                            [% END %]
>+                        </a>
>+                    </li>
>                 [% ELSE %]
>                     [% IF ( hold.item_level_hold ) %]
>-                        <i>
>-                            Only item
>-                            <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&amp;itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]">
>-                                [% IF ( hold.barcodenumber ) %]
>-                                    [% hold.barcodenumber | html %]
>-                                    <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" />
>-                                [% ELSE %]
>-                                    No barcode
>-                                [% END %]
>-                            </a>
>-                        </i>
>+                        <li>
>+                            <i>
>+                                Only item
>+                                <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&amp;itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]">
>+                                    [% IF ( hold.barcodenumber ) %]
>+                                        [% hold.barcodenumber | html %]
>+                                        <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" />
>+                                    [% ELSE %]
>+                                        No barcode
>+                                    [% END %]
>+                                </a>
>+                            </i>
>+                        </li>
>                     [% ELSE %]
>                         [% IF hold.itemtype %]
>-                            <i>Next available [% ItemTypes.GetDescription( hold.itemtype ) | html %] item</i>
>+                            <li>
>+                                <i>Next available [% ItemTypes.GetDescription( hold.itemtype ) | html %] item</i>
>+                            </li>
>                         [% ELSE %]
>-                            <i>Next available</i>
>+                            <li>
>+                                <i>Next available</i>
>+                            </li>
>                         [% END %]
> 
>                         <input type="hidden" name="itemnumber" value="" />
>                     [% END %]
>                 [% END %]
>+                [% IF hold.non_priority %]
>+                    <li>
>+                        <i>Non priority hold</i>
>+                    </li>
>+                [% END %]
>+                </ul>
>             </td>
> 
>             [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>index 820543d509..36ca0a538f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -477,9 +477,10 @@
>                                 [% END %]
>                             [% END # /UNLESS multi_hold %]
> 
>-                            <li>
>+                            <li id="non_priority_list_item">
>                                 <label for="non_priority">Non priority hold:</label>
>                                 <input name="non_priority" id="non_priority" type="checkbox" />
>+                                <div class="hint">A non priority hold doesn't prevent a current checkout from renewing</div>
>                             </li>
>                         </ol>
> 
>diff --git a/reserve/request.pl b/reserve/request.pl
>index b1e8745969..0bf0bae5d4 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -705,6 +705,7 @@ foreach my $biblionumber (@biblionumbers) {
>         $reserve{'reserve_id'}     = $res->reserve_id();
>         $reserve{itemtype}         = $res->itemtype();
>         $reserve{branchcode}       = $res->branchcode();
>+        $reserve{non_priority}     = $res->non_priority();
>         $reserve{object}           = $res;
> 
>         push( @reserveloop, \%reserve );
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 25793192e3..0cffe1501a 100755
>--- a/t/db_dependent/Holds.t
>+++ b/t/db_dependent/Holds.t
>@@ -1256,7 +1256,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
> 
> subtest 'non priority holds' => sub {
> 
>-    plan tests => 4;
>+    plan tests => 6;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1323,6 +1323,30 @@ subtest 'non priority holds' => sub {
>     ok( $ok, 'Can renew' );
>     is( $err, undef, 'Item is on non priority hold' );
> 
>+    my $patron3 = $builder->build_object(
>+        {
>+            class => 'Koha::Patrons',
>+            value => { branchcode => $item->homebranch }
>+        }
>+    );
>+
>+    # Add second hold with non_priority = 0
>+    AddReserve(
>+        {
>+            branchcode     => $item->homebranch,
>+            borrowernumber => $patron3->borrowernumber,
>+            biblionumber   => $item->biblionumber,
>+            priority       => 2,
>+            itemnumber     => $item->itemnumber,
>+        }
>+    );
>+
>+    ( $ok, $err ) =
>+      CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber );
>+
>+    ok( !$ok, 'Cannot renew' );
>+    is( $err, 'on_reserve', 'Item is on hold' );
>+
>     $schema->storage->txn_rollback;
> 
> };
>-- 
>2.25.0

Actions: View | Diff | Splinter Review

Attachments on bug 22789: 108703 | 108704 | 108705 | 108706 | 108707 | 108708 | 108709 | 108710 | 109123 | 109124 | 109125 | 109126 | 109127 | 109128 | 109129 | 109200 | 109201 | 109202 | 109203 | 109204 | 109205