Bug 22630: Allow to change homebranch in course reserves

Test plan:
1. Create a course (disabled)
2. Add a reserve to this course for an item and set a homebranch
   different from the item's homebranch
3. Enable the course
4. Verify that the item's homebranch has changed
5. Disable the course
6. Verify that the item's homebranch was reset to its initial value
7. prove t/db_dependent/CourseReserves/CourseItems.t

Sponsored-by: Université de Lyon 3
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm
index 06101db..3799fb8 100644 (file)
--- a/C4/CourseReserves.pm
+++ b/C4/CourseReserves.pm
@@ -56,7 +56,7 @@ BEGIN {
     %EXPORT_TAGS = ( 'all' => \@EXPORT_OK );
 
     $DEBUG = 0;
-    @FIELDS = ( 'itype', 'ccode', 'holdingbranch', 'location' );
+    @FIELDS = ( 'itype', 'ccode', 'homebranch', 'holdingbranch', 'location' );
 }
 
 =head1 NAME
@@ -495,6 +495,7 @@ sub _AddCourseItem {
     my (%params) = @_;
     warn identify_myself(%params) if $DEBUG;
 
+    $params{homebranch} ||= undef; # Can't be empty string, FK constraint
     $params{holdingbranch} ||= undef; # Can't be empty string, FK constraint
 
     my %data = map { $_ => $params{$_} } @FIELDS;
@@ -524,6 +525,7 @@ sub _UpdateCourseItem {
     my $ci_id         = $params{'ci_id'};
     my $course_item   = $params{'course_item'};
 
+    $params{homebranch} ||= undef; # Can't be empty string, FK constraint
     $params{holdingbranch} ||= undef; # Can't be empty string, FK constraint
 
     return unless ( $ci_id || $course_item );
@@ -539,6 +541,7 @@ sub _UpdateCourseItem {
         $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 )
@@ -572,6 +575,7 @@ sub _RevertFields {
     $item_fields->{itype}         = $course_item->itype_storage         if $course_item->itype_enabled;
     $item_fields->{ccode}         = $course_item->ccode_storage         if $course_item->ccode_enabled;
     $item_fields->{location}      = $course_item->location_storage      if $course_item->location_enabled;
+    $item_fields->{homebranch} = $course_item->homebranch_storage if $course_item->homebranch_enabled;
     $item_fields->{holdingbranch} = $course_item->holdingbranch_storage if $course_item->holdingbranch_enabled;
 
     Koha::Items->find( $course_item->itemnumber )
@@ -582,6 +586,7 @@ sub _RevertFields {
     $course_item->itype_storage(undef);
     $course_item->ccode_storage(undef);
     $course_item->location_storage(undef);
+    $course_item->homebranch_storage(undef);
     $course_item->holdingbranch_storage(undef);
     $course_item->store();
 }
@@ -603,6 +608,7 @@ sub _SwapAllFields {
         $course_item->itype_storage( $item->effective_itemtype )    if $course_item->itype_enabled;
         $course_item->ccode_storage( $item->ccode )                 if $course_item->ccode_enabled;
         $course_item->location_storage( $item->location )           if $course_item->location_enabled;
+        $course_item->homebranch_storage( $item->homebranch )       if $course_item->homebranch_enabled;
         $course_item->holdingbranch_storage( $item->holdingbranch ) if $course_item->holdingbranch_enabled;
         $course_item->store();
 
@@ -610,6 +616,7 @@ sub _SwapAllFields {
         $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 )
@@ -622,6 +629,7 @@ sub _SwapAllFields {
         $item_fields->{itype}         = $course_item->itype_storage         if $course_item->itype_enabled;
         $item_fields->{ccode}         = $course_item->ccode_storage         if $course_item->ccode_enabled;
         $item_fields->{location}      = $course_item->location_storage      if $course_item->location_enabled;
+        $item_fields->{homebranch}    = $course_item->homebranch_storage    if $course_item->homebranch_enabled;
         $item_fields->{holdingbranch} = $course_item->holdingbranch_storage if $course_item->holdingbranch_enabled;
 
         Koha::Items->find( $course_item->itemnumber )
@@ -632,6 +640,7 @@ sub _SwapAllFields {
         $course_item->itype_storage(undef);
         $course_item->ccode_storage(undef);
         $course_item->location_storage(undef);
+        $course_item->homebranch_storage(undef);
         $course_item->holdingbranch_storage(undef);
         $course_item->store();
     }

diff --git a/course_reserves/add_items.pl b/course_reserves/add_items.pl
index edae03b..3e98570 100755 (executable)
--- a/course_reserves/add_items.pl
+++ b/course_reserves/add_items.pl
@@ -93,11 +93,13 @@ if ( $action eq 'lookup' and $item ) {
 } elsif ( $action eq 'add' ) {
     my $itype         = scalar $cgi->param('itype');
     my $ccode         = scalar $cgi->param('ccode');
+    my $homebranch    = $cgi->param('homebranch');
     my $holdingbranch = scalar $cgi->param('holdingbranch');
     my $location      = scalar $cgi->param('location');
 
     my $itype_enabled         = scalar $cgi->param('itype_enabled') ? 1 : 0;
     my $ccode_enabled         = scalar $cgi->param('ccode_enabled') ? 1 : 0;
+    my $homebranch_enabled    = $cgi->param('homebranch_enabled') ? 1 : 0;
     my $holdingbranch_enabled = scalar $cgi->param('holdingbranch_enabled') ? 1 : 0;
     my $location_enabled      = scalar $cgi->param('location_enabled') ? 1 : 0;
 
@@ -105,10 +107,12 @@ if ( $action eq 'lookup' and $item ) {
             itemnumber    => $itemnumber,
             itype         => $itype,
             ccode         => $ccode,
+            homebranch    => $homebranch,
             holdingbranch => $holdingbranch,
             location      => $location,
             itype_enabled         => $itype_enabled,
             ccode_enabled         => $ccode_enabled,
+            homebranch_enabled    => $homebranch_enabled,
             holdingbranch_enabled => $holdingbranch_enabled,
             location_enabled      => $location_enabled,
     );

diff --git a/installer/data/mysql/atomicupdate/bug-22630.perl b/installer/data/mysql/atomicupdate/bug-22630.perl
new file mode 100644 (file)
index 0000000..1b7d483
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug-22630.perl
@@ -0,0 +1,44 @@
+$DBversion = 'XXX';
+if (CheckVersion($DBversion)) {
+    if (!column_exists('course_items', 'homebranch')) {
+        $dbh->do(q{
+            ALTER TABLE course_items
+            ADD COLUMN homebranch VARCHAR(10) NULL DEFAULT NULL AFTER ccode_storage
+        });
+    }
+
+    if (!foreign_key_exists('course_items', 'fk_course_items_homebranch')) {
+        $dbh->do(q{
+            ALTER TABLE course_items
+            ADD CONSTRAINT fk_course_items_homebranch
+              FOREIGN KEY (homebranch) REFERENCES branches (branchcode)
+              ON DELETE CASCADE ON UPDATE CASCADE
+        });
+    }
+
+    if (!column_exists('course_items', 'homebranch_enabled')) {
+        $dbh->do(q{
+            ALTER TABLE course_items
+            ADD COLUMN homebranch_enabled tinyint(1) NOT NULL DEFAULT 0 AFTER homebranch
+        });
+    }
+
+    if (!column_exists('course_items', 'homebranch_storage')) {
+        $dbh->do(q{
+            ALTER TABLE course_items
+            ADD COLUMN homebranch_storage VARCHAR(10) NULL DEFAULT NULL AFTER homebranch_enabled
+        });
+    }
+
+    if (!foreign_key_exists('course_items', 'fk_course_items_homebranch_storage')) {
+        $dbh->do(q{
+            ALTER TABLE course_items
+            ADD CONSTRAINT fk_course_items_homebranch_storage
+              FOREIGN KEY (homebranch_storage) REFERENCES branches (branchcode)
+              ON DELETE CASCADE ON UPDATE CASCADE
+        });
+    }
+
+    SetVersion($DBversion);
+    print "Upgrade to $DBversion done (Bug 22630 - Add course_items.homebranch)\n";
+}

diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index d8d5788..3f8d490 100644 (file)
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -3823,6 +3823,9 @@ CREATE TABLE `course_items` (
   `ccode` varchar(80) DEFAULT NULL, -- new category code for the item to have while on reserve (optional)
   `ccode_enabled` tinyint(1) NOT NULL DEFAULT 0, -- indicates if ccode should be changed while on course reserve
   `ccode_storage` varchar(80) DEFAULT NULL, -- a place to store the ccode when item is on course reserve
+  `homebranch` varchar(10) DEFAULT NULL, -- new home branch for the item to have while on reserve (optional)
+  `homebranch_enabled` tinyint(1) NOT NULL DEFAULT 0, -- indicates if homebranch should be changed while on course reserve
+  `homebranch_storage` varchar(10) DEFAULT NULL, -- a place to store the homebranch when item is on course reserve
   `holdingbranch` varchar(10) DEFAULT NULL, -- new holding branch for the item to have while on reserve (optional)
   `holdingbranch_enabled` tinyint(1) NOT NULL DEFAULT 0, -- indicates if itype should be changed while on course reserve
   `holdingbranch_storage` varchar(10) DEFAULT NULL, -- a place to store the holdingbranch when item is on course reserve
@@ -3841,7 +3844,9 @@ CREATE TABLE `course_items` (
 --
 ALTER TABLE `course_items`
   ADD CONSTRAINT `course_items_ibfk_2` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE,
-  ADD CONSTRAINT `course_items_ibfk_1` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE;
+  ADD CONSTRAINT `course_items_ibfk_1` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE,
+  ADD CONSTRAINT fk_course_items_homebranch FOREIGN KEY (homebranch) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE,
+  ADD CONSTRAINT fk_course_items_homebranch_storage FOREIGN KEY (homebranch_storage) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE;
 
 --
 -- Table structure for table `course_reserves`

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 78dd023..3b3a89d 100644 (file)
--- 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
@@ -118,6 +118,32 @@
                     </li>
 
                     <li>
+                        <label class="required" for="homebranch">Home library:</label>
+
+                        [% IF course_item.homebranch_enabled %]
+                            <input type="checkbox" value="1" class="field-toggle" data-pulldown="homebranch" name="homebranch_enabled" id="homebranch_enabled" checked />
+                        [% ELSE %]
+                            <input type="checkbox" value="1" class="field-toggle" data-pulldown="homebranch" name="homebranch_enabled" id="homebranch_enabled" />
+                        [% END %]
+
+                        [% IF course_item.homebranch_enabled %]
+                            <select id="homebranch" name="homebranch">
+                        [% ELSE %]
+                            <select id="homebranch" name="homebranch" disabled="disabled">
+                        [% END %]
+
+                            <option value=""></option>
+                            [% FOREACH b IN Branches.all() %]
+                                [% IF course_item.homebranch.defined && ( ( course.enabled == 'yes' && b.branchcode == item.homebranch ) || ( course.enabled == 'no' && b.branchcode == course_item.homebranch ) ) %]
+                                    <option value="[% b.branchcode | html %]" selected="selected">[% b.branchname | html %]</option>
+                                [% ELSE %]
+                                    <option value="[% b.branchcode | html %]">[% b.branchname | html %]</option>
+                                [% END %]
+                            [% END %]
+                        </select>
+                    </li>
+
+                    <li>
                         <label class="required" for="holdingbranch">Holding library:</label>
 
                         [% IF course_item.holdingbranch_enabled %]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
index a1e04a9..931604b 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
@@ -81,7 +81,8 @@
                         [% IF item_level_itypes %]<th>Item type</th>[% END %]
                         <th>Collection</th>
                         <th>Location</th>
-                        <th>Library</th>
+                        <th>Home Library</th>
+                        <th>Holding Library</th>
                         <th>Staff note</th>
                         <th>Public note</th>
                         <th>Link</th>
@@ -159,6 +160,26 @@
                                 [% END %]
                             </td>
                             <td>
+                                [% IF cr.course_item.homebranch_enabled %]
+                                    [% IF cr.course_item.enabled == 'yes' %]
+                                        <strong>[% Branches.GetName( cr.item.homebranch ) | html %]</strong>
+                                        [% IF cr.item.homebranch %]
+                                            ([% Branches.GetName( cr.course_item.homebranch ) | html %])
+                                        [% END %]
+                                    [% ELSE %]
+                                        [% Branches.GetName( cr.course_item.homebranch ) | html %]
+                                        [% IF cr.item.homebranch %]
+                                            (<strong>[% Branches.GetName( cr.item.homebranch ) | html %]</strong>)
+                                        [% END %]
+                                    [% END %]
+                                [% ELSE %]
+                                    <i>Unchanged</i>
+                                    [% IF cr.item.homebranch %]
+                                        ([% Branches.GetName( cr.item.homebranch ) | html %])
+                                    [% END %]
+                                [% END %]
+                            </td>
+                            <td>
                                 [% IF cr.course_item.holdingbranch_enabled %]
                                     [% IF cr.course_item.enabled == 'yes' %]
                                         <strong>[% Branches.GetName( cr.item.holdingbranch ) | html %]</strong>

diff --git a/t/db_dependent/CourseReserves/CourseItems.t b/t/db_dependent/CourseReserves/CourseItems.t
index fe375b4..aa6328d 100644 (file)
--- 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 => 29;
+use Test::More tests => 34;
 
 BEGIN {
     require_ok('C4::CourseReserves');
@@ -41,10 +41,12 @@ my $ci_id = ModCourseItem(
     itemnumber            => $itemnumber,
     itype_enabled         => 1,
     ccode_enabled         => 1,
+    homebranch_enabled    => 1,
     holdingbranch_enabled => 1,
     location_enabled      => 1,
     itype                 => 'BK_foo',
     ccode                 => 'BOOK',
+    homebranch            => 'B2',
     holdingbranch         => 'B2',
     location              => 'TH',
 );
@@ -67,12 +69,14 @@ my $cr_id = ModCourseReserve(
 my $course_item = GetCourseItem( ci_id => $ci_id );
 is($course_item->{itype_storage}, 'CD_foo', 'Course item itype storage should be CD_foo');
 is($course_item->{ccode_storage}, 'CD', 'Course item ccode storage should be CD');
+is($course_item->{homebranch_storage}, 'B1', 'Course item holding branch storage should be B1');
 is($course_item->{holdingbranch_storage}, 'B1', 'Course item holding branch storage should be B1');
 is($course_item->{location_storage}, 'HR', 'Course item location storage should be HR');
 
 my $item = Koha::Items->find($itemnumber);
 is($item->effective_itemtype, 'BK_foo', 'Item type in course should be BK_foo');
 is($item->ccode, 'BOOK', 'Item ccode in course should be BOOK');
+is($item->homebranch, 'B2', 'Item home branch in course should be B2');
 is($item->holdingbranch, 'B2', 'Item holding branch in course should be B2');
 is($item->location, 'TH', 'Item location in course should be TH');
 
@@ -80,10 +84,12 @@ ModCourseItem(
     itemnumber            => $itemnumber,
     itype_enabled         => 1,
     ccode_enabled         => 1,
+    homebranch_enabled    => 1,
     holdingbranch_enabled => 1,
     location_enabled      => 1,
     itype                 => 'BK_foo',
     ccode                 => 'DVD',
+    homebranch            => 'B3',
     holdingbranch         => 'B3',
     location              => 'TH',
 );
@@ -98,12 +104,14 @@ ModCourseReserve(
 $course_item = GetCourseItem( ci_id => $ci_id );
 is($course_item->{itype_storage}, 'CD_foo', 'Course item itype storage should be CD_foo');
 is($course_item->{ccode_storage}, 'CD', 'Course item ccode storage should be CD');
+is($course_item->{homebranch_storage}, 'B1', 'Course item home branch storage should be B1');
 is($course_item->{holdingbranch_storage}, 'B1', 'Course item holding branch storage should be B1');
 is($course_item->{location_storage}, 'HR', 'Course item location storage should be HR');
 
 $item = Koha::Items->find($itemnumber);
 is($item->effective_itemtype, 'BK_foo', 'Item type in course should be BK_foo');
 is($item->ccode, 'DVD', 'Item ccode in course should be DVD');
+is($item->homebranch, 'B3', 'Item home branch in course should be B3');
 is($item->holdingbranch, 'B3', 'Item holding branch in course should be B3');
 is($item->location, 'TH', 'Item location in course should be TH');
 
@@ -111,6 +119,7 @@ DelCourseReserve( cr_id => $cr_id );
 $item = Koha::Items->find($itemnumber);
 is($item->effective_itemtype, 'CD_foo', 'Item type removed from course should be set back to CD_foo');
 is($item->ccode, 'CD', 'Item ccode removed from course should be set back to CD');
+is($item->homebranch, 'B1', 'Item home branch removed from course should be set back B1');
 is($item->holdingbranch, 'B1', 'Item holding branch removed from course should be set back B1');
 is($item->location, 'HR', 'Item location removed from course should be TH');
 
@@ -124,10 +133,12 @@ my $ci_id2 = ModCourseItem(
     itemnumber            => $itemnumber,
     itype_enabled         => 1,
     ccode_enabled         => 1,
+    homebranch_enabled    => 1,
     holdingbranch_enabled => 1,
     location_enabled      => 1,
     itype                 => 'CD_foo',
     ccode                 => 'BOOK',
+    homebranch            => 'B1',
     holdingbranch         => 'B1',
     location              => 'HR',
 );
@@ -149,10 +160,12 @@ ModCourseItem(
     itemnumber            => $itemnumber,
     itype_enabled         => 1,
     ccode_enabled         => 1,
+    homebranch_enabled    => 1,
     holdingbranch_enabled => 1,
     location_enabled      => 1,
     itype                 => 'CD_foo',
     ccode                 => 'DVD',
+    homebranch            => 'B1',
     holdingbranch         => 'B1',
     location              => 'HR',
 );
@@ -171,6 +184,7 @@ ModCourseItem(
     itemnumber            => $itemnumber,
     itype_enabled         => 1,
     ccode_enabled         => 1,
+    homebranch_enabled    => 0,             # LEAVE UNCHANGED
     holdingbranch_enabled => 0,             # LEAVE UNCHANGED
     location_enabled      => 1,
     itype                 => 'BK',