From 500a732e095eae3ad7716df9be129e5e1815a2aa Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 11 Jun 2021 19:17:27 +0000 Subject: [PATCH] Bug 14237: (follow-up) Make the routines exclusively take itemnumber, biblionumber, or ci_id This patch changes the parameters for several of the CourseReserves routines to take a single identifier exclusively. Following the existing pattern we simply return if the params are incorrect This patch also: removes an unused 'title' variable adds a prefetch to save some db calls where we fetch related objects adjusts tests Signed-off-by: Nick Clemens --- C4/CourseReserves.pm | 25 +++++++++++++--------- Koha/Schema/Result/CourseItem.pm | 8 +++++-- course_reserves/add_items.pl | 3 --- course_reserves/batch_add_items.pl | 2 +- .../en/modules/course_reserves/add_items-step2.tt | 1 - t/db_dependent/CourseReserves/CourseItems.t | 8 ------- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm index c944886471..f1bd4b163a 100644 --- a/C4/CourseReserves.pm +++ b/C4/CourseReserves.pm @@ -414,7 +414,11 @@ sub ModCourseInstructors { =head2 GetCourseItem { - $course_item = GetCourseItem( itemnumber => $itemnumber [, ci_id => $ci_id ); + Given one of biblionumber, itenumber, or ci_id, returns hashref of the course_items values + + $course_item = GetCourseItem( itemnumber => $itemnumber [, ci_id => $ci_id ] ); + $course_item = GetCourseItem( biblionumber => $biblionumber [, ci_id => $ci_id ]); + $course_item = GetCourseItem( ci_id => $ci_id ); =cut @@ -426,7 +430,7 @@ sub GetCourseItem { my $itemnumber = $params{'itemnumber'}; my $biblionumber = $params{'biblionumber'}; - return unless ( $itemnumber || $biblionumber || $ci_id ); + return unless ( $itemnumber xor $biblionumber xor $ci_id ); my ( $field, $value ); if ( $itemnumber ) { @@ -463,7 +467,7 @@ sub GetCourseItem { ModCourseItem( %params ); - Creates or modifies an existing course item. + Creates or modifies an existing course item. Must be passed either an itemnumber or biblionumber parameter =cut @@ -474,7 +478,7 @@ sub ModCourseItem { my $itemnumber = $params{'itemnumber'}; my $biblionumber = $params{'biblionumber'}; - return unless ($itemnumber || $biblionumber); + return unless ($itemnumber xor $biblionumber); my $course_item = $itemnumber ? GetCourseItem( itemnumber => $itemnumber ) : GetCourseItem( biblionumber => $biblionumber ); @@ -509,6 +513,7 @@ sub ModCourseItem { sub _AddCourseItem { my (%params) = @_; + warn "adding"; warn identify_myself(%params) if $DEBUG; $params{homebranch} ||= undef; # Can't be empty string, FK constraint @@ -883,8 +888,8 @@ sub GetCourseReserves { if ($include_items) { foreach my $cr (@$course_reserves) { - my $item = Koha::Items->find( $cr->{itemnumber} ); - my $biblio = $cr->{itemnumber} ? $item->biblio : Koha::Biblios->find( $cr->{biblionumber} ); + my $item = Koha::Items->find( $cr->{itemnumber}, { prefetch => ['biblio','biblioitem'] }); + my $biblio = $cr->{itemnumber} ? $item->biblio : Koha::Biblios->find( $cr->{biblionumber}, { prefetch => ['biblioitem'] }); my $biblioitem = $biblio->biblioitem; $cr->{'course_item'} = GetCourseItem( ci_id => $cr->{'ci_id'} ); $cr->{'item'} = $item; @@ -946,7 +951,7 @@ sub DelCourseReserve { my $arrayref = GetItemCourseReservesInfo( itemnumber => $itemnumber ); my $arrayref = GetItemCourseReservesInfo( biblionumber => $biblionumber ); - For a given item, returns an arrayref of reserves hashrefs, + For a given itemnumber or biblionumber, returns an arrayref of reserves hashrefs, with a course hashref under the key 'course' =cut @@ -958,7 +963,7 @@ sub GetItemCourseReservesInfo { my $itemnumber = $params{'itemnumber'}; my $biblionumber = $params{'biblionumber'}; - return unless ($itemnumber || $biblionumber); + return unless ($itemnumber xor $biblionumber); my $course_item = $itemnumber ? GetCourseItem( itemnumber => $itemnumber ) : GetCourseItem( biblionumber => $biblionumber ); @@ -998,9 +1003,9 @@ sub CountCourseReservesForItem { my $enabled = $params{'enabled'}; my $biblionumber = $params{'biblionumber'}; - return unless ( $ci_id || ( $itemnumber || $biblionumber ) ); + return unless ( $ci_id xor $itemnumber xor $biblionumber ); - my $course_item = $itemnumber ? GetCourseItem( ci_id => $ci_id, itemnumber => $itemnumber ) : GetCourseItem( ci_id => $ci_id, biblionumber => $biblionumber ); + my $course_item = GetCourseItem( ci_id => $ci_id ) || GetCourseItem( itemnumber => $itemnumber ) || GetCourseItem( biblionumber => $biblionumber ); my @params = ( $course_item->{'ci_id'} ); push( @params, $enabled ) if ($enabled); diff --git a/Koha/Schema/Result/CourseItem.pm b/Koha/Schema/Result/CourseItem.pm index 0cf436962d..53a62686e4 100644 --- a/Koha/Schema/Result/CourseItem.pm +++ b/Koha/Schema/Result/CourseItem.pm @@ -37,12 +37,16 @@ course item id is_foreign_key: 1 is_nullable: 1 +items.itemnumber for the item on reserve + =head2 biblionumber data_type: 'integer' is_foreign_key: 1 is_nullable: 0 +biblio.biblionumber for the bibliographic record on reserve + =head2 itype data_type: 'varchar' @@ -376,8 +380,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-02-03 10:03:26 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:kImBuor/tqSbLlPDt+d2fg +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-06-11 18:35:38 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3fFJ3uJr+5pMZniZ1glIpg __PACKAGE__->add_columns( '+itype_enabled' => { is_boolean => 1 }, diff --git a/course_reserves/add_items.pl b/course_reserves/add_items.pl index ec7906775c..02999c2e0b 100755 --- a/course_reserves/add_items.pl +++ b/course_reserves/add_items.pl @@ -54,15 +54,12 @@ if ( $barcode || $itemnumber ) { if ( $item ) { $itemnumber = $item->id; $biblio = $item->biblio; - $biblionumber = $biblio->biblionumber; } } else { # adding a biblio to course items $biblio = Koha::Biblios->find( $biblionumber ); } -my $title = $biblio->title if $biblio; - my $step = ( $action eq 'lookup' && ( $item or $biblio ) ) ? '2' : '1'; my $tmpl = ($course_id) ? "add_items-step$step.tt" : "invalid-course.tt"; diff --git a/course_reserves/batch_add_items.pl b/course_reserves/batch_add_items.pl index 4e454201c5..8343da2b7d 100755 --- a/course_reserves/batch_add_items.pl +++ b/course_reserves/batch_add_items.pl @@ -86,7 +86,7 @@ if ( $course_id && $course ) { foreach my $item (@items) { my $ci_id = ModCourseItem( itemnumber => $item->id, - biblionumber => $item->biblionumber, + biblionumber => undef, itype => $itype, ccode => $ccode, holdingbranch => $holdingbranch, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt index 6098b397c1..2efd1a2a2c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt @@ -58,7 +58,6 @@ Barcode: [% item.barcode | html %] - [% IF item_level_itypes %] diff --git a/t/db_dependent/CourseReserves/CourseItems.t b/t/db_dependent/CourseReserves/CourseItems.t index f5224143e1..2e6f9eedbb 100755 --- a/t/db_dependent/CourseReserves/CourseItems.t +++ b/t/db_dependent/CourseReserves/CourseItems.t @@ -39,7 +39,6 @@ my ($biblionumber, $itemnumber) = create_bib_and_item(); my $ci_id = ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 1, ccode_enabled => 1, homebranch_enabled => 1, @@ -83,7 +82,6 @@ is($item->location, 'TH', 'Item location in course should be TH'); ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 1, ccode_enabled => 1, homebranch_enabled => 1, @@ -160,7 +158,6 @@ is($course_item2->{ccode_storage}, '', 'Course item ccode storage should be empt ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 1, ccode_enabled => 1, homebranch_enabled => 1, @@ -185,7 +182,6 @@ is($item->ccode, 'DVD', 'Item ccode should be DVD'); ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 1, ccode_enabled => 1, homebranch_enabled => 0, # LEAVE UNCHANGED @@ -212,7 +208,6 @@ subtest 'Ensure modifying fields on existing course items updates the item and c my ($biblionumber, $itemnumber) = create_bib_and_item(); my $ci_id = ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 0, ccode_enabled => 0, homebranch_enabled => 0, @@ -304,7 +299,6 @@ subtest 'Ensure modifying fields on existing course items updates the item and c # Test removing fields from an active course item ModCourseItem( itemnumber => $itemnumber, - biblionumber => $biblionumber, itype_enabled => 0, ccode_enabled => 0, homebranch_enabled => 0, @@ -352,7 +346,6 @@ subtest 'Ensure item info is preserved' => sub { #Add course item but change nothing my $course_item_id = ModCourseItem( itemnumber => $item->itemnumber, - biblionumber => $biblionumber, itype => '', ccode => '', holdingbranch => '', @@ -383,7 +376,6 @@ subtest 'Ensure item info is preserved' => sub { #Add course item but change nothing $course_item_id = ModCourseItem( itemnumber => $item->itemnumber, - biblionumber => $biblionumber, itype => '', ccode => '', holdingbranch => '', -- 2.11.0