Bugzilla – Attachment 179813 Details for
Bug 20644
Per itemtype setting for CheckPrevCheckout
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20644: Add checkprevcheckout on item types
Bug-20644-Add-checkprevcheckout-on-item-types.patch (text/plain), 23.26 KB, created by
Shi Yao Wang
on 2025-03-27 20:27:38 UTC
(
hide
)
Description:
Bug 20644: Add checkprevcheckout on item types
Filename:
MIME Type:
Creator:
Shi Yao Wang
Created:
2025-03-27 20:27:38 UTC
Size:
23.26 KB
patch
obsolete
>From 5be433163b7adf8ab5218b034a86c470df7003f0 Mon Sep 17 00:00:00 2001 >From: Hammat Wele <hammat.wele@inlibro.com> >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: <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 | 39 +++++- > t/db_dependent/Patron/Borrower_PrevCheckout.t | 126 +++++++++++++++++- > 9 files changed, 225 insertions(+), 21 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..bfb30376f1 100755 >--- a/t/db_dependent/Koha/ItemTypes.t >+++ b/t/db_dependent/Koha/ItemTypes.t >@@ -19,8 +19,7 @@ > > use Modern::Perl; > >-use Test::NoWarnings; >-use Test::More tests => 16; >+use Test::More tests => 18; > > use t::lib::Mocks; > use t::lib::TestBuilder; >@@ -41,8 +40,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 +109,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 +159,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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20644
:
156974
|
156977
|
163627
|
165585
|
165586
|
165661
|
165662
|
170619
|
170620
|
170621
|
170622
|
176538
|
176539
|
178093
|
178094
|
179812
|
179813
|
179814
|
179815
|
180375