From fbf407ff35c71d34ce5d1ca7c1e89e9b84afa1a1 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 | 5 +- Koha/Patron.pm | 21 +-- admin/itemtypes.pl | 4 + ...ug_20644-add_checkprevcheckout_to_items.pl | 22 +++ installer/data/mysql/kohastructure.sql | 1 + .../prog/en/modules/admin/itemtypes.tt | 24 +++- .../en/modules/admin/preferences/patrons.pref | 4 +- t/db_dependent/Koha/ItemTypes.t | 38 +++++- t/db_dependent/Patron/Borrower_PrevCheckout.t | 126 +++++++++++++++++- 9 files changed, 225 insertions(+), 20 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 31794b7c02..a956203644 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1058,9 +1058,8 @@ 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 fd7cf85fc2..30a130396a 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -46,6 +46,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; @@ -806,28 +807,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 58258c62fa..172fe249b1 100755 --- a/admin/itemtypes.pl +++ b/admin/itemtypes.pl @@ -99,6 +99,8 @@ if ( $op eq 'add_form' ) { my $rentalcharge_hourly_calendar = $input->param('rentalcharge_hourly_calendar') // 0; my $automatic_checkin = $input->param('automatic_checkin') // 0; my $bookable = $input->param('bookable') // 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); @@ -120,6 +122,7 @@ if ( $op eq 'add_form' ) { $itemtype->rentalcharge_hourly_calendar($rentalcharge_hourly_calendar); $itemtype->automatic_checkin($automatic_checkin); $itemtype->bookable($bookable); + $itemtype->checkprevcheckout($checkprevcheckout); eval { $itemtype->store; @@ -154,6 +157,7 @@ if ( $op eq 'add_form' ) { rentalcharge_hourly_calendar => $rentalcharge_hourly_calendar, automatic_checkin => $automatic_checkin, bookable => $bookable, + 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 9b7dc7fc6e..b7f73fd36d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4238,6 +4238,7 @@ CREATE TABLE `itemtypes` ( `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', `bookable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Activate bookable feature for items related to this item 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 15550f97b2..7064ccb044 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -344,8 +344,8 @@ [% END %] </select> </li> - <li - ><label for="branches">Library limitation: </label> + <li> + <label for="branches">Library limitation: </label> <select id="branches" name="branches" multiple size="10"> <option value="">All libraries</option> [% PROCESS options_for_libraries libraries => Branches.all( selected => itemtype.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %] @@ -358,6 +358,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 671b4f4d41..3b827e722f 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 352aac8535..9ba7b64dea 100755 --- a/t/db_dependent/Koha/ItemTypes.t +++ b/t/db_dependent/Koha/ItemTypes.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 16; +use Test::More tests => 19; use t::lib::Mocks; use t::lib::TestBuilder; @@ -41,8 +41,8 @@ my $database = Koha::Database->new(); my $schema = $database->schema(); $schema->txn_begin; -my $builder = t::lib::TestBuilder->new; -my $initial_count = Koha::ItemTypes->search->count; +my $builder = t::lib::TestBuilder->new; +my $initial_count1 = Koha::ItemTypes->search->count; my $parent1 = $builder->build_object( { class => 'Koha::ItemTypes', value => { description => 'description' } } ); my $child1 = $builder->build_object( @@ -110,7 +110,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, @@ -160,4 +160,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 cb5480d5e7..a9064e36d2 100755 --- a/t/db_dependent/Patron/Borrower_PrevCheckout.t +++ b/t/db_dependent/Patron/Borrower_PrevCheckout.t @@ -8,7 +8,7 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Patrons; use Test::NoWarnings; -use Test::More tests => 62; +use Test::More tests => 170; use_ok('Koha::Patron'); @@ -49,6 +49,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' } ); @@ -57,6 +78,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 @@ -213,6 +235,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.43.0