Bugzilla – Attachment 173331 Details for
Bug 35906
Add bookable option on itemtypes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t
Bug-35906-QA-follow-up-Tidy-adminitemtypespl-Items.patch (text/plain), 13.75 KB, created by
Paul Derscheid
on 2024-10-25 09:29:44 UTC
(
hide
)
Description:
Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t
Filename:
MIME Type:
Creator:
Paul Derscheid
Created:
2024-10-25 09:29:44 UTC
Size:
13.75 KB
patch
obsolete
>From 87649062b6aeb7d763d951be23c6a66310c3ce0e Mon Sep 17 00:00:00 2001 >From: Paul Derscheid <paul.derscheid@lmscloud.de> >Date: Fri, 18 Oct 2024 09:56:50 +0000 >Subject: [PATCH] Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t > >Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> >--- > admin/itemtypes.pl | 131 ++++++++++++++++++------------------ > t/db_dependent/Koha/Items.t | 32 ++++++--- > 2 files changed, 88 insertions(+), 75 deletions(-) > >diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl >index b5dbc7c98c..7eceef6b1a 100755 >--- a/admin/itemtypes.pl >+++ b/admin/itemtypes.pl >@@ -25,10 +25,9 @@ > use Modern::Perl; > use CGI qw ( -utf8 ); > >- > use C4::Koha qw( getImageSets GetAuthorisedValues ); > 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 Koha::ItemTypes; > use Koha::ItemType; >@@ -41,10 +40,11 @@ my $op = $input->param('op') // 'list'; > my @messages; > $searchfield =~ s/\,//g if $searchfield; > my ( $template, $borrowernumber, $cookie ) = get_template_and_user( >- { template_name => "admin/itemtypes.tt", >- query => $input, >- type => "intranet", >- flagsrequired => { parameters => 'manage_itemtypes' }, >+ { >+ template_name => "admin/itemtypes.tt", >+ query => $input, >+ 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' ) { > my $itemtype = Koha::ItemTypes->find($itemtype_code); > >- my $parent_type = $itemtype ? $itemtype->parent_type : undef; >- my $parent_types = Koha::ItemTypes->search({parent_type=>undef,itemtype => {'!='=>$itemtype_code}}); >- my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) ); >+ my $parent_type = $itemtype ? $itemtype->parent_type : undef; >+ my $parent_types = Koha::ItemTypes->search( { parent_type => undef, itemtype => { '!=' => $itemtype_code } } ); >+ my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) ); > my $searchcategory = GetAuthorisedValues("ITEMTYPECAT"); > my $translated_languages = C4::Languages::getTranslatedLanguages( "both", C4::Context->preference('template') ); > $template->param( >- itemtype => $itemtype, >- parent_type => $parent_type, >- parent_types => $parent_types, >- is_a_parent => $itemtype ? Koha::ItemTypes->search({parent_type=>$itemtype_code})->count : 0, >- imagesets => $imagesets, >- searchcategory => $searchcategory, >+ itemtype => $itemtype, >+ parent_type => $parent_type, >+ parent_types => $parent_types, >+ is_a_parent => $itemtype ? Koha::ItemTypes->search( { parent_type => $itemtype_code } )->count : 0, >+ imagesets => $imagesets, >+ searchcategory => $searchcategory, > can_be_translated => ( scalar(@$translated_languages) > 1 ? 1 : 0 ), > ); > } elsif ( $op eq 'cud-add_validate' ) { >- my $is_a_modif = $input->param('is_a_modif'); >- my $itemtype = Koha::ItemTypes->find($itemtype_code); >- my $parent_type = $input->param('parent_type') || undef; >- my $description = $input->param('description'); >- my $rentalcharge = $input->param('rentalcharge'); >- my $rentalcharge_daily = $input->param('rentalcharge_daily'); >+ my $is_a_modif = $input->param('is_a_modif'); >+ my $itemtype = Koha::ItemTypes->find($itemtype_code); >+ my $parent_type = $input->param('parent_type') || undef; >+ my $description = $input->param('description'); >+ my $rentalcharge = $input->param('rentalcharge'); >+ my $rentalcharge_daily = $input->param('rentalcharge_daily'); > my $rentalcharge_hourly = $input->param('rentalcharge_hourly'); >- my $defaultreplacecost = $input->param('defaultreplacecost'); >- my $processfee = $input->param('processfee'); >- my $image = $input->param('image') || q||; >- my @branches = grep { $_ ne q{} } $input->multi_param('branches'); >+ my $defaultreplacecost = $input->param('defaultreplacecost'); >+ my $processfee = $input->param('processfee'); >+ my $image = $input->param('image') || q||; >+ my @branches = grep { $_ ne q{} } $input->multi_param('branches'); > > my $notforloan = $input->param('notforloan') ? 1 : 0; > my $imageurl = >- $image eq 'removeImage' ? '' >- : ( >+ $image eq 'removeImage' ? '' >+ : ( > $image eq 'remoteImage' ? $input->param('remoteImage') > : $image >- ); >- my $summary = $input->param('summary'); >- my $checkinmsg = $input->param('checkinmsg'); >- my $checkinmsgtype = $input->param('checkinmsgtype'); >- my $hideinopac = $input->param('hideinopac') // 0; >- my $searchcategory = $input->param('searchcategory'); >- my $rentalcharge_daily_calendar = $input->param('rentalcharge_daily_calendar') // 0; >+ ); >+ my $summary = $input->param('summary'); >+ my $checkinmsg = $input->param('checkinmsg'); >+ my $checkinmsgtype = $input->param('checkinmsgtype'); >+ my $hideinopac = $input->param('hideinopac') // 0; >+ my $searchcategory = $input->param('searchcategory'); >+ my $rentalcharge_daily_calendar = $input->param('rentalcharge_daily_calendar') // 0; > my $rentalcharge_hourly_calendar = $input->param('rentalcharge_hourly_calendar') // 0; >- my $automatic_checkin = $input->param('automatic_checkin') // 0; >- my $bookable = $input->param('bookable')// 0; >+ my $automatic_checkin = $input->param('automatic_checkin') // 0; >+ my $bookable = $input->param('bookable') // 0; > > if ( $itemtype and $is_a_modif ) { # it's a modification > $itemtype->description($description); >@@ -122,8 +122,8 @@ if ( $op eq 'add_form' ) { > $itemtype->bookable($bookable); > > eval { >- $itemtype->store; >- $itemtype->replace_library_limits( \@branches ); >+ $itemtype->store; >+ $itemtype->replace_library_limits( \@branches ); > }; > > if ($@) { >@@ -134,31 +134,31 @@ if ( $op eq 'add_form' ) { > } elsif ( not $itemtype and not $is_a_modif ) { > my $itemtype = Koha::ItemType->new( > { >- itemtype => $itemtype_code, >- description => $description, >- parent_type => $parent_type, >- rentalcharge => $rentalcharge, >- rentalcharge_daily => $rentalcharge_daily, >- rentalcharge_hourly => $rentalcharge_hourly, >- defaultreplacecost => $defaultreplacecost, >- processfee => $processfee, >- notforloan => $notforloan, >- imageurl => $imageurl, >- summary => $summary, >- checkinmsg => $checkinmsg, >- checkinmsgtype => $checkinmsgtype, >- sip_media_type => $sip_media_type, >- hideinopac => $hideinopac, >- searchcategory => $searchcategory, >+ itemtype => $itemtype_code, >+ description => $description, >+ parent_type => $parent_type, >+ rentalcharge => $rentalcharge, >+ rentalcharge_daily => $rentalcharge_daily, >+ rentalcharge_hourly => $rentalcharge_hourly, >+ defaultreplacecost => $defaultreplacecost, >+ processfee => $processfee, >+ notforloan => $notforloan, >+ imageurl => $imageurl, >+ summary => $summary, >+ checkinmsg => $checkinmsg, >+ checkinmsgtype => $checkinmsgtype, >+ sip_media_type => $sip_media_type, >+ hideinopac => $hideinopac, >+ searchcategory => $searchcategory, > rentalcharge_daily_calendar => $rentalcharge_daily_calendar, > rentalcharge_hourly_calendar => $rentalcharge_hourly_calendar, >- automatic_checkin => $automatic_checkin, >- bookable => $bookable, >+ automatic_checkin => $automatic_checkin, >+ bookable => $bookable, > } > ); > eval { >- $itemtype->store; >- $itemtype->replace_library_limits( \@branches ); >+ $itemtype->store; >+ $itemtype->replace_library_limits( \@branches ); > }; > > if ($@) { >@@ -168,19 +168,20 @@ if ( $op eq 'add_form' ) { > } > } else { > push @messages, >- { type => 'alert', >+ { >+ type => 'alert', > code => 'already_exists', >- }; >+ }; > } > > $searchfield = ''; > $op = 'list'; > >- } elsif ( $op eq 'delete_confirm' ) { >- my $itemtype = Koha::ItemTypes->find($itemtype_code); >+} elsif ( $op eq 'delete_confirm' ) { >+ my $itemtype = Koha::ItemTypes->find($itemtype_code); > my $can_be_deleted = $itemtype->can_be_deleted(); >- if ($can_be_deleted == 0) { >- push @messages, { type => 'alert', code => 'cannot_be_deleted'}; >+ if ( $can_be_deleted == 0 ) { >+ push @messages, { type => 'alert', code => 'cannot_be_deleted' }; > $op = 'list'; > } else { > $template->param( itemtype => $itemtype, ); >@@ -189,7 +190,7 @@ if ( $op eq 'add_form' ) { > } elsif ( $op eq 'cud-delete_confirmed' ) { > > my $itemtype = Koha::ItemTypes->find($itemtype_code); >- my $deleted = eval { $itemtype->delete }; >+ my $deleted = eval { $itemtype->delete }; > if ( $@ or not $deleted ) { > push @messages, { type => 'alert', code => 'error_on_delete' }; > } else { >diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t >index 61a3909a0b..03b26550eb 100755 >--- a/t/db_dependent/Koha/Items.t >+++ b/t/db_dependent/Koha/Items.t >@@ -2180,20 +2180,26 @@ subtest 'filter_by_bookable' => sub { > > $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 $biblio = $builder->build_sample_biblio({ itemtype => $bookable_item_type->itemtype }); >+ my $biblio = $builder->build_sample_biblio( { itemtype => $bookable_item_type->itemtype } ); > > # 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 >- my $non_bookable_item1 = $builder->build_sample_item( { 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 } ); >+ my $non_bookable_item1 = $builder->build_sample_item( >+ { 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( > $biblio->items->filter_by_bookable->next->itemnumber, $bookable_item1->itemnumber, > "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_item2->bookable(undef)->store(); > $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); >- 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" ); >+ 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" >+ ); > > $schema->storage->txn_rollback; > }; >-- >2.47.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=173331">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35906
:
161397
|
161638
|
161744
|
161961
|
162242
|
162365
|
162465
|
162466
|
162467
|
162468
|
162469
|
162470
|
162472
|
163291
|
163292
|
163293
|
163294
|
163295
|
163547
|
163548
|
163549
|
163550
|
163551
|
164579
|
164580
|
164581
|
164582
|
164583
|
164584
|
164585
|
165437
|
166866
|
166867
|
166868
|
166869
|
166870
|
166871
|
168120
|
168121
|
168122
|
168160
|
168161
|
168162
|
168163
|
168164
|
170187
|
170188
|
170189
|
170190
|
170191
|
170582
|
170583
|
170584
|
170585
|
170586
|
172127
|
172128
|
172129
|
172130
|
172131
|
172132
|
172941
|
172945
|
172950
|
172951
|
172952
|
172953
|
172954
|
172955
|
172956
|
172957
|
172958
|
172959
|
172960
|
172961
|
173324
|
173325
|
173326
|
173327
|
173328
|
173329
|
173330
| 173331 |
173332
|
173333
|
173334
|
173335
|
173893