From 84230706499b4673d9061adc2d2ac025fa31a342 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 26 May 2020 09:08:15 -0400 Subject: [PATCH] Bug 25444: Backup/restore course items fields correctly This patch makes the code set the *_storage fields when adding new fields to an existing course item. And reverts those fields correctly when removing the item from the course. If a new field is enabled on an existing course reserve, the storage field is not given a value, so when the item goes off reserve, the item field will always be updated to NULL. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/CourseReserves/CourseItems.t => FAIL: Tests fail, data is not reverted correctly 3. Apply this patch and repeat 2 => SUCCESS: Tests pass! Data is correctly reverted 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart --- C4/CourseReserves.pm | 76 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 10 deletions(-) diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm index 595f0b71e2..ed12690fe8 100644 --- a/C4/CourseReserves.pm +++ b/C4/CourseReserves.pm @@ -535,22 +535,78 @@ sub _UpdateCourseItem { my %data = map { $_ => $params{$_} } @FIELDS; my %enabled = map { $_ . "_enabled" => $params{ $_ . "_enabled" } } @FIELDS; - $course_item->update( { %data, %enabled } ); + my $item = Koha::Items->find( $course_item->itemnumber ); + + # Handle updates to changed fields for a course item, both adding and removing if ( $course_item->is_enabled ) { my $item_fields = {}; - $item_fields->{itype} = $course_item->itype if $course_item->itype_enabled; - $item_fields->{ccode} = $course_item->ccode if $course_item->ccode_enabled; - $item_fields->{location} = $course_item->location if $course_item->location_enabled; - $item_fields->{homebranch} = $course_item->homebranch if $course_item->homebranch_enabled; - $item_fields->{holdingbranch} = $course_item->holdingbranch if $course_item->holdingbranch_enabled; - Koha::Items->find( $course_item->itemnumber ) - ->set( $item_fields ) - ->store - if keys %$item_fields; + # Find newly enabled field and add item value to storage + if ( $params{itype_enabled} && !$course_item->itype_enabled ) { + $enabled{itype_storage} = $item->itype; + $item_fields->{itype} = $params{itype}; + } + # Find newly disabled field and copy the storage value to the item, unset storage value + elsif ( !$params{itype_enabled} && $course_item->itype_enabled ) { + $item_fields->{itype} = $course_item->itype_storage; + $enabled{itype_storage} = undef; + } + # The field was already enabled, copy the incoming value to the item. + # The "original" ( when not on course reserve ) value is already in the storage field + elsif ( $course_item->itype_enabled) { + $item_fields->{itype} = $params{itype}; + } + if ( $params{ccode_enabled} && !$course_item->ccode_enabled ) { + $enabled{ccode_storage} = $item->ccode; + $item_fields->{ccode} = $params{ccode}; + } + elsif ( !$params{ccode_enabled} && $course_item->ccode_enabled ) { + $item_fields->{ccode} = $course_item->ccode_storage; + $enabled{ccode_storage} = undef; + } elsif ( $course_item->ccode_enabled) { + $item_fields->{ccode} = $params{ccode}; + } + + if ( $params{location_enabled} && !$course_item->location_enabled ) { + $enabled{location_storage} = $item->location; + $item_fields->{location} = $params{location}; + } + elsif ( !$params{location_enabled} && $course_item->location_enabled ) { + $item_fields->{location} = $course_item->location_storage; + $enabled{location_storage} = undef; + } elsif ( $course_item->location_enabled) { + $item_fields->{location} = $params{location}; + } + + if ( $params{homebranch_enabled} && !$course_item->homebranch_enabled ) { + $enabled{homebranch_storage} = $item->homebranch; + $item_fields->{homebranch} = $params{homebranch}; + } + elsif ( !$params{homebranch_enabled} && $course_item->homebranch_enabled ) { + $item_fields->{homebranch} = $course_item->homebranch_storage; + $enabled{homebranch_storage} = undef; + } elsif ( $course_item->homebranch_enabled) { + $item_fields->{homebranch} = $params{homebranch}; + } + + if ( $params{holdingbranch_enabled} && !$course_item->holdingbranch_enabled ) { + $enabled{holdingbranch_storage} = $item->holdingbranch; + $item_fields->{holdingbranch} = $params{holdingbranch}; + } + elsif ( !$params{holdingbranch_enabled} && $course_item->holdingbranch_enabled ) { + $item_fields->{holdingbranch} = $course_item->holdingbranch_storage; + $enabled{holdingbranch_storage} = undef; + } elsif ( $course_item->holdingbranch_enabled) { + $item_fields->{holdingbranch} = $params{holdingbranch}; + } + + $item->set( $item_fields )->store + if keys %$item_fields; } + $course_item->update( { %data, %enabled } ); + } =head2 _RevertFields -- 2.39.5