Koha-community/Koha
15
7
Fork 0
Code Issues Releases Wiki Activity

Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t

Browse source 
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Paul Derscheid 2024-10-18 09:56:50 +00:00 committed by Katrin Fischer
parent 50b9af836e
commit 6380c89ab9
Signed by: kfischer
GPG key ID: 0EF6E2C03357A834
2 changed files with 88 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

131
admin/itemtypes.pl
View file

@ -25,10 +25,9 @@
use Modern::Perl; use Modern::Perl;
use CGI qw ( -utf8 ); use CGI qw ( -utf8 );
use C4::Koha qw( getImageSets GetAuthorisedValues ); use C4::Koha qw( getImageSets GetAuthorisedValues );
use C4::Context; use C4::Context;
use C4::Auth qw( get_template_and_user ); use C4::Auth qw( get_template_and_user );
use C4::Output qw( output_html_with_http_headers ); use C4::Output qw( output_html_with_http_headers );
use Koha::ItemTypes; use Koha::ItemTypes;
use Koha::ItemType; use Koha::ItemType;
@ -41,10 +40,11 @@ my $op = $input->param('op') // 'list';
my @messages; my @messages;
$searchfield =~ s/\,//g if $searchfield; $searchfield =~ s/\,//g if $searchfield;
my ( $template, $borrowernumber, $cookie ) = get_template_and_user( my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
{ template_name => "admin/itemtypes.tt", {
query => $input, template_name => "admin/itemtypes.tt",
type => "intranet", query => $input,
flagsrequired => { parameters => 'manage_itemtypes' }, type => "intranet",
flagsrequired => { parameters => 'manage_itemtypes' },
} }
); );
@ -56,49 +56,49 @@ undef($sip_media_type) if defined($sip_media_type) and $sip_media_type =~ /^\s*$
if ( $op eq 'add_form' ) { if ( $op eq 'add_form' ) {
my $itemtype = Koha::ItemTypes->find($itemtype_code); my $itemtype = Koha::ItemTypes->find($itemtype_code);
my $parent_type = $itemtype ? $itemtype->parent_type : undef; my $parent_type = $itemtype ? $itemtype->parent_type : undef;
my $parent_types = Koha::ItemTypes->search({parent_type=>undef,itemtype => {'!='=>$itemtype_code}}); my $parent_types = Koha::ItemTypes->search( { parent_type => undef, itemtype => { '!=' => $itemtype_code } } );
my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) ); my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) );
my $searchcategory = GetAuthorisedValues("ITEMTYPECAT"); my $searchcategory = GetAuthorisedValues("ITEMTYPECAT");
my $translated_languages = C4::Languages::getTranslatedLanguages( "both", C4::Context->preference('template') ); my $translated_languages = C4::Languages::getTranslatedLanguages( "both", C4::Context->preference('template') );
$template->param( $template->param(
itemtype => $itemtype, itemtype => $itemtype,
parent_type => $parent_type, parent_type => $parent_type,
parent_types => $parent_types, parent_types => $parent_types,
is_a_parent => $itemtype ? Koha::ItemTypes->search({parent_type=>$itemtype_code})->count : 0, is_a_parent => $itemtype ? Koha::ItemTypes->search( { parent_type => $itemtype_code } )->count : 0,
imagesets => $imagesets, imagesets => $imagesets,
searchcategory => $searchcategory, searchcategory => $searchcategory,
can_be_translated => ( scalar(@$translated_languages) > 1 ? 1 : 0 ), can_be_translated => ( scalar(@$translated_languages) > 1 ? 1 : 0 ),
); );
} elsif ( $op eq 'cud-add_validate' ) { } elsif ( $op eq 'cud-add_validate' ) {
my $is_a_modif = $input->param('is_a_modif'); my $is_a_modif = $input->param('is_a_modif');
my $itemtype = Koha::ItemTypes->find($itemtype_code); my $itemtype = Koha::ItemTypes->find($itemtype_code);
my $parent_type = $input->param('parent_type') || undef; my $parent_type = $input->param('parent_type') || undef;
my $description = $input->param('description'); my $description = $input->param('description');
my $rentalcharge = $input->param('rentalcharge'); my $rentalcharge = $input->param('rentalcharge');
my $rentalcharge_daily = $input->param('rentalcharge_daily'); my $rentalcharge_daily = $input->param('rentalcharge_daily');
my $rentalcharge_hourly = $input->param('rentalcharge_hourly'); my $rentalcharge_hourly = $input->param('rentalcharge_hourly');
my $defaultreplacecost = $input->param('defaultreplacecost'); my $defaultreplacecost = $input->param('defaultreplacecost');
my $processfee = $input->param('processfee'); my $processfee = $input->param('processfee');
my $image = $input->param('image') || q||; my $image = $input->param('image') || q||;
my @branches = grep { $_ ne q{} } $input->multi_param('branches'); my @branches = grep { $_ ne q{} } $input->multi_param('branches');
my $notforloan = $input->param('notforloan') ? 1 : 0; my $notforloan = $input->param('notforloan') ? 1 : 0;
my $imageurl = my $imageurl =
$image eq 'removeImage' ? '' $image eq 'removeImage' ? ''
: ( : (
$image eq 'remoteImage' ? $input->param('remoteImage') $image eq 'remoteImage' ? $input->param('remoteImage')
: $image : $image
); );
my $summary = $input->param('summary'); my $summary = $input->param('summary');
my $checkinmsg = $input->param('checkinmsg'); my $checkinmsg = $input->param('checkinmsg');
my $checkinmsgtype = $input->param('checkinmsgtype'); my $checkinmsgtype = $input->param('checkinmsgtype');
my $hideinopac = $input->param('hideinopac') // 0; my $hideinopac = $input->param('hideinopac') // 0;
my $searchcategory = $input->param('searchcategory'); my $searchcategory = $input->param('searchcategory');
my $rentalcharge_daily_calendar = $input->param('rentalcharge_daily_calendar') // 0; my $rentalcharge_daily_calendar = $input->param('rentalcharge_daily_calendar') // 0;
my $rentalcharge_hourly_calendar = $input->param('rentalcharge_hourly_calendar') // 0; my $rentalcharge_hourly_calendar = $input->param('rentalcharge_hourly_calendar') // 0;
my $automatic_checkin = $input->param('automatic_checkin') // 0; my $automatic_checkin = $input->param('automatic_checkin') // 0;
my $bookable = $input->param('bookable')// 0; my $bookable = $input->param('bookable') // 0;
if ( $itemtype and $is_a_modif ) { # it's a modification if ( $itemtype and $is_a_modif ) { # it's a modification
$itemtype->description($description); $itemtype->description($description);
@ -122,8 +122,8 @@ if ( $op eq 'add_form' ) {
$itemtype->bookable($bookable); $itemtype->bookable($bookable);
eval { eval {
$itemtype->store; $itemtype->store;
$itemtype->replace_library_limits( \@branches ); $itemtype->replace_library_limits( \@branches );
}; };
if ($@) { if ($@) {
@ -134,31 +134,31 @@ if ( $op eq 'add_form' ) {
} elsif ( not $itemtype and not $is_a_modif ) { } elsif ( not $itemtype and not $is_a_modif ) {
my $itemtype = Koha::ItemType->new( my $itemtype = Koha::ItemType->new(
{ {
itemtype => $itemtype_code, itemtype => $itemtype_code,
description => $description, description => $description,
parent_type => $parent_type, parent_type => $parent_type,
rentalcharge => $rentalcharge, rentalcharge => $rentalcharge,
rentalcharge_daily => $rentalcharge_daily, rentalcharge_daily => $rentalcharge_daily,
rentalcharge_hourly => $rentalcharge_hourly, rentalcharge_hourly => $rentalcharge_hourly,
defaultreplacecost => $defaultreplacecost, defaultreplacecost => $defaultreplacecost,
processfee => $processfee, processfee => $processfee,
notforloan => $notforloan, notforloan => $notforloan,
imageurl => $imageurl, imageurl => $imageurl,
summary => $summary, summary => $summary,
checkinmsg => $checkinmsg, checkinmsg => $checkinmsg,
checkinmsgtype => $checkinmsgtype, checkinmsgtype => $checkinmsgtype,
sip_media_type => $sip_media_type, sip_media_type => $sip_media_type,
hideinopac => $hideinopac, hideinopac => $hideinopac,
searchcategory => $searchcategory, searchcategory => $searchcategory,
rentalcharge_daily_calendar => $rentalcharge_daily_calendar, rentalcharge_daily_calendar => $rentalcharge_daily_calendar,
rentalcharge_hourly_calendar => $rentalcharge_hourly_calendar, rentalcharge_hourly_calendar => $rentalcharge_hourly_calendar,
automatic_checkin => $automatic_checkin, automatic_checkin => $automatic_checkin,
bookable => $bookable, bookable => $bookable,
} }
); );
eval { eval {
$itemtype->store; $itemtype->store;
$itemtype->replace_library_limits( \@branches ); $itemtype->replace_library_limits( \@branches );
}; };
if ($@) { if ($@) {
@ -168,19 +168,20 @@ if ( $op eq 'add_form' ) {
} }
} else { } else {
push @messages, push @messages,
{ type => 'alert', {
type => 'alert',
code => 'already_exists', code => 'already_exists',
}; };
} }
$searchfield = ''; $searchfield = '';
$op = 'list'; $op = 'list';
} elsif ( $op eq 'delete_confirm' ) { } elsif ( $op eq 'delete_confirm' ) {
my $itemtype = Koha::ItemTypes->find($itemtype_code); my $itemtype = Koha::ItemTypes->find($itemtype_code);
my $can_be_deleted = $itemtype->can_be_deleted(); my $can_be_deleted = $itemtype->can_be_deleted();
if ($can_be_deleted == 0) { if ( $can_be_deleted == 0 ) {
push @messages, { type => 'alert', code => 'cannot_be_deleted'}; push @messages, { type => 'alert', code => 'cannot_be_deleted' };
$op = 'list'; $op = 'list';
} else { } else {
$template->param( itemtype => $itemtype, ); $template->param( itemtype => $itemtype, );
@ -189,7 +190,7 @@ if ( $op eq 'add_form' ) {
} elsif ( $op eq 'cud-delete_confirmed' ) { } elsif ( $op eq 'cud-delete_confirmed' ) {
my $itemtype = Koha::ItemTypes->find($itemtype_code); my $itemtype = Koha::ItemTypes->find($itemtype_code);
my $deleted = eval { $itemtype->delete }; my $deleted = eval { $itemtype->delete };
if ( $@ or not $deleted ) { if ( $@ or not $deleted ) {
push @messages, { type => 'alert', code => 'error_on_delete' }; push @messages, { type => 'alert', code => 'error_on_delete' };
} else { } else {

32
t/db_dependent/Koha/Items.t
View file

@ -2180,20 +2180,26 @@ subtest 'filter_by_bookable' => sub {
$schema->storage->txn_begin; $schema->storage->txn_begin;
t::lib::Mocks::mock_preference('item-level_itypes', 0); t::lib::Mocks::mock_preference( 'item-level_itypes', 0 );
my $bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 1 } } ); my $bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 1 } } );
my $non_bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 0 } } ); my $non_bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 0 } } );
my $biblio = $builder->build_sample_biblio({ itemtype => $bookable_item_type->itemtype }); my $biblio = $builder->build_sample_biblio( { itemtype => $bookable_item_type->itemtype } );
# bookable items # bookable items
my $bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $bookable_item_type->itemtype, bookable => 1 } ); my $bookable_item1 = $builder->build_sample_item(
{ biblionumber => $biblio->biblionumber, itype => $bookable_item_type->itemtype, bookable => 1 } );
# not bookable items # not bookable items
my $non_bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } ); my $non_bookable_item1 = $builder->build_sample_item(
my $non_bookable_item2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } ); { biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } );
my $non_bookable_item2 = $builder->build_sample_item(
{ biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } );
is( $biblio->items->filter_by_bookable->count, 1, "filter_by_bookable returns the correct number of items set at item level" ); is(
$biblio->items->filter_by_bookable->count, 1,
"filter_by_bookable returns the correct number of items set at item level"
);
is( is(
$biblio->items->filter_by_bookable->next->itemnumber, $bookable_item1->itemnumber, $biblio->items->filter_by_bookable->next->itemnumber, $bookable_item1->itemnumber,
"the correct item is returned from filter_by_bookable at the item level" "the correct item is returned from filter_by_bookable at the item level"
@ -2203,10 +2209,16 @@ subtest 'filter_by_bookable' => sub {
$non_bookable_item1->bookable(undef)->store(); $non_bookable_item1->bookable(undef)->store();
$non_bookable_item2->bookable(undef)->store(); $non_bookable_item2->bookable(undef)->store();
$biblio->get_from_storage; $biblio->get_from_storage;
is( $biblio->items->filter_by_bookable->count, 3, "filter_by_bookable returns the correct number of items when not set at item level and using biblio level itemtypes" ); is(
$biblio->items->filter_by_bookable->count, 3,
"filter_by_bookable returns the correct number of items when not set at item level and using biblio level itemtypes"
);
t::lib::Mocks::mock_preference('item-level_itypes', 1); t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
is( $biblio->items->filter_by_bookable->count, 1, "filter_by_bookable returns the correct number of items when not set at item level and using item level itemtypes" ); is(
$biblio->items->filter_by_bookable->count, 1,
"filter_by_bookable returns the correct number of items when not set at item level and using item level itemtypes"
);
$schema->storage->txn_rollback; $schema->storage->txn_rollback;
}; };