From ac7cc74263e97974f3f6466b8302d36e053d40ee Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 25 Jan 2021 15:15:00 +0100 Subject: [PATCH] Bug 27545: Keep the location if passed Change in behaviour here, but expect. We must keep the location value if passed to store. Same for the permanent_location Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- Koha/Item.pm | 5 +- t/db_dependent/Koha/Items.t | 162 ++++++++++++++++++++++++------------ 2 files changed, 112 insertions(+), 55 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 9ce848d9f0..c0da07c4b9 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -100,8 +100,9 @@ sub store { } my $default_location = C4::Context->preference('NewItemsDefaultLocation'); - if ( $default_location ) { - $self->permanent_location( $self->location ); # FIXME To confirm - even if passed? + unless ( $self->location || !$default_location ) { + $self->permanent_location( $self->location || $default_location ) + unless $self->permanent_location; $self->location($default_location); } diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 2a3fdc5b1d..d1d910b020 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -92,63 +92,119 @@ subtest 'store' => sub { $item->delete; subtest 'permanent_location' => sub { - plan tests => 7; + plan tests => 2; - my $location = 'my_loc'; - my $attributes = { - homebranch => $library->{branchcode}, - holdingbranch => $library->{branchcode}, - biblionumber => $biblio->biblionumber, - location => $location, + subtest 'location passed to ->store' => sub { + plan tests => 7; + + my $location = 'my_loc'; + my $attributes = { + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + biblionumber => $biblio->biblionumber, + location => $location, + }; + + { + # NewItemsDefaultLocation not set + t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' ); + + # Not passing permanent_location on creating the item + my $item = Koha::Item->new($attributes)->store->get_from_storage; + is( $item->location, $location, + 'location must have been set to location if given' ); + is( $item->permanent_location, $item->location, + 'permanent_location must have been set to location if not given' ); + $item->delete; + + # Passing permanent_location on creating the item + $item = Koha::Item->new( + { %$attributes, permanent_location => 'perm_loc' } ) + ->store->get_from_storage; + is( $item->permanent_location, 'perm_loc', + 'permanent_location must have been kept if given' ); + $item->delete; + } + + { + # NewItemsDefaultLocation set + my $default_location = 'default_location'; + t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location ); + + # Not passing permanent_location on creating the item + my $item = Koha::Item->new($attributes)->store->get_from_storage; + is( $item->location, $location, + 'location must have been kept if given' ); + is( $item->permanent_location, $location, + 'permanent_location must have been set to the location given' ); + $item->delete; + + # Passing permanent_location on creating the item + $item = Koha::Item->new( + { %$attributes, permanent_location => 'perm_loc' } ) + ->store->get_from_storage; + is( $item->location, $location, + 'location must have been kept if given' ); + is( $item->permanent_location, 'perm_loc', + 'permanent_location must have been kept if given' ); + $item->delete; + } }; - { - # NewItemsDefaultLocation not set - t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' ); - - # Not passing permanent_location on creating the item - my $item = Koha::Item->new($attributes)->store->get_from_storage; - is( $item->location, $location, - 'location must have been set to location if given' ); - is( $item->permanent_location, $item->location, - 'permanent_location must have been set to location if not given' ); - $item->delete; - - # Passing permanent_location on creating the item - $item = Koha::Item->new( - { %$attributes, permanent_location => 'perm_loc' } ) - ->store->get_from_storage; - is( $item->permanent_location, 'perm_loc', - 'permanent_location must have been kept if given' ); - $item->delete; - } + subtest 'location NOT passed to ->store' => sub { + plan tests => 7; - { - # NewItemsDefaultLocation set - my $default_location = 'default_location'; - t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location ); - - # Not passing permanent_location on creating the item - my $item = Koha::Item->new($attributes)->store->get_from_storage; - is( $item->location, $default_location, - 'location must have been set to default_location even if given' # FIXME this sounds wrong! Must be done in any cases? - ); - is( $item->permanent_location, $location, - 'permanent_location must have been set to the location given' ); - $item->delete; - - # Passing permanent_location on creating the item - $item = Koha::Item->new( - { %$attributes, permanent_location => 'perm_loc' } ) - ->store->get_from_storage; - is( $item->location, $default_location, - 'location must have been set to default_location even if given' # FIXME this sounds wrong! Must be done in any cases? - ); - is( $item->permanent_location, $location, - 'permanent_location must have been set to the location given' - ); - $item->delete; - } + my $attributes = { + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + biblionumber => $biblio->biblionumber, + }; + + { + # NewItemsDefaultLocation not set + t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' ); + + # Not passing permanent_location on creating the item + my $item = Koha::Item->new($attributes)->store->get_from_storage; + is( $item->location, undef, + 'location not passed and no default, it is undef' ); + is( $item->permanent_location, $item->location, + 'permanent_location must have been set to location if not given' ); + $item->delete; + + # Passing permanent_location on creating the item + $item = Koha::Item->new( + { %$attributes, permanent_location => 'perm_loc' } ) + ->store->get_from_storage; + is( $item->permanent_location, 'perm_loc', + 'permanent_location must have been kept if given' ); + $item->delete; + } + + { + # NewItemsDefaultLocation set + my $default_location = 'default_location'; + t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location ); + + # Not passing permanent_location on creating the item + my $item = Koha::Item->new($attributes)->store->get_from_storage; + is( $item->location, $default_location, + 'location must have been set to default location if not given' ); + is( $item->permanent_location, $default_location, + 'permanent_location must have been set to the default location as well' ); + $item->delete; + + # Passing permanent_location on creating the item + $item = Koha::Item->new( + { %$attributes, permanent_location => 'perm_loc' } ) + ->store->get_from_storage; + is( $item->location, $default_location, + 'location must have been set to default location if not given' ); + is( $item->permanent_location, 'perm_loc', + 'permanent_location must have been kept if given' ); + $item->delete; + } + }; }; -- 2.20.1