From 27fe80a88f1227f420d1e19e22a8931a16ddf13e Mon Sep 17 00:00:00 2001 From: Lisette Scheer Date: Fri, 26 Jun 2026 14:40:54 +0000 Subject: [PATCH] Bug 41889: Update API to allow for null patron_id to be returned when listing checkouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a patron is deleted and there's a checkout associated with them, an api call to {url}/api/v1/checkouts?checked_in=true will result in an error. This patch updates the api to allow for a null value to be returned for patron_id To test: Before applying the patch: 1. In Administration->System preference enable the RESTBasicAuth system preference. 2. Checkout an item to a patron (I used item barcode: 39999000000252 and patron barcode 23529000121682) 3. Check the item back in 4. In your brower go to {sandbox/ktdurl}/api/v1/checkouts?checked_in=1 5. Observe there is a patron ID in the JSON response 6. Delete the patron 7. Repeat step 4 8. Observe an error because a "null" value was returned 9. Apply patch Sandbox: In sandbox management Actions->Apply patches Once complete Actions->Build CSS and JS Actions->Restart services KTD: ktd --shell git bz apply 41889 yarn build restart_all 10. Repeat step 4 11. Observe the checkout loads, but now has a null patron id. 12. Sign off On KTD can also run the script: qa to run tests. Signed-off-by: David Nind Signed-off-by: Tomás Cohen Arazi Signed-off-by: Lucas Gass (cherry picked from commit 64bea0833a264c60986fb011e069efc0da599bde) Signed-off-by: Baptiste Wojtkowski --- api/v1/swagger/definitions/checkout.yaml | 4 +- t/db_dependent/api/v1/checkouts.t | 277 ++++++++++++++++++++++- 2 files changed, 279 insertions(+), 2 deletions(-) diff --git a/api/v1/swagger/definitions/checkout.yaml b/api/v1/swagger/definitions/checkout.yaml index 3829255ffa..3e104e23b1 100644 --- a/api/v1/swagger/definitions/checkout.yaml +++ b/api/v1/swagger/definitions/checkout.yaml @@ -5,7 +5,9 @@ properties: type: integer description: internally assigned checkout identifier patron_id: - type: integer + type: + - integer + - "null" description: Internal patron identifier item_id: type: diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index a0d77e1d74..bcf2910a43 100755 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -18,7 +18,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 111; +use Test::More tests => 112; use Test::MockModule; use Test::Mojo; use t::lib::Mocks; @@ -33,6 +33,8 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Token; +use JSON qw(encode_json ); + my $schema = Koha::Database->schema; my $builder = t::lib::TestBuilder->new; @@ -599,3 +601,276 @@ subtest 'add checkout' => sub { $schema->storage->txn_rollback; }; +<<<<<<< HEAD +||||||| parent of 64bea0833a (Bug 41889: Update API to allow for null patron_id to be returned when listing checkouts) + +subtest 'renew with fine override (x-koha-override: debt_limit)' => sub { + plan tests => 15; + + $schema->storage->txn_begin; + + # Create a librarian with circulate permission + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2 } # Superlibrarian + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + # Create a regular patron + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + + # Create a branch and set up userenv + my $branch = $builder->build( { source => 'Branch' } ); + t::lib::Mocks::mock_userenv( + { branchcode => $branch->{branchcode}, borrowernumber => $librarian->borrowernumber } ); + + # Set up circulation rules + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + renewalperiod => 7, + renewalsallowed => 10, + issuelength => 5, + } + } + ); + + my $item = $builder->build_sample_item; + + # Check out the item + my $checkout = AddIssue( $patron, $item->barcode ); + + # Set FineNoRenewals to a low amount + t::lib::Mocks::mock_preference( 'FineNoRenewals', 5 ); + + # Add a fine that exceeds the limit + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + $account->add_debit( + { + amount => 10.00, + type => 'OVERDUE', + interface => 'test', + description => 'Test fine', + item_id => $item->itemnumber, + } + ); + + my $checkout_id = $checkout->id; + + # Test 1: Renewal should be blocked due to excessive fines + $t->post_ok("//$userid:$password@/api/v1/checkouts/$checkout_id/renewal") + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Test 2: Override should fail when AllowFineOverrideRenewing is disabled + t::lib::Mocks::mock_preference( 'AllowFineOverrideRenewing', 0 ); + + $t->post_ok( "//$userid:$password@/api/v1/checkouts/$checkout_id/renewal" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Test 3: Override should succeed when AllowFineOverrideRenewing is enabled + t::lib::Mocks::mock_preference( 'AllowFineOverrideRenewing', 1 ); + + $t->post_ok( "//$userid:$password@/api/v1/checkouts/$checkout_id/renewal" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(201) + ->json_has('/due_date'); + + # Refresh checkout + $checkout = Koha::Checkouts->find($checkout_id); + + # Test 4: Test with /renewals endpoint (alternative endpoint) + # Add another checkout to test the other endpoint + my $item2 = $builder->build_sample_item; + my $checkout2 = AddIssue( $patron, $item2->barcode ); + my $checkout2_id = $checkout2->id; + + # Should be blocked without override + $t->post_ok("//$userid:$password@/api/v1/checkouts/$checkout2_id/renewals") + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Should succeed with override + $t->post_ok( + "//$userid:$password@/api/v1/checkouts/$checkout2_id/renewals" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(201) + ->json_has('/due_date'); + + $schema->storage->txn_rollback; +}; +======= + +subtest 'renew with fine override (x-koha-override: debt_limit)' => sub { + plan tests => 15; + + $schema->storage->txn_begin; + + # Create a librarian with circulate permission + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2 } # Superlibrarian + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + # Create a regular patron + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + + # Create a branch and set up userenv + my $branch = $builder->build( { source => 'Branch' } ); + t::lib::Mocks::mock_userenv( + { branchcode => $branch->{branchcode}, borrowernumber => $librarian->borrowernumber } ); + + # Set up circulation rules + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + renewalperiod => 7, + renewalsallowed => 10, + issuelength => 5, + } + } + ); + + my $item = $builder->build_sample_item; + + # Check out the item + my $checkout = AddIssue( $patron, $item->barcode ); + + # Set FineNoRenewals to a low amount + t::lib::Mocks::mock_preference( 'FineNoRenewals', 5 ); + + # Add a fine that exceeds the limit + my $account = Koha::Account->new( { patron_id => $patron->borrowernumber } ); + $account->add_debit( + { + amount => 10.00, + type => 'OVERDUE', + interface => 'test', + description => 'Test fine', + item_id => $item->itemnumber, + } + ); + + my $checkout_id = $checkout->id; + + # Test 1: Renewal should be blocked due to excessive fines + $t->post_ok("//$userid:$password@/api/v1/checkouts/$checkout_id/renewal") + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Test 2: Override should fail when AllowFineOverrideRenewing is disabled + t::lib::Mocks::mock_preference( 'AllowFineOverrideRenewing', 0 ); + + $t->post_ok( "//$userid:$password@/api/v1/checkouts/$checkout_id/renewal" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Test 3: Override should succeed when AllowFineOverrideRenewing is enabled + t::lib::Mocks::mock_preference( 'AllowFineOverrideRenewing', 1 ); + + $t->post_ok( "//$userid:$password@/api/v1/checkouts/$checkout_id/renewal" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(201) + ->json_has('/due_date'); + + # Refresh checkout + $checkout = Koha::Checkouts->find($checkout_id); + + # Test 4: Test with /renewals endpoint (alternative endpoint) + # Add another checkout to test the other endpoint + my $item2 = $builder->build_sample_item; + my $checkout2 = AddIssue( $patron, $item2->barcode ); + my $checkout2_id = $checkout2->id; + + # Should be blocked without override + $t->post_ok("//$userid:$password@/api/v1/checkouts/$checkout2_id/renewals") + ->status_is(403) + ->json_like( '/error' => qr/too_much_owing/ ); + + # Should succeed with override + $t->post_ok( + "//$userid:$password@/api/v1/checkouts/$checkout2_id/renewals" => { 'x-koha-override' => 'debt_limit' } ) + ->status_is(201) + ->json_has('/due_date'); + + $schema->storage->txn_rollback; +}; + +subtest 'list checkouts that have been checked in and the patron has been deleted' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + # Create a librarian with circulate permission + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2 } # Superlibrarian + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + # Create a regular patron + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + my $patron_id = $patron->borrowernumber; + + my $old_checkout = $builder->build_object( + { + class => 'Koha::Old::Checkouts', + value => { borrowernumber => $patron_id } + } + ); + my $old_checkout_id = encode_json( { checkout_id => $old_checkout->id } ); + + # Test 1: Checkout should be returned + $t->get_ok("//$userid:$password@/api/v1/checkouts?checked_in=1&q=$old_checkout_id") + ->status_is(200) + ->json_is( '/0/patron_id' => $patron_id ); + + my $old_checkout_2 = $builder->build_object( + { + class => 'Koha::Old::Checkouts', + value => { borrowernumber => undef } + } + ); + my $old_checkout_2_id = encode_json( { checkout_id => $old_checkout_2->id } ); + + # Test 2: Checkout should be returned with null patron_id + + $t->get_ok("//$userid:$password@/api/v1/checkouts?checked_in=1&q=$old_checkout_2_id") + ->status_is(200) + ->json_is( '/0/patron_id' => undef ); + + $schema->storage->txn_rollback; +}; +>>>>>>> 64bea0833a (Bug 41889: Update API to allow for null patron_id to be returned when listing checkouts)