Attachment 111443 Details for Bug 24194 – Bug 24194: Add DisableReserveExpiration system preference to hide expiration date options for reserves

[patch] Bug 24194: Add DisableReserveExpiration system preference to hide expiration date options for reserves

Bug-24194-Add-DisableReserveExpiration-system-pref.patch (text/plain), 16.21 KB, created by Katrin Fischer on 2020-10-11 10:40:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2020-10-11 10:40:23 UTC

Size: 16.21 KB

patch

obsolete

>From 473b755422fa01192cec8d5b1ea1fd563c80096a Mon Sep 17 00:00:00 2001
>From: Aleisha Amohia <aleishaamohia@hotmail.com>
>Date: Mon, 9 Dec 2019 04:33:42 +0000
>Subject: [PATCH] Bug 24194: Add DisableReserveExpiration system preference to
> hide expiration date options for reserves
>
>To test:
>1) Update database
>2) Go to place a hold on any biblio in the staff intranet and confirm
>you can see the 'Hold expires on date' field.
>3) In another tab, go to place a hold on any biblio in the OPAC and
>confirm you can see the 'Hold not needed after' field as an option.
>4) In yet another tab, open the staff intranet and place a reserve for a user. Check it in and set the reserve as
>waiting. Notice that an expiration date has now been generated for this
>reserve.
>5) Attempt to check out the item you reserved to some other borrower.
>Revert waiting status. Notice that the expiration date that was
>generated remains.
>6) Go to Administration -> system preferences and set DisableReserveExpiration system preference to 'Disable'
>7) Refresh the hold request page in the intranet. Confirm the expiration
>date field disappears.
>8) Refresh the hold request page in the OPAC. Confirm the expiration
>date field disappears.
>9) Place another reserve. Check it in and set the reserve as waiting.
>Notice that no expiration date was generated for this reserve.
>10) Attempt to check out the item you reserved to some other borrower.
>Revert waiting status. The expiration date should remain null.
>11) Confirm tests pass
>t/db_dependent/Hold.t
>t/db_dependent/Reserves.t
>
>Sponsored-by: Horowhenua Library Trust
>
>Signed-off-by: David Nind <david@davidnind.com>
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> C4/Reserves.pm                                     | 28 ++++++++----
> Koha/Hold.pm                                       | 51 ++++++++++++----------
> ...24194-add-DisableReserveExpiration-syspref.perl |  7 +++
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../en/modules/admin/preferences/circulation.pref  |  6 +++
> .../prog/en/modules/reserve/request.tt             | 14 +++---
> .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 12 ++---
> t/db_dependent/Hold.t                              | 36 ++++++++++++++-
> t/db_dependent/Reserves.t                          | 35 ++++++++++++++-
> 9 files changed, 145 insertions(+), 45 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 2422d4facd..d77206143b 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -2027,14 +2027,26 @@ sub RevertWaitingStatus {
>                            ->update({ priority => \'priority + 1' }, { no_triggers => 1 });
> 
>     ## Fix up the currently waiting reserve
>-    $hold->set(
>-        {
>-            priority    => 1,
>-            found       => undef,
>-            waitingdate => undef,
>-            itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
>-        }
>-    )->store();
>+    if ( C4::Context->preference('DisableReserveExpiration') ){
>+        $hold->set(
>+            {
>+                priority    => 1,
>+                found       => undef,
>+                waitingdate => undef,
>+                expirationdate => undef,
>+                itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
>+            }
>+        )->store();
>+    } else {
>+        $hold->set(
>+            {
>+                priority    => 1,
>+                found       => undef,
>+                waitingdate => undef,
>+                itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
>+            }
>+        )->store();
>+    }
> 
>     _FixPriority( { biblionumber => $hold->biblionumber } );
> 
>diff --git a/Koha/Hold.pm b/Koha/Hold.pm
>index 360b43e7b5..2c8fea6624 100644
>--- a/Koha/Hold.pm
>+++ b/Koha/Hold.pm
>@@ -173,36 +173,39 @@ sub set_waiting {
>         waitingdate => $today->ymd,
>     };
> 
>-    my $requested_expiration;
>-    if ($self->expirationdate) {
>-        $requested_expiration = dt_from_string($self->expirationdate);
>-    }
>+    if ( C4::Context->preference('DisableReserveExpiration') ){
>+        $values->{expirationdate} = undef;
>+    } else {
>+        my $requested_expiration;
>+        if ($self->expirationdate) {
>+            $requested_expiration = dt_from_string($self->expirationdate);
>+        }
> 
>-    my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
>-    my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
>+        my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
>+        my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
> 
>-    my $expirationdate = $today->clone;
>-    $expirationdate->add(days => $max_pickup_delay);
>+        my $expirationdate = $today->clone;
>+        $expirationdate->add(days => $max_pickup_delay);
> 
>-    if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) {
>-        my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype;
>-        my $daysmode = Koha::CirculationRules->get_effective_daysmode(
>-            {
>-                categorycode => $self->borrower->categorycode,
>-                itemtype     => $itemtype,
>-                branchcode   => $self->branchcode,
>-            }
>-        );
>-        my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode );
>+        if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) {
>+            my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype;
>+            my $daysmode = Koha::CirculationRules->get_effective_daysmode(
>+                {
>+                    categorycode => $self->borrower->categorycode,
>+                    itemtype     => $itemtype,
>+                    branchcode   => $self->branchcode,
>+                }
>+            );
>+            my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode );
>+            $expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay );
>+        }
> 
>-        $expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay );
>+        # If patron's requested expiration date is prior to the
>+        # calculated one, we keep the patron's one.
>+        my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0;
>+        $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd;
>     }
> 
>-    # If patron's requested expiration date is prior to the
>-    # calculated one, we keep the patron's one.
>-    my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0;
>-    $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd;
>-
>     $self->set($values)->store();
> 
>     return $self;
>diff --git a/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl
>new file mode 100644
>index 0000000000..a5fe773c18
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl
>@@ -0,0 +1,7 @@
>+$DBversion = 'XXX';  # will be replaced by the RM
>+if( CheckVersion( $DBversion ) ) {
>+    $dbh->do(q{ INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES ( "DisableReserveExpiration", 0, NULL, "Disable the use of expiration date in reserves module.", "YesNo" ) });
>+
>+    SetVersion( $DBversion );
>+    print "Upgrade to $DBversion done (Bug 24194 - DisableReserveExpiration system preference)\n";
>+}
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 313296cef3..1d6a2cb09d 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -163,6 +163,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('DefaultToLoggedInLibraryNoticesSlips',  '0', NULL ,  'If enabled,slips and notices editor will default to the logged in library''s rules, rather than the ''all libraries'' rules.',  'YesNo'),
> ('DefaultToLoggedInLibraryOverdueTriggers',  '0', NULL ,  'If enabled, overdue status triggers editor will default to the logged in library''s rules, rather than the ''default'' rules.',  'YesNo'),
> ('delimiter',';',';|tabulation|,|/|\\|#||','Define the default separator character for exporting reports','Choice'),
>+('DisableReserveExpiration', 0, NULL, 'Disable the use of expiration date in reserves module.', 'YesNo'),
> ('Display856uAsImage','OFF','OFF|Details|Results|Both','Display the URI in the 856u field as an image, the corresponding staff interface XSLT option must be on','Choice'),
> ('DisplayClearScreenButton','0','','If set to ON, a clear screen button will appear on the circulation page.','YesNo'),
> ('displayFacetCount','0',NULL,NULL,'YesNo'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>index dd803e71b2..1482c64a4e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -168,6 +168,12 @@ Circulation:
>                   yes: "Use"
>                   no: "Don't use"
>             - circulation desks with circulation.
>+        -
>+            - pref: DisableReserveExpiration
>+              choices:
>+                  yes: Disable
>+                  no: "Don't disable"
>+            - the use of expiration dates in reserves.
> 
>     Checkout policy:
>         -
>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 5d9ac5aefd..6b77f905e3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -445,12 +445,14 @@
>                                 </li>
>                             [% END %]
> 
>-                            <li>
>-                                <label for="to">Hold expires on date:</label>
>-                                <input name="expiration_date" id="to" size="10" class="datepickerto" type="text" />
>-                                <input type="hidden" class="datepickerto_hidden" />
>-                                <a href="#" id="clear-date-to" class="clear-date">Clear date</a>
>-                            </li>
>+                            [% IF !Koha.Preference('DisableReserveExpiration') %]
>+                                <li>
>+                                    <label for="to">Hold expires on date:</label>
>+                                    <input name="expiration_date" id="to" size="10" class="datepickerto" type="text" />
>+                                    <input type="hidden" class="datepickerto_hidden" />
>+                                    <a href="#" id="clear-date-to" class="clear-date">Clear date</a>
>+                                </li>
>+                            [% END %]
> 
>                             [% UNLESS ( multi_hold ) %]
>                                 <li>
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>index 1b9dd3bdee..8539c582ad 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>@@ -284,11 +284,13 @@
>                                                     </li>
>                                                 [% END %]
> 
>-                                                <li>
>-                                                    <label for="to[% bibitemloo.biblionumber | html %]">Hold not needed after:</label>
>-                                                    <input type="text" name="expiration_date_[% bibitemloo.biblionumber | html %]" id="to[% bibitemloo.biblionumber | html %]" size="10" class="holddateto" />
>-                                                    <span class="date-format to" data-biblionumber="[% bibitemloo.biblionumber | html %]">[% INCLUDE 'date-format.inc' %]</span>
>-                                                </li>
>+                                                [% IF !Koha.Preference('DisableReserveExpiration') %]
>+                                                    <li>
>+                                                        <label for="to[% bibitemloo.biblionumber | html %]">Hold not needed after:</label>
>+                                                        <input type="text" name="expiration_date_[% bibitemloo.biblionumber | html %]" id="to[% bibitemloo.biblionumber | html %]" size="10" class="holddateto" />
>+                                                        <span class="date-format to" data-biblionumber="[% bibitemloo.biblionumber | html %]">[% INCLUDE 'date-format.inc' %]</span>
>+                                                    </li>
>+                                                [% END %]
> 
>                                                 [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
>                                                     [% itemtypes = [] %]
>diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
>index dc588ded2b..f0f7adc0b8 100755
>--- a/t/db_dependent/Hold.t
>+++ b/t/db_dependent/Hold.t
>@@ -29,7 +29,7 @@ use Koha::Item;
> use Koha::DateUtils;
> use t::lib::TestBuilder;
> 
>-use Test::More tests => 29;
>+use Test::More tests => 30;
> use Test::Exception;
> use Test::Warn;
> 
>@@ -270,3 +270,37 @@ subtest 'suspend() tests' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'DisableReserveExpiration syspref tests' => sub {
>+
>+    plan tests => 2;
>+
>+    $schema->storage->txn_begin;
>+
>+    # Disable expiration date for reserves
>+    t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 1 );
>+
>+    my $expirationdate = dt_from_string->add( days => 1 );
>+    my $hold = $builder->build_object(
>+        {   class => 'Koha::Holds',
>+            value => { expirationdate => $expirationdate }
>+        }
>+    );
>+    $hold->set_waiting;
>+
>+    is( $hold->expirationdate, undef, "No expiration date should be set with reserve expiration disabled" );
>+
>+    # Re-enable expiration date for reserves
>+    t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 0 );
>+
>+    $hold = $builder->build_object(
>+        {   class => 'Koha::Holds',
>+            value => { expirationdate => $expirationdate }
>+        }
>+    );
>+    $hold->set_waiting();
>+
>+    is( $hold->expirationdate, $expirationdate->ymd, "Expiration date is set as expected" );
>+
>+    $schema->storage->txn_rollback;
>+};
>diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
>index c1c4758c86..37d5ff5b64 100755
>--- a/t/db_dependent/Reserves.t
>+++ b/t/db_dependent/Reserves.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 64;
>+use Test::More tests => 65;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -1150,7 +1150,40 @@ subtest 'CheckReserves additional test' => sub {
>     is( $matched_reserve->{reserve_id},
>         $reserve1->reserve_id, "We got the Transit reserve" );
>     is( scalar @$possible_reserves, 2, 'We do get both reserves' );
>+};
>+
>+subtest 'DisableReserveExpiration syspref tests' => sub {
>+
>+    plan tests => 2;
>+
>+    t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 0 );
>+
>+    my $expirationdate = dt_from_string->add( days => 1 );
>+    my $hold = $builder->build_object(
>+        {   class => 'Koha::Holds',
>+            value => { expirationdate => $expirationdate }
>+        }
>+    );
>+
>+    # Disable expiration date for reserves
>+    t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 1 );
>+
>+    my $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber });
>+
>+    is( $reverted->expirationdate, undef, "Expiration date should be removed with reserve expiration disabled" );
>+
>+    # Re-enable expiration date for reserves
>+    t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 0 );
>+
>+    $hold = $builder->build_object(
>+        {   class => 'Koha::Holds',
>+            value => { expirationdate => $expirationdate }
>+        }
>+    );
>+
>+    $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber });
> 
>+    is( $reverted->expirationdate, $expirationdate->ymd, "Expiration date remains as expected" );
> };
> 
> sub count_hold_print_messages {
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 24194: 96120 | 96121 | 98048 | 98368 | 100927 | 110082 | 110127 | 111443 | 113284 | 113285 | 116626 | 116627 | 117016 | 117085 | 117089 | 117097 | 117098 | 117099 | 117100 | 117101 | 118258 | 118259 | 118260 | 118261 | 118262 | 118263 | 128301 | 128302 | 128303 | 128304 | 128305 | 128306 | 128307 | 128308 | 140646 | 140647 | 144076 | 147527 | 147528 | 150799 | 150800 | 159060 | 159061 | 167796 | 167797 | 169174 | 174650 | 174651 | 174652