From af27b5c1dfb69c3491fe4c8f8fb480a8177ea20c Mon Sep 17 00:00:00 2001 From: Baptiste Date: Mon, 9 Dec 2024 16:57:40 +0100 Subject: [PATCH] Bug 35292: Move tests for UpdateNotForLoanStatusOnCheckin in a subtest Note: These patches have been split for sake of understandability for review but they should probably be squashe --- t/db_dependent/Circulation/issue.t | 154 +++++++++++++++-------------- 1 file changed, 82 insertions(+), 72 deletions(-) diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index b4e787c1b06..8f377d67c82 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 66; +use Test::More tests => 60; use DateTime::Duration; use t::lib::Mocks; @@ -417,78 +417,88 @@ C4::Circulation::AddIssue( $patron_1, $barcode_1, $daysago10, 0, $today ); AddReturn($barcode_1, undef, undef, dt_from_string('2014-04-01 23:42')); $return = $dbh->selectrow_hashref("SELECT * FROM old_issues LIMIT 1" ); ok( $return->{returndate} eq '2014-04-01 23:42:00', "Item returned with a return date of '2014-04-01 23:42' has that return date" ); +subtest 'AddIssue Update notforloanstatus according to UpdateNotForLoanStatusOnCheckin' => sub { + plan tests => 7; + my $itemnumber = Koha::Item->new( + { + biblionumber => $biblionumber, + barcode => 'barcode_3', + itemcallnumber => 'callnumber3', + homebranch => $branchcode_1, + holdingbranch => $branchcode_1, + notforloan => 1, + itype => $itemtype, + biblioitemnumber => $biblioitemnumber + }, + )->store->itemnumber; -my $itemnumber = Koha::Item->new( - { - biblionumber => $biblionumber, - barcode => 'barcode_3', - itemcallnumber => 'callnumber3', - homebranch => $branchcode_1, - holdingbranch => $branchcode_1, - notforloan => 1, - itype => $itemtype, - biblioitemnumber => $biblioitemnumber - }, -)->store->itemnumber; - -t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckin', q{} ); -t::lib::Mocks::mock_preference( 'CataloguingLog', 1 ); -my $log_count_before = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); - -AddReturn( 'barcode_3', $branchcode_1 ); -my $item = Koha::Items->find( $itemnumber ); -ok( $item->notforloan eq 1, 'UpdateNotForLoanStatusOnCheckin does not modify value when not enabled' ); - -my $updatenotforloanstatusoncheckin = " -_ALL_:\n - 1: 0\n - 9: 1\n\n -$itemtype:\n - 1: 9 -"; -t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckin', $updatenotforloanstatusoncheckin ); -AddReturn( 'barcode_3', $branchcode_1 ); -$item = Koha::Items->find( $itemnumber ); -ok( $item->notforloan eq 9, q{UpdateNotForLoanStatusOnCheckin prioritises item type specific rule over _ALL_ rules} ); -my $log_count_after = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); -is($log_count_before, $log_count_after, "Change from UpdateNotForLoanStatusOnCheckin is not logged"); - -AddReturn( 'barcode_3', $branchcode_1 ); -$item = Koha::Items->find( $itemnumber ); -ok( $item->notforloan eq 9, q{UpdateNotForLoanStatusOnCheckin uses item type specific rules even if they do not target the returned items' notforloan value} ); + t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckin', q{} ); + t::lib::Mocks::mock_preference( 'CataloguingLog', 1 ); + my $log_count_before = $schema->resultset('ActionLog')->search( { module => 'CATALOGUING' } )->count(); + + AddReturn( 'barcode_3', $branchcode_1 ); + my $item = Koha::Items->find($itemnumber); + ok( $item->notforloan eq 1, 'UpdateNotForLoanStatusOnCheckin does not modify value when not enabled' ); + + my $updatenotforloanstatusoncheckin = " + _ALL_:\n + 1: 0\n + 9: 1\n\n + $itemtype:\n + 1: 9 + "; + t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckin', $updatenotforloanstatusoncheckin ); + AddReturn( 'barcode_3', $branchcode_1 ); + $item = Koha::Items->find($itemnumber); + ok( + $item->notforloan eq 9, + q{UpdateNotForLoanStatusOnCheckin prioritises item type specific rule over _ALL_ rules} + ); + my $log_count_after = $schema->resultset('ActionLog')->search( { module => 'CATALOGUING' } )->count(); + is( $log_count_before, $log_count_after, "Change from UpdateNotForLoanStatusOnCheckin is not logged" ); -# Change the returning item to an item type without a rule -Koha::Items->find( $itemnumber )->itype( $itemtype2 )->store; -Koha::Items->find( $itemnumber )->notforloan( 1 )->store; -AddReturn( 'barcode_3', $branchcode_1 ); -$item = Koha::Items->find( $itemnumber ); -ok( $item->notforloan eq 0, q{UpdateNotForLoanStatusOnCheckin _ALL_ rules are applied if there are no specific item type rule matching the returned item} ); + AddReturn( 'barcode_3', $branchcode_1 ); + $item = Koha::Items->find($itemnumber); + ok( + $item->notforloan eq 9, + q{UpdateNotForLoanStatusOnCheckin uses item type specific rules even if they do not target the returned items' notforloan value} + ); -t::lib::Mocks::mock_preference( - 'UpdateNotForLoanStatusOnCheckin', q{_ALL_: - 1: ONLYMESSAGE -} -); -$item->notforloan(1)->store; -AddReturn( 'barcode_3', $branchcode_1 ); -$item->discard_changes; -is( - $item->notforloan, 1, - q{UpdateNotForLoanStatusOnCheckin does not update notforloan value from 1 for _ALL_ with setting "1: ONLYMESSAGE"} -); + # Change the returning item to an item type without a rule + Koha::Items->find($itemnumber)->itype($itemtype2)->store; + Koha::Items->find($itemnumber)->notforloan(1)->store; + AddReturn( 'barcode_3', $branchcode_1 ); + $item = Koha::Items->find($itemnumber); + ok( + $item->notforloan eq 0, + q{UpdateNotForLoanStatusOnCheckin _ALL_ rules are applied if there are no specific item type rule matching the returned item} + ); + t::lib::Mocks::mock_preference( + 'UpdateNotForLoanStatusOnCheckin', q{_ALL_: + 1: ONLYMESSAGE + } + ); + $item->notforloan(1)->store; + AddReturn( 'barcode_3', $branchcode_1 ); + $item->discard_changes; + is( + $item->notforloan, 1, + q{UpdateNotForLoanStatusOnCheckin does not update notforloan value from 1 for _ALL_ with setting "1: ONLYMESSAGE"} + ); -t::lib::Mocks::mock_preference( - 'UpdateNotForLoanStatusOnCheckin', "$itemtype:\n - 1: ONLYMESSAGE -" -); -AddReturn( 'barcode_3', $branchcode_1 ); -$item->discard_changes; -is( - $item->notforloan, 1, - q{UpdateNotForLoanStatusOnCheckin does not update notforloan value for specific itemtype from 1 with setting "1: ONLYMESSAGE"} -); + t::lib::Mocks::mock_preference( + 'UpdateNotForLoanStatusOnCheckin', "$itemtype:\n + 1: ONLYMESSAGE + " + ); + AddReturn( 'barcode_3', $branchcode_1 ); + $item->discard_changes; + is( + $item->notforloan, 1, + q{UpdateNotForLoanStatusOnCheckin does not update notforloan value for specific itemtype from 1 with setting "1: ONLYMESSAGE"} + ); +}; ############################################## # @@ -514,7 +524,7 @@ subtest 'AddReturn: Update notforloanstatus according to UpdateNotForLoanStatusO t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckout', q{} ); AddIssue( $patron_2, 'barcode_6', dt_from_string ); - $item = Koha::Items->find($itemnumber4); + my $item = Koha::Items->find($itemnumber4); ok( $item->notforloan eq -1, 'UpdateNotForLoanStatusOnCheckout does not modify value when not enabled' ); AddReturn( 'barcode_6', $branchcode_1 ); @@ -606,7 +616,7 @@ subtest '_updateNotForLoanFromYaml' => sub { t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckout', q{} ); t::lib::Mocks::mock_preference( 'UpdateNotForLoanStatusOnCheckin', q{} ); - $item = Koha::Items->find($itemnumber4); + my $item = Koha::Items->find($itemnumber4); $item->notforloan(-1)->store; C4::Circulation::_updateNotForLoanFromYaml( $item, 'UpdateNotForLoanStatusOnCheckout' ); ok( @@ -749,12 +759,12 @@ my $item2 = Koha::Items->find( $itemnumber2 ); ok( $item2->location eq 'FIC', 'UpdateItemLocationOnCheckin does not modify value when not enabled' ); t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckin', 'FIC: GEN' ); -$log_count_before = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); +my $log_count_before = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); AddReturn( 'barcode_4', $branchcode_1 ); $item2 = Koha::Items->find( $itemnumber2 ); is( $item2->location, 'GEN', q{UpdateItemLocationOnCheckin updates location value from 'FIC' to 'GEN' with setting "FIC: GEN"} ); is( $item2->permanent_location, 'GEN', q{UpdateItemLocationOnCheckin updates permanent_location value from 'FIC' to 'GEN' with setting "FIC: GEN"} ); -$log_count_after = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); +my $log_count_after = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); is($log_count_before, $log_count_after, "Change from UpdateNotForLoanStatusOnCheckin is not logged"); AddReturn( 'barcode_4', $branchcode_1 ); $item2 = Koha::Items->find( $itemnumber2 ); -- 2.30.2