From 43225267c26e073511f6decdf79f44d581a03e2e Mon Sep 17 00:00:00 2001 From: Hammat Wele Date: Thu, 22 Aug 2024 21:05:57 +0000 Subject: [PATCH] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: . Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. --- C4/Circulation.pm | 7 +- Koha/Patron.pm | 21 +-- admin/itemtypes.pl | 3 + ...ug_20644-add_checkprevcheckout_to_items.pl | 22 +++ installer/data/mysql/kohastructure.sql | 1 + .../prog/en/modules/admin/itemtypes.tt | 20 +++ .../en/modules/admin/preferences/patrons.pref | 4 +- t/db_dependent/Koha/ItemTypes.t | 36 ++++- t/db_dependent/Patron/Borrower_PrevCheckout.t | 126 +++++++++++++++++- 9 files changed, 224 insertions(+), 16 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_20644-add_checkprevcheckout_to_items.pl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 70ccd7fb54..7beb7a64f2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1043,8 +1043,11 @@ sub CanBookBeIssued { # # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON # - $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed - if ( $patron->wants_check_for_previous_checkout && $patron->do_check_for_previous_checkout($item_unblessed) ) { + $patron = Koha::Patrons->find( $patron->borrowernumber ) + ; # FIXME Refetch just in case, to avoid regressions. But must not be needed + if ( $patron->wants_check_for_previous_checkout($item_object) + && $patron->do_check_for_previous_checkout($item_unblessed) ) + { $needsconfirmation{PREVISSUE} = 1; } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 7234a3be5b..7fb461a18c 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -45,6 +45,7 @@ use Koha::Encryption; use Koha::Exceptions; use Koha::Exceptions::Password; use Koha::Holds; +use Koha::ItemTypes; use Koha::Old::Checkouts; use Koha::OverdueRules; use Koha::Patron::Attributes; @@ -760,28 +761,32 @@ Return 1 if Koha needs to perform PrevIssue checking, else 0. =cut sub wants_check_for_previous_checkout { - my ( $self ) = @_; + my ( $self, $item ) = @_; my $syspref = C4::Context->preference("checkPrevCheckout"); # Simple cases ## Hard syspref trumps all return 1 if ($syspref eq 'hardyes'); return 0 if ($syspref eq 'hardno'); - ## Now, patron pref trumps all + + # Now, item pref trumps all + if ($item) { + my $itype = Koha::ItemTypes->find( $item->effective_itemtype ); + return 1 if ($itype->checkprevcheckout eq 'yes'); + return 0 if ($itype->checkprevcheckout eq 'no'); + } + + # Now, item type inherits -> determine patron preference return 1 if ($self->checkprevcheckout eq 'yes'); return 0 if ($self->checkprevcheckout eq 'no'); - # More complex: patron inherits -> determine category preference + # More complex: item type inherit and patron inherits -> determine category preference my $checkPrevCheckoutByCat = $self->category->checkprevcheckout; return 1 if ($checkPrevCheckoutByCat eq 'yes'); return 0 if ($checkPrevCheckoutByCat eq 'no'); # Finally: category preference is inherit, default to 0 - if ($syspref eq 'softyes') { - return 1; - } else { - return 0; - } + return $syspref eq 'softyes' ? 1 : 0; } =head3 do_check_for_previous_checkout diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl index 684f9377db..54c30c225a 100755 --- a/admin/itemtypes.pl +++ b/admin/itemtypes.pl @@ -98,6 +98,7 @@ if ( $op eq 'add_form' ) { my $rentalcharge_daily_calendar = $input->param('rentalcharge_daily_calendar') // 0; my $rentalcharge_hourly_calendar = $input->param('rentalcharge_hourly_calendar') // 0; my $automatic_checkin = $input->param('automatic_checkin') // 0; + my $checkprevcheckout = $input->param('checkprevcheckout'); if ( $itemtype and $is_a_modif ) { # it's a modification $itemtype->description($description); @@ -118,6 +119,7 @@ if ( $op eq 'add_form' ) { $itemtype->rentalcharge_daily_calendar($rentalcharge_daily_calendar); $itemtype->rentalcharge_hourly_calendar($rentalcharge_hourly_calendar); $itemtype->automatic_checkin($automatic_checkin); + $itemtype->checkprevcheckout($checkprevcheckout); eval { $itemtype->store; @@ -151,6 +153,7 @@ if ( $op eq 'add_form' ) { rentalcharge_daily_calendar => $rentalcharge_daily_calendar, rentalcharge_hourly_calendar => $rentalcharge_hourly_calendar, automatic_checkin => $automatic_checkin, + checkprevcheckout => $checkprevcheckout, } ); eval { diff --git a/installer/data/mysql/atomicupdate/bug_20644-add_checkprevcheckout_to_items.pl b/installer/data/mysql/atomicupdate/bug_20644-add_checkprevcheckout_to_items.pl new file mode 100755 index 0000000000..9e975facbf --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20644-add_checkprevcheckout_to_items.pl @@ -0,0 +1,22 @@ +use Modern::Perl; + +return { + bug_number => "20644", + description => "Add the column checkprevcheckout to itemtypes table", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + if( !column_exists( 'itemtypes', 'checkprevcheckout' ) ) { + $dbh->do( + q{ + ALTER TABLE itemtypes + ADD IF NOT EXISTS checkprevcheckout varchar(7) NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for a patron if a item of this type has previously been checked out to the same patron if ''yes'', not if ''no'', defer to category setting if ''inherit''.' + AFTER automatic_checkin; + } + ); + } + + say $out "Added column 'itemtypes.checkprevcheckout'"; + }, +}; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 753ae3cc57..abf5941371 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4137,6 +4137,7 @@ CREATE TABLE `itemtypes` ( `hideinopac` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Hide the item type from the search options in OPAC', `searchcategory` varchar(80) DEFAULT NULL COMMENT 'Group this item type with others with the same value on OPAC search options', `automatic_checkin` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'If automatic checkin is enabled for items of this type', + `checkprevcheckout` varchar(7) NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for a patron if a item of this type has previously been checked out to the same patron if ''yes'', not if ''no'', defer to category setting if ''inherit''.', PRIMARY KEY (`itemtype`), UNIQUE KEY `itemtype` (`itemtype`), KEY `itemtypes_ibfk_1` (`parent_type`), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt index b9ea060e30..2969140630 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -344,6 +344,26 @@ <div class="hint">Enter a summary that will overwrite the default one in search results lists. Example, for a website itemtype : </div> <div class="hint"><strong><a href="[856u]">open site</a></strong> will show the link just below the title</div> </li> + [% IF Koha.Preference('CheckPrevCheckout') == 'softyes' || Koha.Preference('CheckPrevCheckout') == 'softno' %] + <li> + <label for="checkprevcheckout">Check for previous checkouts: </label> + <select name="checkprevcheckout" id="checkprevcheckout"> + [% IF itemtype.checkprevcheckout == 'yes' %] + <option value="yes" selected="selected">Yes and override system preferences</option> + <option value="no">No and override system preferences</option> + <option value="inherit">Inherit from system preferences</option> + [% ELSIF itemtype.checkprevcheckout == 'no' %] + <option value="yes">Yes and override system preferences</option> + <option value="no" selected="selected">No and override system preferences</option> + <option value="inherit">Inherit from system preferences</option> + [% ELSE %] + <option value="yes">Yes and override system preferences</option> + <option value="no">No and override system preferences</option> + <option value="inherit" selected="selected">Inherit from system preferences</option> + [% END %] + </select> + </li> + [% END %] </ol> </fieldset> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 3db000432f..c54b7b9121 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -5,8 +5,8 @@ Patrons: default: no choices: hardyes: "Do" - softyes: "Unless overridden by patron category, do" - softno: "Unless overridden by patron category, do not" + softyes: "Unless overridden by patron category or by item type, do" + softno: "Unless overridden by patron category or by item type, do not" hardno: "Do not" - " check borrower checkout history to see if the current item has been checked out before." - diff --git a/t/db_dependent/Koha/ItemTypes.t b/t/db_dependent/Koha/ItemTypes.t index d62cf05e48..d797e53b84 100755 --- a/t/db_dependent/Koha/ItemTypes.t +++ b/t/db_dependent/Koha/ItemTypes.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 15; +use Test::More tests => 18; use t::lib::Mocks; use t::lib::TestBuilder; @@ -41,7 +41,7 @@ my $schema = $database->schema(); $schema->txn_begin; my $builder = t::lib::TestBuilder->new; -my $initial_count = Koha::ItemTypes->search->count; +my $initial_count1 = Koha::ItemTypes->search->count; my $parent1 = $builder->build_object({ class => 'Koha::ItemTypes', value => { description => 'description' } }); my $child1 = $builder->build_object({ @@ -103,7 +103,7 @@ is_deeply( $type->unblessed, $child2->unblessed, "We got back the same object" ) t::lib::Mocks::mock_preference('language', 'en'); t::lib::Mocks::mock_preference('OPACLanguages', 'en'); my $itemtypes = Koha::ItemTypes->search_with_localization; -is( $itemtypes->count, $initial_count + 4, 'We added 4 item types' ); +is( $itemtypes->count, $initial_count1 + 4, 'We added 4 item types' ); my $first_itemtype = $itemtypes->next; is( $first_itemtype->translated_description, @@ -150,4 +150,34 @@ subtest 'image_location' => sub { ); }; +# test for checkprevcheckout +my $initial_count2 = Koha::ItemTypes->search->count; + +my @itypes = ( + Koha::ItemType->new( + { + itemtype => 'ITYPE1', + } + )->store, + Koha::ItemType->new( + { + itemtype => 'ITYPE2', + checkprevcheckout => undef, + } + )->store, +); + +for my $itype (@itypes) { + my $retrived_itype = Koha::ItemTypes->find( $itype->itemtype ); + is( + $retrived_itype->checkprevcheckout, 'inherit', + 'Koha::ItemType->store should default checkprevcheckout to inherit' + ); +} + +is( + Koha::ItemTypes->search->count, $initial_count2 + 2, + 'We added two item types with checkprevcheckout set to inherit' +); + $schema->txn_rollback; diff --git a/t/db_dependent/Patron/Borrower_PrevCheckout.t b/t/db_dependent/Patron/Borrower_PrevCheckout.t index 310458d119..767df5fa94 100755 --- a/t/db_dependent/Patron/Borrower_PrevCheckout.t +++ b/t/db_dependent/Patron/Borrower_PrevCheckout.t @@ -7,7 +7,7 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string ); use Koha::Patrons; -use Test::More tests => 61; +use Test::More tests => 169; use_ok('Koha::Patron'); @@ -42,6 +42,27 @@ my $inheritCatCode = $builder->build({ }, }); +my $yesItypeCode = $builder->build_object({ + class => 'Koha::ItemTypes', + value => { + checkprevcheckout => 'yes', + } +}); + +my $noItypeCode = $builder->build_object({ + class => 'Koha::ItemTypes', + value => { + checkprevcheckout => 'no', + } +}); + +my $inheritItypeCode = $builder->build_object({ + class => 'Koha::ItemTypes', + value => { + checkprevcheckout => 'inherit', + } +}); + # Create context for some tests late on in the file. my $library = $builder->build({ source => 'Branch' }); my $staff = $builder->build({source => 'Borrower'}); @@ -50,6 +71,7 @@ t::lib::Mocks::mock_userenv({ branchcode => $library->{branchcode} }); # wants_check_for_previous_checkout +# We want to test the subroutine without passing the $item parameter # We expect the following result matrix: # # (1/0 indicates the return value of WantsCheckPrevCheckout; i.e. 1 says we @@ -205,6 +227,108 @@ map { } @{$_->{categories}}; } @{$mappings}; + +# wants_check_for_previous_checkout + +# We want to test the subroutine by passing the $item parameter +# We expect the following result matrix: +# +# (1/0 indicates the return value of WantsCheckPrevCheckout; i.e. 1 says we +# should check whether the item was previously issued) +# +# | System Preference | hardyes | +# |-------------------+-----------------------------------------------------------------------------------------------------------| +# | Item Type Setting | yes | no | inherit | +# |-------------------+-----------------------------------+-----------------------------------+-----------------------------------| +# | Category Setting | yes | no | inherit | yes | no | inherit | yes | no | inherit | +# |-------------------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------| +# | Patron Setting | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | +# |-------------------+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---| +# | Expected Result | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | +# +# +# | System Preference | softyes | +# |-------------------+-----------------------------------------------------------------------------------------------------------| +# | Item Type Setting | yes | no | inherit | +# |-------------------+-----------------------------------+-----------------------------------+-----------------------------------| +# | Category Setting | yes | no | inherit | yes | no | inherit | yes | no | inherit | +# |-------------------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------| +# | Patron Setting | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | +# |-------------------+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---| +# | Expected Result | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 1 | 0 | 1 | 1 | 0 | 0 | 1 | 0 | 1 | +# +# +# | System Preference | softno | +# |-------------------+-----------------------------------------------------------------------------------------------------------| +# | Item Type Setting | yes | no | inherit | +# |-------------------+-----------------------------------+-----------------------------------+-----------------------------------| +# | Category Setting | yes | no | inherit | yes | no | inherit | yes | no | inherit | +# |-------------------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------| +# | Patron Setting | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | +# |-------------------+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---| +# | Expected Result | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 1 | 0 | 1 | 1 | 0 | 0 | 1 | 0 | 0 | +# +# +# | System Preference | hardno | +# |-------------------+-----------------------------------------------------------------------------------------------------------| +# | Item Type Setting | yes | no | inherit | +# |-------------------+-----------------------------------+-----------------------------------+-----------------------------------| +# | Category Setting | yes | no | inherit | yes | no | inherit | yes | no | inherit | +# |-------------------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------+-----------| +# | Patron Setting | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | y | n | i | +# |-------------------+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---| +# | Expected Result | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | + +my $itypeCode = { + 'yes' => $yesItypeCode->itemtype, + 'no' => $noItypeCode->itemtype, + 'inherit' => $inheritItypeCode->itemtype, +}; + +foreach my $syspref ('hardyes','softyes','softno','hardno'){ + t::lib::Mocks::mock_preference( 'checkprevcheckout', $syspref ); + foreach my $itemtype_setting ('yes','no','inherit'){ #itemtype Setting + my $item = $builder->build_sample_item( { itype => $itypeCode->{$itemtype_setting} } ); + foreach my $categorie_settings('yes','no','inherit'){ + my $catCode = $categorie_settings . 'Cat'; + foreach my $patron_setting('yes','no','inherit'){ + my $result = undef; + $result = 1 if($syspref eq 'hardyes'); + $result = 0 if($syspref eq 'hardno'); + $result = 1 if(!defined $result && $itemtype_setting eq 'yes'); + $result = 0 if(!defined $result && $itemtype_setting eq 'no'); + $result = 1 if(!defined $result && $patron_setting eq 'yes'); + $result = 0 if(!defined $result && $patron_setting eq 'no'); + $result = 1 if(!defined $result && $categorie_settings eq 'yes'); + $result = 0 if(!defined $result && $categorie_settings eq 'no'); + $result = 1 if(!defined $result && $syspref eq 'softyes'); + $result = 0 if(!defined $result && $syspref eq 'softno'); + my $kpatron = $builder->build( + { + source => 'Borrower', + value => { + checkprevcheckout => $patron_setting, + categorycode => $catCode, + }, + } + ); + my $patron = Koha::Patrons->find( $kpatron->{borrowernumber} ); + is( + $patron->wants_check_for_previous_checkout($item), $result, + "Predicate with syspref " + . $syspref + . ", cat " + . $catCode + . ", patron " + . $patron_setting + . ", item type " + . $itypeCode->{$itemtype_setting} + ); + } + } + } +} + # do_check_for_previous_checkout # We want to test: -- 2.34.1