From 39a1cc8dc3693ff1d638639b709433c93d4ffa9f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 18 Apr 2024 13:39:43 +0200 Subject: [PATCH] Bug 36535: (bug 33568 follow-up) Add missing tests Bug 33568 introduced a lot of new simple accessors without providing unit tests for them. Test plan: All tests modified by this patch should pass Note that one bug was found! Logic issue in lost/not lost Signed-off-by: David Nind --- Koha/Item.pm | 4 +- t/db_dependent/Items.t | 4 +- t/db_dependent/Koha/Biblio/ItemGroups.t | 10 +- t/db_dependent/Koha/Courses.t | 39 ++++++++ t/db_dependent/Koha/Item.t | 21 +++-- t/db_dependent/Koha/Items.t | 117 ++++++++++++++++++++---- t/db_dependent/api/v1/biblios.t | 36 +++++++- 7 files changed, 202 insertions(+), 29 deletions(-) create mode 100755 t/db_dependent/Koha/Courses.t diff --git a/Koha/Item.pm b/Koha/Item.pm index d9c89be3cd..91c15b22d5 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1972,7 +1972,7 @@ Returns the items associated with this bundle that are not lost sub bundle_items_not_lost { my ($self) = @_; - return $self->bundle_items->search( { itemlost => { '!=' => 0 } } ); + return $self->bundle_items->search( { itemlost => 0 } ); } =head3 bundle_items_lost @@ -1985,7 +1985,7 @@ Returns the items associated with this bundle that are lost sub bundle_items_lost { my ($self) = @_; - return $self->bundle_items->search( { itemlost => 0 } ); + return $self->bundle_items->search( { itemlost => { '!=' => 0 } } ); } =head3 is_bundle diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 84932499af..f6451c6f4e 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -541,7 +541,7 @@ subtest 'SearchItems test' => sub { subtest 'Koha::Item(s) tests' => sub { - plan tests => 7; + plan tests => 9; $schema->storage->txn_begin(); @@ -575,10 +575,12 @@ subtest 'Koha::Item(s) tests' => sub { my $homebranch = $item->home_branch(); is( ref($homebranch), 'Koha::Library', "Got Koha::Library from home_branch method" ); is( $homebranch->branchcode(), $library1->{branchcode}, "Home branch code matches homebranch" ); + is( ref( $item->home_library ), 'Koha::Library', 'home_library returns a Koha::Library object' ); my $holdingbranch = $item->holding_branch(); is( ref($holdingbranch), 'Koha::Library', "Got Koha::Library from holding_branch method" ); is( $holdingbranch->branchcode(), $library2->{branchcode}, "Home branch code matches holdingbranch" ); + is( ref( $item->holding_library ), 'Koha::Library', 'holding_library returns a Koha::Library object' ); $biblio = $item->biblio(); is( ref($item->biblio), 'Koha::Biblio', "Got Koha::Biblio from biblio method" ); diff --git a/t/db_dependent/Koha/Biblio/ItemGroups.t b/t/db_dependent/Koha/Biblio/ItemGroups.t index f0cbe5baf2..f0d8accf14 100755 --- a/t/db_dependent/Koha/Biblio/ItemGroups.t +++ b/t/db_dependent/Koha/Biblio/ItemGroups.t @@ -36,7 +36,7 @@ t::lib::Mocks::mock_preference('EnableItemGroups', 1); subtest 'add_item() and items() tests' => sub { - plan tests => 10; + plan tests => 12; $schema->storage->txn_begin; @@ -53,6 +53,14 @@ subtest 'add_item() and items() tests' => sub { my @items = $item_group->items->as_list(); is( scalar(@items), 1, 'Item group has one item'); is( $items[0]->id, $item_1->id, 'Item 1 is correct' ); + is( + ref( $items[0]->item_group ), 'Koha::Biblio::ItemGroup', + '->item_group should return a Koha::Biblio::ItemGroup object' + ); + is( + $items[0]->item_group->item_group_id, $item_group->item_group_id, + '->item_group should return the correct item group' + ); $item_group->add_item({ item_id => $item_2->id }); @items = $item_group->items->as_list(); diff --git a/t/db_dependent/Koha/Courses.t b/t/db_dependent/Koha/Courses.t new file mode 100755 index 0000000000..31c17b75b7 --- /dev/null +++ b/t/db_dependent/Koha/Courses.t @@ -0,0 +1,39 @@ +#!/usr/bin/perl + +# Copyright 2024 Koha Development team +# +# 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 => 1; + +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'Koha::Course::Reserve->course' => sub { + + plan tests => 1; + + $schema->storage->txn_begin; + + my $reserve = $builder->build_object( { class => 'Koha::Course::Reserves' } ); + is( ref( $reserve->course ), 'Koha::Course', '->course should return a Koha::Course object' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 438436a501..5df382edf5 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -161,7 +161,7 @@ subtest 'in_bundle tests' => sub { }; subtest 'bundle_items tests' => sub { - plan tests => 3; + plan tests => 6; $schema->storage->txn_begin; @@ -189,6 +189,11 @@ subtest 'bundle_items tests' => sub { is( $bundle_items->count, 3, 'bundle_items returns all the bundled items in the set' ); + $bundle_item2->itemlost(1)->store; + is( $host_item->bundle_items_not_lost->count, 2 ); + is( $host_item->bundle_items_lost->count, 1 ); + is( $host_item->bundle_items_lost->next->itemnumber, $bundle_item2->itemnumber ); + $schema->storage->txn_rollback; }; @@ -1157,18 +1162,20 @@ subtest 'renewal_branchcode' => sub { $schema->storage->txn_rollback; }; -subtest 'Tests for itemtype' => sub { - plan tests => 2; +subtest 'Tests for itemtype|item_type' => sub { + plan tests => 4; $schema->storage->txn_begin; my $biblio = $builder->build_sample_biblio; my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' }); my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, itype => $itemtype->itemtype }); - t::lib::Mocks::mock_preference('item-level_itypes', 1); - is( $item->itemtype->itemtype, $item->itype, 'Pref enabled' ); - t::lib::Mocks::mock_preference('item-level_itypes', 0); - is( $item->itemtype->itemtype, $biblio->biblioitem->itemtype, 'Pref disabled' ); + t::lib::Mocks::mock_preference( 'item-level_itypes', 1 ); + is( $item->itemtype->itemtype, $item->itype, 'Pref enabled' ); + is( $item->item_type->itemtype, $item->itype, 'Pref enabled' ); + t::lib::Mocks::mock_preference( 'item-level_itypes', 0 ); + is( $item->itemtype->itemtype, $biblio->biblioitem->itemtype, 'Pref disabled' ); + is( $item->item_type->itemtype, $biblio->biblioitem->itemtype, 'Pref disabled' ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index c29200e9a0..5b78910311 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 17; +use Test::More tests => 20; use Test::MockModule; use Test::Exception; @@ -1382,11 +1382,69 @@ subtest 'store' => sub { }; }; -subtest 'get_transfer' => sub { - plan tests => 7; +subtest 'serial_item' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + + my $item = $builder->build_sample_item; + my $serial_item = + $builder->build_object( { class => 'Koha::Serial::Items', value => { itemnumber => $item->itemnumber } } ); + is( ref( $item->serial_item ), 'Koha::Serial::Item' ); + is( $item->serial_item->itemnumber, $item->itemnumber ); + + is( ref( $item->serial_item->serial ), 'Koha::Serial', 'Koha::Serial::Item->serial returns a Koha::Serial object' ); + + $schema->storage->txn_rollback; + +}; + +subtest 'item_group_item' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $biblio = $builder->build_sample_biblio(); + my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + + my $item_group = Koha::Biblio::ItemGroup->new( { biblio_id => $biblio->id } )->store(); + $item_group->add_item( { item_id => $item_1->itemnumber } ); + + is( + ref( $item_1->item_group_item ), 'Koha::Biblio::ItemGroup::Item', + '->item_group_item should return a Koha::Biblio::ItemGroup::Item object' + ); + is( $item_1->item_group_item->item_id, $item_1->itemnumber, '->item_group_item should return the correct item' ); + + $schema->storage->txn_rollback; + +}; + +subtest 'course_item' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $item = $builder->build_sample_item; + my $course_item = + $builder->build_object( { class => 'Koha::Course::Items', value => { itemnumber => $item->itemnumber } } ); + is( ref( $item->course_item ), 'Koha::Course::Item', '->course_item should return a Koha::Course::Item object' ); + is( $item->course_item->ci_id, $course_item->ci_id, '->course_item should return the correct object' ); + + $schema->storage->txn_rollback; + +}; + +subtest 'get_transfer|transfer' => sub { + plan tests => 9; my $transfer = $new_item_1->get_transfer(); is( $transfer, undef, 'Koha::Item->get_transfer should return undef if the item is not in transit' ); + is( $new_item_1->transfer, undef ); my $library_to = $builder->build( { source => 'Branch' } ); @@ -1408,6 +1466,7 @@ subtest 'get_transfer' => sub { $transfer = $new_item_1->get_transfer(); is( ref($transfer), 'Koha::Item::Transfer', 'Koha::Item->get_transfer should return a Koha::Item::Transfer object' ); + is( ref($new_item_1->transfer), 'Koha::Item::Transfer' ); my $transfer_2 = $builder->build_object( { @@ -1459,21 +1518,47 @@ subtest 'get_transfer' => sub { }; subtest 'holds' => sub { - plan tests => 5; + plan tests => 7; my $biblio = $builder->build_sample_biblio(); - my $item = $builder->build_sample_item({ - biblionumber => $biblio->biblionumber, - }); - is($item->holds->count, 0, "Nothing returned if no holds"); - my $hold1 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'T' }}); - my $hold2 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'W' }}); - my $hold3 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'W' }}); - - is($item->holds()->count,3,"Three holds found"); - is($item->holds({found => 'W'})->count,2,"Two waiting holds found"); - is_deeply($item->holds({found => 'T'})->next->unblessed,$hold1,"Found transit holds matches the hold"); - is($item->holds({found => undef})->count, 0,"Nothing returned if no matching holds"); + my $item = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + } + ); + is( $item->holds->count, 0, "Nothing returned if no holds" ); + is( $item->first_hold, undef, 'No hold yet' ); + my $yesterday = dt_from_string->subtract( days => 1 )->ymd; + my $hold1 = $builder->build( + { + source => 'Reserve', + value => { + itemnumber => $item->itemnumber, found => 'T', reservedate => $yesterday, suspend => 0, priority => 2 + } + } + ); + my $hold2 = $builder->build( + { + source => 'Reserve', + value => { + itemnumber => $item->itemnumber, found => 'W', reservedate => $yesterday, suspend => 0, priority => 1 + } + } + ); + my $hold3 = $builder->build( + { + source => 'Reserve', + value => { + itemnumber => $item->itemnumber, found => 'W', reservedate => $yesterday, suspend => 0, priority => 3 + } + } + ); + + is( $item->holds()->count, 3, "Three holds found" ); + is( $item->holds( { found => 'W' } )->count, 2, "Two waiting holds found" ); + is_deeply( $item->holds( { found => 'T' } )->next->unblessed, $hold1, "Found transit holds matches the hold" ); + is( $item->holds( { found => undef } )->count, 0, "Nothing returned if no matching holds" ); + is( $item->first_hold->reserve_id, $hold2->{reserve_id}, '->first_hold returns the correct hold' ); }; subtest 'biblio' => sub { diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index b7e51a046f..946f580697 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -35,7 +35,7 @@ use C4::Circulation qw( AddIssue AddReturn ); use Koha::Biblios; use Koha::Database; -use Koha::DateUtils qw (dt_from_string); +use Koha::DateUtils qw (dt_from_string output_pref); use Koha::Checkouts; use Koha::Old::Checkouts; @@ -165,7 +165,7 @@ subtest 'get() tests' => sub { subtest 'get_items() tests' => sub { - plan tests => 11; + plan tests => 12; $schema->storage->txn_begin; @@ -202,6 +202,38 @@ subtest 'get_items() tests' => sub { ->status_is(200) ->json_has( '/0/_strings/home_library_id/str' => $item_1->holding_branch->branchname, '_strings are embedded' ); + subtest 'first_hold+strings' => sub { + + plan tests => 6; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + flags => 1, + } + } + ); + $patron->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $patron->userid; + my $pickup_library = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + + my $item = $builder->build_sample_item; + my $data = { + patron_id => $patron->borrowernumber, + biblio_id => $item->biblionumber, + item_id => $item->itemnumber, + pickup_library_id => $pickup_library->branchcode, + expiration_date => output_pref( { dt => DateTime->now->add(days => "3")->truncate(to => 'day'), dateformat => 'iso', dateonly => 1 } ), + }; + $t->post_ok( "//$userid:$password@/api/v1/holds" => json => $data )->status_is(201)->json_has('/hold_id'); + + $t->get_ok( "//$userid:$password@/api/v1/biblios/" + . $item->biblionumber + . "/items" => { "x-koha-embed" => "first_hold+strings" } )->status_is(200) + ->json_is( '/0/first_hold/_strings/pickup_library_id' => { type => 'library', str => $pickup_library->branchname } ); + }; + $schema->storage->txn_rollback; }; -- 2.30.2