Bugzilla – Attachment 72022 Details for
Bug 10382
collection and location not returning to null when removed from course reserves
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10382 - Course reserves: handle empty values
Bug-10382---Course-reserves-handle-empty-values.patch (text/plain), 9.55 KB, created by
Alex Arnaud
on 2018-02-21 13:11:58 UTC
(
hide
)
Description:
Bug 10382 - Course reserves: handle empty values
Filename:
MIME Type:
Creator:
Alex Arnaud
Created:
2018-02-21 13:11:58 UTC
Size:
9.55 KB
patch
obsolete
>From 6635301590686f663b45dc273998c40fb3bba8a2 Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >Date: Wed, 21 Feb 2018 13:10:50 +0000 >Subject: [PATCH] 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 >--- > C4/CourseReserves.pm | 83 +++++------------------------ > t/db_dependent/CourseReserves/CourseItems.t | 78 +++++++++++++++++++++++++-- > 2 files changed, 85 insertions(+), 76 deletions(-) > >diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm >index 19ed685..f405196 100644 >--- a/C4/CourseReserves.pm >+++ b/C4/CourseReserves.pm >@@ -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{$_} || '' ); > } > > 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{$_}; >- } >- } >- 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; >+ my %mod_params; > foreach (@FIELDS) { >- if ( $params{$_} && !$course_item->{$_} ) { >- $mod_params_new{$_} = $params{$_}; >- $mod_params_old{$_} = $item->{$_}; >- } >+ $mod_params{$_} = $params{$_}; > } >- _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); >+ ModItem( $mod_item_params, undef, $course_item->{'itemnumber'} ) if $mod_item_params && %$mod_item_params; > } > > =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 >diff --git a/t/db_dependent/CourseReserves/CourseItems.t b/t/db_dependent/CourseReserves/CourseItems.t >index c2bb237..45b4833 100644 >--- a/t/db_dependent/CourseReserves/CourseItems.t >+++ b/t/db_dependent/CourseReserves/CourseItems.t >@@ -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_objets(); > 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'); >@@ -142,6 +193,14 @@ sub create_dependent_objets { > }); > > $builder->build({ >+ source => 'Branch', >+ value => { >+ branchcode => 'B3', >+ branchname => 'Branch 3' >+ } >+ }); >+ >+ $builder->build({ > source => 'AuthorisedValue', > value => { > category => 'CCODE', >@@ -154,6 +213,15 @@ sub create_dependent_objets { > source => 'AuthorisedValue', > value => { > category => 'CCODE', >+ authorised_value => 'DVD', >+ lib => 'Book' >+ } >+ }); >+ >+ $builder->build({ >+ source => 'AuthorisedValue', >+ value => { >+ category => 'CCODE', > authorised_value => 'CD', > lib => 'Compact Disk' > } >-- >2.7.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10382
:
18674
|
19611
|
60947
|
60948
|
60979
|
61814
|
61815
|
61816
|
63531
|
72022
|
79030
|
79031
|
79032
|
79114
|
79115
|
79116
|
79207
|
79208
|
79209
|
79722
|
79723
|
79724
|
80007
|
80008
|
80009