Bug 15530 - Editing a course item via a disabled course disables it even if it is...
authorKyle M Hall <kyle@bywatersolutions.com>
Fri, 8 Jan 2016 17:00:36 +0000 (17:00 +0000)
committerBrendan A Gallagher <brendan@bywatersolutions.com>
Wed, 27 Jan 2016 00:58:14 +0000 (00:58 +0000)
It appears that if the course item is edited by clicking the edit link
from an active course, the course item will be set to enabled and the
fields will be swapped, if the same course item is edited from a course
that is *not* active, the course item will be set to *not* enabled, and
the original fields will be swapped back in!

The short term work-around is to only edit course items from an enabled
course if the item has a course that is enabled. If all the courses it
is on are disabled, it doesn't matter what course the item is edited
from.

Test Plan:
1) Create two courses, 1 enabled and 1 disabled
2) Add an item as a course reserve to both courses
3) Edit the course reserve data for the item via the enabled course
4) Note the course item is enabled ( easy way is to check the database )
5) Edit the same course reserve data, but via the disabled course
6) Note the course item is now disabled even though it is part of
   an enabled course!
7) Apply this patch
8) Repeat steps 1 through 5
9) Note the course item is still enabled

Signed-off-by: Margaret Holt <mholt@bastyr.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

C4/CourseReserves.pm

index 7e160d0..1e415a7 100644 (file)
@@ -17,6 +17,8 @@ package C4::CourseReserves;
 
 use Modern::Perl;
 
+use List::MoreUtils qw(any);
+
 use C4::Context;
 use C4::Items qw(GetItem ModItem);
 use C4::Biblio qw(GetBiblioFromItemNumber);
@@ -259,7 +261,6 @@ sub EnableOrDisableCourseItems {
               ) {
                 EnableOrDisableCourseItem(
                     ci_id   => $course_reserve->{'ci_id'},
-                    enabled => 'yes',
                 );
             }
         }
@@ -274,7 +275,6 @@ sub EnableOrDisableCourseItems {
               ) {
                 EnableOrDisableCourseItem(
                     ci_id   => $course_reserve->{'ci_id'},
-                    enabled => 'no',
                 );
             }
         }
@@ -283,10 +283,7 @@ sub EnableOrDisableCourseItems {
 
 =head2 EnableOrDisableCourseItem
 
-    EnableOrDisableCourseItem( ci_id => $ci_id, enabled => $enabled );
-
-    enabled => 'yes' to enable course items
-    enabled => 'no' to disable course items
+    EnableOrDisableCourseItem( ci_id => $ci_id );
 
 =cut
 
@@ -295,13 +292,16 @@ sub EnableOrDisableCourseItem {
     warn identify_myself(%params) if $DEBUG;
 
     my $ci_id   = $params{'ci_id'};
-    my $enabled = $params{'enabled'};
 
-    return unless ( $ci_id && $enabled );
-    return unless ( $enabled eq 'yes' || $enabled eq 'no' );
+    return unless ( $ci_id );
 
     my $course_item = GetCourseItem( ci_id => $ci_id );
 
+    my $info = GetItemCourseReservesInfo( itemnumber => $course_item->{itemnumber} );
+
+    my $enabled = any { $_->{course}->{enabled} eq 'yes' } @$info;
+    $enabled = $enabled ? 'yes' : 'no';
+
     ## We don't want to 'enable' an already enabled item,
     ## or disable and already disabled item,
     ## as that would cause the fields to swap
@@ -828,10 +828,8 @@ sub ModCourseReserve {
         $cr_id = $dbh->last_insert_id( undef, undef, 'course_reserves', 'cr_id' );
     }
 
-    my $course = GetCourse($course_id);
     EnableOrDisableCourseItem(
         ci_id   => $params{'ci_id'},
-        enabled => $course->{'enabled'}
     );
 
     return $cr_id;
@@ -932,7 +930,7 @@ sub DelCourseReserve {
 
 }
 
-=head2 GetReservesInfo
+=head2 GetItemCourseReservesInfo
 
     my $arrayref = GetItemCourseReservesInfo( itemnumber => $itemnumber );