Bug 10382: Course reserves: handle empty values
Test Plan:
1) Create an item, do not set a collection code
2) Add the item to a course, and choose to set a collection code
3) Ensure the course is enabled, and the collection code is now visible
4) Disable the course, ensure the collection code is no longer visible
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 9c6025ccbf)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Alex Arnaud
Martin Renvoize
parent
84bf7c9468
commit
e9c0c22630
|
|
@ -459,11 +459,7 @@ sub ModCourseItem {
|
|||
my (%params) = @_;
|
||||
warn identify_myself(%params) if $DEBUG;
|
||||
|
||||
my $itemnumber = $params{'itemnumber'};
|
||||
my $itype = $params{'itype'};
|
||||
my $ccode = $params{'ccode'};
|
||||
my $holdingbranch = $params{'holdingbranch'};
|
||||
my $location = $params{'location'};
|
||||
my $itemnumber = $params{'itemnumber'};
|
||||
|
||||
return unless ($itemnumber);
|
||||
|
||||
|
|
@ -503,10 +499,8 @@ sub _AddCourseItem {
|
|||
push( @values, $params{'itemnumber'} );
|
||||
|
||||
foreach (@FIELDS) {
|
||||
if ( $params{$_} ) {
|
||||
push( @fields, "$_ = ?" );
|
||||
push( @values, $params{$_} );
|
||||
}
|
||||
push( @fields, "$_ = ?" );
|
||||
push( @values, $params{$_} || undef );
|
||||
}
|
||||
|
||||
my $query = "INSERT INTO course_items SET " . join( ',', @fields );
|
||||
|
|
@ -530,10 +524,6 @@ sub _UpdateCourseItem {
|
|||
|
||||
my $ci_id = $params{'ci_id'};
|
||||
my $course_item = $params{'course_item'};
|
||||
my $itype = $params{'itype'};
|
||||
my $ccode = $params{'ccode'};
|
||||
my $holdingbranch = $params{'holdingbranch'};
|
||||
my $location = $params{'location'};
|
||||
|
||||
return unless ( $ci_id || $course_item );
|
||||
|
||||
|
|
@ -541,49 +531,13 @@ sub _UpdateCourseItem {
|
|||
unless ($course_item);
|
||||
$ci_id = $course_item->{'ci_id'} unless ($ci_id);
|
||||
|
||||
## Revert fields that had an 'original' value, but now don't
|
||||
## Update the item fields to the stored values from course_items
|
||||
## and then set those fields in course_items to NULL
|
||||
my @fields_to_revert;
|
||||
foreach (@FIELDS) {
|
||||
if ( !$params{$_} && $course_item->{$_} ) {
|
||||
push( @fields_to_revert, $_ );
|
||||
}
|
||||
}
|
||||
_RevertFields(
|
||||
ci_id => $ci_id,
|
||||
fields => \@fields_to_revert,
|
||||
course_item => $course_item
|
||||
) if (@fields_to_revert);
|
||||
|
||||
## Update fields that still have an original value, but it has changed
|
||||
## This necessitates only changing the current item values, as we still
|
||||
## have the original values stored in course_items
|
||||
my %mod_params;
|
||||
foreach (@FIELDS) {
|
||||
if ( $params{$_}
|
||||
&& $course_item->{$_}
|
||||
&& $params{$_} ne $course_item->{$_} ) {
|
||||
$mod_params{$_} = $params{$_};
|
||||
}
|
||||
$mod_params{$_} = $params{$_};
|
||||
}
|
||||
ModItem( \%mod_params, undef, $course_item->{'itemnumber'} ) if %mod_params;
|
||||
|
||||
## Update fields that didn't have an original value, but now do
|
||||
## We must save the original value in course_items, and also
|
||||
## update the item fields to the new value
|
||||
my $item = GetItem( $course_item->{'itemnumber'} );
|
||||
my %mod_params_new;
|
||||
my %mod_params_old;
|
||||
foreach (@FIELDS) {
|
||||
if ( $params{$_} && !$course_item->{$_} ) {
|
||||
$mod_params_new{$_} = $params{$_};
|
||||
$mod_params_old{$_} = $item->{$_};
|
||||
}
|
||||
}
|
||||
_ModStoredFields( 'ci_id' => $params{'ci_id'}, %mod_params_old );
|
||||
ModItem( \%mod_params_new, undef, $course_item->{'itemnumber'} ) if %mod_params_new;
|
||||
|
||||
ModItem( \%mod_params, undef, $course_item->{'itemnumber'} );
|
||||
}
|
||||
|
||||
=head2 _ModStoredFields
|
||||
|
|
@ -627,31 +581,18 @@ sub _RevertFields {
|
|||
my (%params) = @_;
|
||||
warn identify_myself(%params) if $DEBUG;
|
||||
|
||||
my $ci_id = $params{'ci_id'};
|
||||
my $course_item = $params{'course_item'};
|
||||
my $fields = $params{'fields'};
|
||||
my @fields = @$fields;
|
||||
my $ci_id = $params{'ci_id'};
|
||||
|
||||
return unless ($ci_id);
|
||||
|
||||
$course_item = GetCourseItem( ci_id => $params{'ci_id'} )
|
||||
unless ($course_item);
|
||||
my $course_item = GetCourseItem( ci_id => $params{'ci_id'} );
|
||||
|
||||
my $mod_item_params;
|
||||
my @fields_to_null;
|
||||
foreach my $field (@fields) {
|
||||
foreach (@FIELDS) {
|
||||
if ( $field eq $_ && $course_item->{$_} ) {
|
||||
$mod_item_params->{$_} = $course_item->{$_};
|
||||
push( @fields_to_null, $_ );
|
||||
}
|
||||
}
|
||||
foreach my $field ( @FIELDS ) {
|
||||
$mod_item_params->{$field} = $course_item->{$field};
|
||||
}
|
||||
|
||||
ModItem( $mod_item_params, undef, $course_item->{'itemnumber'} ) if $mod_item_params && %$mod_item_params;
|
||||
|
||||
my $query = "UPDATE course_items SET " . join( ',', map { "$_=NULL" } @fields_to_null ) . " WHERE ci_id = ?";
|
||||
|
||||
C4::Context->dbh->do( $query, undef, $ci_id ) if (@fields_to_null);
|
||||
}
|
||||
|
||||
=head2 _SwapAllFields
|
||||
|
|
@ -736,7 +677,7 @@ sub DelCourseItem {
|
|||
|
||||
return unless ($ci_id);
|
||||
|
||||
_RevertFields( ci_id => $ci_id, fields => \@FIELDS );
|
||||
_RevertFields( ci_id => $ci_id );
|
||||
|
||||
my $query = "
|
||||
DELETE FROM course_items
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ use C4::CourseReserves qw/ModCourseItem ModCourseReserve DelCourseReserve GetCou
|
|||
use C4::Context;
|
||||
use Koha::Items;
|
||||
|
||||
use Test::More tests => 17;
|
||||
use Test::More tests => 27;
|
||||
|
||||
BEGIN {
|
||||
require_ok('C4::CourseReserves');
|
||||
|
|
@ -37,10 +37,6 @@ my $builder = t::lib::TestBuilder->new();
|
|||
create_dependent_objects();
|
||||
my ($biblionumber, $itemnumber) = create_bib_and_item();
|
||||
|
||||
my $course = $builder->build({
|
||||
source => 'CourseReserve',
|
||||
});
|
||||
|
||||
my $ci_id = ModCourseItem(
|
||||
itemnumber => $itemnumber,
|
||||
itype => 'BK_foo',
|
||||
|
|
@ -49,6 +45,13 @@ my $ci_id = ModCourseItem(
|
|||
location => 'TH',
|
||||
);
|
||||
|
||||
my $course = $builder->build({
|
||||
source => 'CourseReserve',
|
||||
value => {
|
||||
ci_id => $ci_id,
|
||||
}
|
||||
});
|
||||
|
||||
my $cr_id = ModCourseReserve(
|
||||
course_id => $course->{course_id},
|
||||
ci_id => $ci_id,
|
||||
|
|
@ -68,6 +71,33 @@ is($item->ccode, 'BOOK', 'Item ccode in course should be BOOK');
|
|||
is($item->holdingbranch, 'B2', 'Item holding branch in course should be B2');
|
||||
is($item->location, 'TH', 'Item location in course should be TH');
|
||||
|
||||
ModCourseItem(
|
||||
itemnumber => $itemnumber,
|
||||
itype => 'BK_foo',
|
||||
ccode => 'DVD',
|
||||
holdingbranch => 'B3',
|
||||
location => 'TH',
|
||||
);
|
||||
|
||||
ModCourseReserve(
|
||||
course_id => $course->{course_id},
|
||||
ci_id => $ci_id,
|
||||
staff_note => '',
|
||||
public_note => '',
|
||||
);
|
||||
|
||||
$course_item = GetCourseItem( ci_id => $ci_id );
|
||||
is($course_item->{itype}, 'CD_foo', 'Course item itype should be CD_foo');
|
||||
is($course_item->{ccode}, 'CD', 'Course item ccode should be CD');
|
||||
is($course_item->{holdingbranch}, 'B1', 'Course item holding branch should be B1');
|
||||
is($course_item->{location}, 'HR', 'Course item location should be HR');
|
||||
|
||||
$item = Koha::Items->find($itemnumber);
|
||||
is($item->itype, 'BK_foo', 'Item type in course should be BK_foo');
|
||||
is($item->ccode, 'DVD', 'Item ccode in course should be DVD');
|
||||
is($item->holdingbranch, 'B3', 'Item holding branch in course should be B3');
|
||||
is($item->location, 'TH', 'Item location in course should be TH');
|
||||
|
||||
DelCourseReserve( cr_id => $cr_id );
|
||||
$item = Koha::Items->find($itemnumber);
|
||||
is($item->itype, 'CD_foo', 'Item type removed from course should be set back to CD_foo');
|
||||
|
|
@ -102,6 +132,27 @@ is($item->ccode, 'BOOK', 'Item ccode should be BOOK');
|
|||
my $course_item2 = GetCourseItem( ci_id => $ci_id2 );
|
||||
is($course_item2->{ccode}, '', 'Course item ccode should be empty');
|
||||
|
||||
ModCourseItem(
|
||||
itemnumber => $itemnumber,
|
||||
itype => 'CD_foo',
|
||||
ccode => 'DVD',
|
||||
holdingbranch => 'B1',
|
||||
location => 'HR',
|
||||
);
|
||||
|
||||
ModCourseReserve(
|
||||
course_id => $course->{course_id},
|
||||
ci_id => $ci_id2,
|
||||
staff_note => '',
|
||||
public_note => '',
|
||||
);
|
||||
|
||||
$item = Koha::Items->find($itemnumber);
|
||||
is($item->ccode, 'DVD', 'Item ccode should be BOOK');
|
||||
|
||||
$course_item2 = GetCourseItem( ci_id => $ci_id2 );
|
||||
is($course_item2->{ccode}, '', 'Course item ccode should be empty');
|
||||
|
||||
DelCourseReserve( cr_id => $cr_id2 );
|
||||
$item = Koha::Items->find($itemnumber);
|
||||
is($item->ccode, '', 'Item ccode should be set back to empty');
|
||||
|
|
@ -141,6 +192,14 @@ sub create_dependent_objects {
|
|||
}
|
||||
});
|
||||
|
||||
$builder->build({
|
||||
source => 'Branch',
|
||||
value => {
|
||||
branchcode => 'B3',
|
||||
branchname => 'Branch 3'
|
||||
}
|
||||
});
|
||||
|
||||
$builder->build({
|
||||
source => 'AuthorisedValue',
|
||||
value => {
|
||||
|
|
@ -150,6 +209,15 @@ sub create_dependent_objects {
|
|||
}
|
||||
});
|
||||
|
||||
$builder->build({
|
||||
source => 'AuthorisedValue',
|
||||
value => {
|
||||
category => 'CCODE',
|
||||
authorised_value => 'DVD',
|
||||
lib => 'Book'
|
||||
}
|
||||
});
|
||||
|
||||
$builder->build({
|
||||
source => 'AuthorisedValue',
|
||||
value => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue