From 7e1aad421c14cd4099baee9567b4860468218d66 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 15 Sep 2025 13:27:08 +0100 Subject: [PATCH] Bug 40808: Add comprehensive test coverage for account line linking Adds comprehensive test coverage for the polymorphic account line linking system with proper transaction isolation: 1. t/db_dependent/Koha/Account/Link.t - Tests individual Link objects - Object creation and validation - Link integrity validation - Exception handling for invalid link types and targets - Integration with account lines and holds - Unique constraint enforcement 2. t/db_dependent/Koha/Account/Links.t - Tests Links collection - Collection filtering by link_type and linked_id - Polymorphic access to linked objects (holds, checkouts) - Proper transaction isolation between subtests 3. Updates to t/db_dependent/Koha/Account/Line.t - New subtest for Account::Line linking methods - Tests add_link(), links(), holds(), checkouts() methods - Proper integration testing with transaction isolation All tests pass with comprehensive coverage of the linking functionality. --- t/db_dependent/Koha/Account/Line.t | 52 ++++- t/db_dependent/Koha/Account/Link.t | 295 ++++++++++++++++++++++++++++ t/db_dependent/Koha/Account/Links.t | 279 ++++++++++++++++++++++++++ 3 files changed, 625 insertions(+), 1 deletion(-) create mode 100755 t/db_dependent/Koha/Account/Link.t create mode 100755 t/db_dependent/Koha/Account/Links.t diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index 07e0f1cfa66..e827a38bb18 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 16; +use Test::More tests => 17; use Test::Exception; use Test::MockModule; @@ -1470,4 +1470,54 @@ subtest "cancel() tests" => sub { $schema->storage->txn_rollback; }; +subtest 'linking methods tests' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + # Create test data + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + + # Create a hold + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + itemnumber => $item->itemnumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + # Create an account line + my $account_line = $patron->account->add_debit( + { + amount => 5.00, + description => 'Mixed fee', + type => 'MANUAL', + interface => 'intranet' + } + ); + + # Test add_link method + my $hold_link = $account_line->add_link( 'hold', $hold->id ); + isa_ok( $hold_link, 'Koha::Account::Link', 'add_link returns Link object' ); + + # Test links method + my $links = $account_line->links; + isa_ok( $links, 'Koha::Account::Links', 'links returns Links collection' ); + is( $links->count, 1, 'Account line has one link' ); + + # Test holds method + my @holds = $account_line->holds; + is( scalar @holds, 1, 'holds() returns correct number of holds' ); + isa_ok( $holds[0], 'Koha::Hold', 'holds() returns Hold object' ); + is( $holds[0]->id, $hold->id, 'holds() returns correct hold' ); + + $schema->storage->txn_rollback; +}; + 1; diff --git a/t/db_dependent/Koha/Account/Link.t b/t/db_dependent/Koha/Account/Link.t new file mode 100755 index 00000000000..d9a81e9f160 --- /dev/null +++ b/t/db_dependent/Koha/Account/Link.t @@ -0,0 +1,295 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 6; +use Test::Exception; +use Test::Warn; + +use Koha::Database; +use Koha::Account::Link; +use Koha::Account::Links; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $builder = t::lib::TestBuilder->new; + +subtest 'Basic object creation and validation' => sub { + plan tests => 8; + + # Create test data + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + + # Create a hold + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + itemnumber => $item->itemnumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + # Create an account line + my $account_line = $patron->account->add_debit( + { + amount => 5.00, + description => 'Hold fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + # Test link creation + my $link = Koha::Account::Link->new( + { + accountlines_id => $account_line->id, + link_type => 'hold', + linked_id => $hold->id, + } + ); + + is( $link->accountlines_id, $account_line->id, 'Account line ID set correctly' ); + is( $link->link_type, 'hold', 'Link type set correctly' ); + is( $link->linked_id, $hold->id, 'Linked ID set correctly' ); + + # Test validation passes + ok( $link->_validate_link_integrity(), 'Link validation passes for valid data' ); + + # Test store + lives_ok { $link->store() } 'Link stores without error'; + ok( $link->id, 'Link has ID after storing' ); + + # Test linked_object method + my $linked_obj = $link->linked_object; + isa_ok( $linked_obj, 'Koha::Schema::Result::Reserve', 'linked_object returns correct type' ); + is( $linked_obj->reserve_id, $hold->id, 'linked_object returns correct hold' ); +}; + +subtest 'Link validation' => sub { + plan tests => 4; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $account_line = $patron->account->add_debit( + { + amount => 3.00, + description => 'Test fee', + type => 'MANUAL', + interface => 'intranet' + } + ); + + # Test invalid link type + my $invalid_link = Koha::Account::Link->new( + { + accountlines_id => $account_line->id, + link_type => 'invalid_type', + linked_id => 999, + } + ); + + ok( !$invalid_link->_validate_link_integrity(), 'Validation fails for invalid link type' ); + + # Test nonexistent linked entity + my $nonexistent_link = Koha::Account::Link->new( + { + accountlines_id => $account_line->id, + link_type => 'hold', + linked_id => 99999, # Doesn't exist + } + ); + + ok( !$nonexistent_link->_validate_link_integrity(), 'Validation fails for nonexistent linked entity' ); + + # Test store with invalid data + throws_ok { + $invalid_link->store() + } + 'Koha::Exceptions::Account::InvalidLinkType', 'Store throws InvalidLinkType exception for invalid link'; + + throws_ok { + $nonexistent_link->store() + } + 'Koha::Exceptions::Account::InvalidLinkTarget', 'Store throws InvalidLinkTarget exception for nonexistent entity'; +}; + +subtest 'Linked Koha object access' => sub { + plan tests => 3; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + my $account_line = $patron->account->add_debit( + { + amount => 2.00, + description => 'Hold fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + my $link = Koha::Account::Link->new( + { + accountlines_id => $account_line->id, + link_type => 'hold', + linked_id => $hold->id, + } + )->store; + + # Test linked_koha_object method + my $koha_hold = $link->linked_koha_object; + isa_ok( $koha_hold, 'Koha::Hold', 'linked_koha_object returns Koha::Hold object' ); + is( $koha_hold->id, $hold->id, 'Returned object has correct ID' ); + is( $koha_hold->borrowernumber, $patron->borrowernumber, 'Returned object has correct patron' ); +}; + +subtest 'Account line integration' => sub { + plan tests => 4; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + my $account_line = $patron->account->add_debit( + { + amount => 1.50, + description => 'Hold fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + # Test add_link method + my $link = $account_line->add_link( 'hold', $hold->id ); + isa_ok( $link, 'Koha::Account::Link', 'add_link returns Link object' ); + is( $link->accountlines_id, $account_line->id, 'Link has correct account line ID' ); + + # Test links method + my $links = $account_line->links; + isa_ok( $links, 'Koha::Account::Links', 'links returns Links collection' ); + is( $links->count, 1, 'Account line has one link' ); +}; + +subtest 'Hold integration' => sub { + plan tests => 3; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + # Create multiple account lines for this hold + my $fee1 = $patron->account->add_debit( + { + amount => 2.00, + description => 'Hold placement fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + my $fee2 = $patron->account->add_debit( + { + amount => 1.00, + description => 'Hold collection fee', + type => 'RESERVE_EXPIRED', + interface => 'intranet' + } + ); + + # Link both fees to the hold + $fee1->add_link( 'hold', $hold->id ); + $fee2->add_link( 'hold', $hold->id ); + + # Test hold can find its account lines + my $debits = $hold->debits; + isa_ok( $debits, 'Koha::Account::Debits', 'debits returns Debits collection' ); + is( $debits->search( {} )->count, 2, 'Hold has two linked debit lines' ); + + my $total_fees = $debits->total_outstanding; + is( $total_fees, 3.00, 'Total fees calculated correctly' ); +}; + +subtest 'Unique constraint and duplicate prevention' => sub { + plan tests => 2; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + my $account_line = $patron->account->add_debit( + { + amount => 5.00, + description => 'Hold fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + # Create first link + my $link1 = $account_line->add_link( 'hold', $hold->id ); + ok( $link1->id, 'First link created successfully' ); + + # Try to create duplicate link + throws_ok { + $account_line->add_link( 'hold', $hold->id ) + } + qr/duplicate/i, 'Duplicate link creation fails with unique constraint error'; +}; + +$schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Account/Links.t b/t/db_dependent/Koha/Account/Links.t new file mode 100755 index 00000000000..dd69288deae --- /dev/null +++ b/t/db_dependent/Koha/Account/Links.t @@ -0,0 +1,279 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 4; + +use Koha::Database; +use Koha::Account::Links; + +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $builder = t::lib::TestBuilder->new; + +subtest 'Basic collection functionality' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create test data + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + + my $hold1 = Koha::Hold->new( + { + borrowernumber => $patron1->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron1->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + my $hold2 = Koha::Hold->new( + { + borrowernumber => $patron2->borrowernumber, + biblionumber => $biblio->biblionumber, + branchcode => $patron2->branchcode, + reservedate => '2023-01-02', + priority => 2, + } + )->store; + + # Create account lines and links + my $fee1 = $patron1->account->add_debit( + { + amount => 2.00, + description => 'Hold fee 1', + type => 'RESERVE', + interface => 'intranet' + } + ); + + my $fee2 = $patron2->account->add_debit( + { + amount => 3.00, + description => 'Hold fee 2', + type => 'RESERVE', + interface => 'intranet' + } + ); + + $fee1->add_link( 'hold', $hold1->id ); + $fee2->add_link( 'hold', $hold2->id ); + + # Test collection methods + my $all_links = Koha::Account::Links->search( {} ); + is( $all_links->count, 2, 'Found both links in collection' ); + + my $hold_links = $all_links->by_link_type('hold'); + is( $hold_links->count, 2, 'by_link_type filters correctly' ); + + my $specific_links = $all_links->by_linked_id( $hold1->id ); + is( $specific_links->count, 1, 'by_linked_id filters correctly' ); + + $schema->storage->txn_rollback; +}; + +subtest 'Filtering methods' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + # Create diverse test data + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + + # Create hold + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + itemnumber => $item->itemnumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + # Create different types of fees + my $hold_fee = $patron->account->add_debit( + { + amount => 2.00, + description => 'Hold fee', + type => 'RESERVE', + interface => 'intranet' + } + ); + + my $manual_fee = $patron->account->add_debit( + { + amount => 1.00, + description => 'Manual fee', + type => 'MANUAL', + interface => 'intranet' + } + ); + + # Create links + $hold_fee->add_link( 'hold', $hold->id ); + + # Manual fee has no link + + my $all_links = Koha::Account::Links->search( {} ); + is( $all_links->count, 1, 'Total links created correctly' ); + + # Test filtering by type + my $hold_links = $all_links->by_link_type('hold'); + is( $hold_links->count, 1, 'Hold links filtered correctly' ); + + # Test filtering by ID + my $hold_id_links = $all_links->by_linked_id( $hold->id ); + is( $hold_id_links->count, 1, 'Links filtered by ID correctly' ); + + $schema->storage->txn_rollback; +}; + +subtest 'Collection methods for related objects' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio1 = $builder->build_sample_biblio(); + my $biblio2 = $builder->build_sample_biblio(); + + # Create multiple holds + my $hold1 = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio1->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + my $hold2 = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio2->biblionumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-02', + priority => 1, + } + )->store; + + # Create fees for both holds + my $fee1 = $patron->account->add_debit( + { + amount => 2.00, + description => 'Hold fee 1', + type => 'RESERVE', + interface => 'intranet' + } + ); + + my $fee2 = $patron->account->add_debit( + { + amount => 3.00, + description => 'Hold fee 2', + type => 'RESERVE', + interface => 'intranet' + } + ); + + # Link fees to holds + $fee1->add_link( 'hold', $hold1->id ); + $fee2->add_link( 'hold', $hold2->id ); + + # Test holds collection method + my $links = Koha::Account::Links->search( {} ); + my @holds = $links->holds(); + + is( scalar @holds, 2, 'holds() method returns correct number of holds' ); + + my @hold_ids = sort map { $_->id } @holds; + my @expected_ids = sort ( $hold1->id, $hold2->id ); + + is_deeply( \@hold_ids, \@expected_ids, 'holds() method returns correct holds' ); + + # Test filtering and then getting related objects + my $hold1_links = $links->by_linked_id( $hold1->id ); + my @filtered_holds = $hold1_links->holds(); + is( scalar @filtered_holds, 1, 'Filtered links return correct number of holds' ); + + $schema->storage->txn_rollback; +}; + +subtest 'Integration with account line methods' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + + # Create hold + my $hold = Koha::Hold->new( + { + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + itemnumber => $item->itemnumber, + branchcode => $patron->branchcode, + reservedate => '2023-01-01', + priority => 1, + } + )->store; + + # Create account line + my $account_line = $patron->account->add_debit( + { + amount => 4.00, + description => 'Mixed fee', + type => 'MANUAL', + interface => 'intranet' + } + ); + + # Link to both hold and checkout (edge case) + $account_line->add_link( 'hold', $hold->id ); + + # Test account line can access its links + my $links = $account_line->links; + is( $links->count, 1, 'Account line has links' ); + + # Test account line can get specific types + my @holds = $account_line->holds; + is( scalar @holds, 1, 'Account line holds() method works' ); + is( $holds[0]->id, $hold->id, 'Correct hold returned' ); + + # Test hold can find its account lines + my $hold_fees = $hold->debits; + is( $hold_fees->count, 1, 'Hold can find its account lines' ); + + $schema->storage->txn_rollback; +}; + +$schema->storage->txn_rollback; -- 2.51.0