From bc7090fae78626cba939323815023ff407dbc78b Mon Sep 17 00:00:00 2001 From: Hammat Wele Date: Thu, 22 Aug 2024 20:35:37 +0000 Subject: [PATCH] Bug 20644: Add checkprevcheckout on item types --- 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 @@
Enter a summary that will overwrite the default one in search results lists. Example, for a website itemtype :
<a href="[856u]">open site</a> will show the link just below the title
+ [% IF Koha.Preference('CheckPrevCheckout') == 'softyes' || Koha.Preference('CheckPrevCheckout') == 'softno' %] +
  • + + +
  • + [% END %] 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