Bug 23727: Editing course reserve items is broken
authorKyle M Hall <kyle@bywatersolutions.com>
Fri, 14 Feb 2020 20:22:17 +0000 (15:22 -0500)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Fri, 17 Apr 2020 12:45:56 +0000 (13:45 +0100)
Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

Test Plan:
1) Apply this patch
2) Add and edit course items, not the new checkboxes for enabling fields
3) Everything should function as before

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

C4/CourseReserves.pm
Koha/Course/Instructors.pm
Koha/Course/Item.pm
Koha/Course/Items.pm
Koha/Course/Reserves.pm
course_reserves/add_items.pl
course_reserves/batch_add_items.pl
koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt
koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/batch_add_items.tt
koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
t/db_dependent/CourseReserves/CourseItems.t

index 18fa4a1..06101db 100644 (file)
@@ -22,6 +22,11 @@ use List::MoreUtils qw(any);
 use C4::Context;
 use C4::Circulation qw(GetOpenIssue);
 
+use Koha::Courses;
+use Koha::Course::Instructors;
+use Koha::Course::Items;
+use Koha::Course::Reserves;
+
 use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $DEBUG @FIELDS);
 
 BEGIN {
@@ -78,19 +83,17 @@ sub GetCourse {
     my ($course_id) = @_;
     warn whoami() . "( $course_id )" if $DEBUG;
 
-    my $query = "SELECT * FROM courses WHERE course_id = ?";
-    my $dbh   = C4::Context->dbh;
-    my $sth   = $dbh->prepare($query);
-    $sth->execute($course_id);
-
-    my $course = $sth->fetchrow_hashref();
+    my $course = Koha::Courses->find( $course_id );
+    return undef unless $course;
+    $course = $course->unblessed;
 
-    $query = "
+    my $dbh = C4::Context->dbh;
+    my $query = "
         SELECT b.* FROM course_instructors ci
         LEFT JOIN borrowers b ON ( ci.borrowernumber = b.borrowernumber )
         WHERE course_id =  ?
     ";
-    $sth = $dbh->prepare($query);
+    my $sth = $dbh->prepare($query);
     $sth->execute($course_id);
     $course->{'instructors'} = $sth->fetchall_arrayref( {} );
 
@@ -304,7 +307,7 @@ sub EnableOrDisableCourseItem {
     ## or disable and already disabled item,
     ## as that would cause the fields to swap
     if ( $course_item->{'enabled'} ne $enabled ) {
-        _SwapAllFields($ci_id);
+        _SwapAllFields($ci_id, $enabled );
 
         my $query = "
             UPDATE course_items
@@ -492,23 +495,20 @@ sub _AddCourseItem {
     my (%params) = @_;
     warn identify_myself(%params) if $DEBUG;
 
-    my ( @fields, @values );
+    $params{holdingbranch} ||= undef; # Can't be empty string, FK constraint
 
-    push( @fields, 'itemnumber = ?' );
-    push( @values, $params{'itemnumber'} );
+    my %data = map { $_ => $params{$_} } @FIELDS;
+    my %enabled = map { $_ . "_enabled" => $params{ $_ . "_enabled" } } @FIELDS;
 
-    foreach (@FIELDS) {
-        push( @fields, "$_ = ?" );
-        push( @values, $params{$_} || undef );
-    }
-
-    my $query = "INSERT INTO course_items SET " . join( ',', @fields );
-    my $dbh = C4::Context->dbh;
-    $dbh->do( $query, undef, @values );
-
-    my $ci_id = $dbh->last_insert_id( undef, undef, 'course_items', 'ci_id' );
+    my $ci = Koha::Course::Item->new(
+        {
+            itemnumber => $params{itemnumber},
+            %data,
+            %enabled,
+        }
+    )->store();
 
-    return $ci_id;
+    return $ci->id;
 }
 
 =head2 _UpdateCourseItem
@@ -524,59 +524,38 @@ sub _UpdateCourseItem {
     my $ci_id         = $params{'ci_id'};
     my $course_item   = $params{'course_item'};
 
-    return unless ( $ci_id || $course_item );
-
-    $course_item = GetCourseItem( ci_id => $ci_id )
-      unless ($course_item);
-    $ci_id = $course_item->{'ci_id'} unless ($ci_id);
-
-    my %mod_params =
-      map {
-        defined $params{$_} && $params{$_} ne ''
-          ? ( $_ => $params{$_} )
-          : ()
-      } @FIELDS;
-
-    Koha::Items->find( $course_item->{itemnumber} )
-               ->set( \%mod_params )
-               ->store;
-}
-
-=head2 _ModStoredFields
-
-    _ModStoredFields( %params );
+    $params{holdingbranch} ||= undef; # Can't be empty string, FK constraint
 
-    Updates the values for the 'original' fields in course_items
-    for a given ci_id
+    return unless ( $ci_id || $course_item );
 
-=cut
+    $course_item = Koha::Course::Items->find( $ci_id || $course_item->{ci_id} );
 
-sub _ModStoredFields {
-    my (%params) = @_;
-    warn identify_myself(%params) if $DEBUG;
+    my %data = map { $_ => $params{$_} } @FIELDS;
+    my %enabled = map { $_ . "_enabled" => $params{ $_ . "_enabled" } } @FIELDS;
 
-    return unless ( $params{'ci_id'} );
+    $course_item->update( { %data, %enabled } );
+    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->{holdingbranch} = $course_item->holdingbranch if $course_item->holdingbranch_enabled;
 
-    my ( @fields_to_update, @values_to_update );
+        Koha::Items->find( $course_item->itemnumber )
+                   ->set( $item_fields )
+                   ->store
+            if keys %$item_fields;
 
-    foreach (@FIELDS) {
-        if ( defined($params{$_}) ) {
-            push( @fields_to_update, $_ );
-            push( @values_to_update, $params{$_} );
-        }
     }
 
-    my $query = "UPDATE course_items SET " . join( ',', map { "$_=?" } @fields_to_update ) . " WHERE ci_id = ?";
-
-    C4::Context->dbh->do( $query, undef, @values_to_update, $params{'ci_id'} )
-      if (@values_to_update);
-
 }
 
 =head2 _RevertFields
 
     _RevertFields( ci_id => $ci_id, fields => \@fields_to_revert );
 
+    Copies fields from course item storage back to the actual item
+
 =cut
 
 sub _RevertFields {
@@ -585,15 +564,26 @@ sub _RevertFields {
 
     my $ci_id = $params{'ci_id'};
 
-    return unless ($ci_id);
+    return unless $ci_id;
 
-    my $course_item = GetCourseItem( ci_id => $params{'ci_id'} );
-    my $course_item_object = Koha::Items->find($course_item->{'itemnumber'});
-    foreach my $field ( @FIELDS ) {
-        next unless defined $course_item->{$field};
-        $course_item_object->$field($course_item->{$field});
-    }
-    $course_item_object->store;
+    my $course_item = Koha::Course::Items->find( $ci_id );
+
+    my $item_fields = {};
+    $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->{holdingbranch} = $course_item->holdingbranch_storage if $course_item->holdingbranch_enabled;
+
+    Koha::Items->find( $course_item->itemnumber )
+               ->set( $item_fields )
+               ->store
+        if keys %$item_fields;
+
+    $course_item->itype_storage(undef);
+    $course_item->ccode_storage(undef);
+    $course_item->location_storage(undef);
+    $course_item->holdingbranch_storage(undef);
+    $course_item->store();
 }
 
 =head2 _SwapAllFields
@@ -603,21 +593,48 @@ sub _RevertFields {
 =cut
 
 sub _SwapAllFields {
-    my ($ci_id) = @_;
+    my ( $ci_id, $enabled ) = @_;
     warn "C4::CourseReserves::_SwapFields( $ci_id )" if $DEBUG;
 
-    my $course_item = GetCourseItem( ci_id => $ci_id );
-    my $item = Koha::Items->find($course_item->{'itemnumber'});
-
-    my %item_fields;
-    foreach (@FIELDS) {
-        if ( defined( $course_item->{$_} ) ) {
-            $item_fields{$_}        = $item->$_ || q{};
-            $item->$_($course_item->{$_});
-        }
+    my $course_item = Koha::Course::Items->find( $ci_id );
+    my $item = Koha::Items->find( $course_item->itemnumber );
+
+    if ( $enabled eq 'yes' ) { # Copy item fields to course item storage, course item fields to item
+        $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->holdingbranch_storage( $item->holdingbranch ) if $course_item->holdingbranch_enabled;
+        $course_item->store();
+
+        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->{holdingbranch} = $course_item->holdingbranch if $course_item->holdingbranch_enabled;
+
+        Koha::Items->find( $course_item->itemnumber )
+                   ->set( $item_fields )
+                   ->store
+            if keys %$item_fields;
+
+    } else { # Copy course item storage to item
+        my $item_fields = {};
+        $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->{holdingbranch} = $course_item->holdingbranch_storage if $course_item->holdingbranch_enabled;
+
+        Koha::Items->find( $course_item->itemnumber )
+                   ->set( $item_fields )
+                   ->store
+            if keys %$item_fields;
+
+        $course_item->itype_storage(undef);
+        $course_item->ccode_storage(undef);
+        $course_item->location_storage(undef);
+        $course_item->holdingbranch_storage(undef);
+        $course_item->store();
     }
-    $item->store;
-    _ModStoredFields( %item_fields, ci_id => $ci_id );
 }
 
 =head2 GetCourseItems {
index 418ec4f..68211eb 100644 (file)
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use Carp;
 
-use Koha::Course;
+use Koha::Course::Instructor;
 
 use base qw(Koha::Objects);
 
index 7f9cd91..948a90c 100644 (file)
@@ -21,12 +21,48 @@ use Carp;
 
 use base qw(Koha::Object);
 
+use Koha::Course::Reserves;
+
 =head1 NAME
 
 Koha::Course::Item - Koha Course Item Object class
 
 =head1 API
 
+=head2 Methods
+
+=head3 is_enabled
+
+=cut
+
+sub is_enabled {
+    my ( $self ) = @_;
+
+    return $self->enabled eq 'yes';
+}
+
+=head3 course_reserves
+
+=cut
+
+sub course_reserves {
+    my ($self) = @_;
+
+    my $rs = $self->_result->course_reserves;
+    return Koha::Course::Reserves->_new_from_dbic( $rs );
+}
+
+=head3 item
+
+=cut
+
+sub item {
+    my ($self) = @_;
+
+    my $rs = $self->_result->itemnumber;
+    return Koha::Item->_new_from_dbic( $rs );
+}
+
 =head2 Internal methods
 
 =cut
index efd0608..7fc713d 100644 (file)
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use Carp;
 
-use Koha::Course;
+use Koha::Course::Item;
 
 use base qw(Koha::Objects);
 
index c23a7a9..631ba49 100644 (file)
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use Carp;
 
-use Koha::Course;
+use Koha::Course::Reserve;
 
 use base qw(Koha::Objects);
 
index 42416b3..edae03b 100755 (executable)
@@ -39,10 +39,12 @@ my $action       = $cgi->param('action')       || '';
 my $course_id    = $cgi->param('course_id')    || '';
 my $barcode      = $cgi->param('barcode')      || '';
 my $return       = $cgi->param('return')       || '';
-my $itemnumber   = ($cgi->param('itemnumber') && $action eq 'lookup') ? $cgi->param('itemnumber') : '';
+my $itemnumber   = $cgi->param('itemnumber')   || '';
 my $is_edit      = $cgi->param('is_edit')      || '';
 
 my $item = Koha::Items->find( { ( $itemnumber ? ( itemnumber => $itemnumber ) : ( barcode => $barcode ) ) } );
+$itemnumber = $item->id if $item;
+
 my $title = ($item) ? $item->biblio->title : undef;
 
 my $step = ( $action eq 'lookup' && $item ) ? '2' : '1';
@@ -65,12 +67,12 @@ if ( !$item && $action eq 'lookup' ){
 $template->param( course => GetCourse($course_id) );
 
 if ( $action eq 'lookup' and $item ) {
-    my $course_item = GetCourseItem( itemnumber => $item->itemnumber );
+    my $course_item = Koha::Course::Items->find({ itemnumber => $item->id });
     my $course_reserve =
       ($course_item)
       ? GetCourseReserve(
         course_id => $course_id,
-        ci_id     => $course_item->{'ci_id'}
+        ci_id     => $course_item->ci_id,
       )
       : undef;
 
@@ -89,12 +91,26 @@ if ( $action eq 'lookup' and $item ) {
     );
 
 } elsif ( $action eq 'add' ) {
+    my $itype         = scalar $cgi->param('itype');
+    my $ccode         = scalar $cgi->param('ccode');
+    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 $holdingbranch_enabled = scalar $cgi->param('holdingbranch_enabled') ? 1 : 0;
+    my $location_enabled      = scalar $cgi->param('location_enabled') ? 1 : 0;
+
     my $ci_id = ModCourseItem(
-        itemnumber    => scalar $cgi->param('itemnumber'),
-        itype         => scalar $cgi->param('itype'),
-        ccode         => scalar $cgi->param('ccode'),
-        holdingbranch => scalar $cgi->param('holdingbranch'),
-        location      => scalar $cgi->param('location'),
+            itemnumber    => $itemnumber,
+            itype         => $itype,
+            ccode         => $ccode,
+            holdingbranch => $holdingbranch,
+            location      => $location,
+            itype_enabled         => $itype_enabled,
+            ccode_enabled         => $ccode_enabled,
+            holdingbranch_enabled => $holdingbranch_enabled,
+            location_enabled      => $location_enabled,
     );
 
     my $cr_id = ModCourseReserve(
index 76d0caa..2a924b6 100755 (executable)
@@ -42,6 +42,11 @@ my $location      = $cgi->param('location');
 my $staff_note    = $cgi->param('staff_note');
 my $public_note   = $cgi->param('public_note');
 
+my $itype_enabled         = scalar $cgi->param('itype_enabled') ? 1 : 0;
+my $ccode_enabled         = scalar $cgi->param('ccode_enabled') ? 1 : 0;
+my $holdingbranch_enabled = scalar $cgi->param('holdingbranch_enabled') ? 1 : 0;
+my $location_enabled      = scalar $cgi->param('location_enabled') ? 1 : 0;
+
 my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     {
         template_name   => "course_reserves/batch_add_items.tt",
@@ -78,11 +83,15 @@ if ( $course_id && $course ) {
 
         foreach my $item (@items) {
             my $ci_id = ModCourseItem(
-                itemnumber    => $item->id,
-                itype         => $itype,
-                ccode         => $ccode,
-                holdingbranch => $holdingbranch,
-                location      => $location,
+                itemnumber            => $item->id,
+                itype                 => $itype,
+                ccode                 => $ccode,
+                holdingbranch         => $holdingbranch,
+                location              => $location,
+                itype_enabled         => $itype_enabled,
+                ccode_enabled         => $ccode_enabled,
+                holdingbranch_enabled => $holdingbranch_enabled,
+                location_enabled      => $location_enabled,
             );
 
             my $cr_id = ModCourseReserve(
index 061821d..0c6bcff 100644 (file)
@@ -1,4 +1,5 @@
 [% USE Branches %]
+[% SET footerjs = 1 %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Course reserves &rsaquo;[% IF is_edit || course_reserve %] Edit item[% ELSE %] Add items[% END %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -16,7 +17,7 @@
         <div class="col-md-8 col-md-offset-2">
 
         [% IF course_reserve && !is_edit%]<div class="dialog message" id="already_on_reserve_this">This course already has this item on reserve.</div>[% END %]
-        [% IF course_item %]<div class="dialog message" id="already_on_reserve">Number of courses reserving this item: [% course_item.course_reserves.size | html %]</div>[% END %]
+        [% IF course_item %]<div class="dialog message" id="already_on_reserve">Number of courses reserving this item: [% course_item.course_reserves.count | html %]</div>[% END %]
 
         <form method="post" action="/cgi-bin/koha/course_reserves/add_items.pl">
             <input type="hidden" name="course_id" value="[% course.course_id | html %]" />
                     [% IF item_level_itypes %]
                     <li>
                         <label class="required" for="itype">Item type:</label>
-                        <select id="itype" name="itype">
-                            <option value="">LEAVE UNCHANGED</option>
 
+                        [% IF course_item.itype_enabled %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="itype" value="1" name="itype_enabled" id="itype_enabled" checked="checked" />
+                        [% ELSE %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="itype" value="1" name="itype_enabled" id="itype_enabled" />
+                        [% END %]
+
+                        [% IF course_item.itype_enabled %]
+                            <select id="itype" name="itype">
+                        [% ELSE %]
+                            <select id="itype" name="itype" disabled="disabled">
+                        [% END %]
+
+                            <option value=""></option>
                             [% FOREACH it IN itypes %]
-                                [% IF course_item.itype.defined && ( ( course.enabled == 'yes' && it.itemtype == item.itype ) || ( course.enabled == 'no' && it.itemtype == course_item.itype ) ) %]
+                                [% IF it.itemtype == course_item.itype %]
                                     <option value="[% it.itemtype | html %]" selected="selected">[% it.description | html %]</option>
                                 [% ELSE %]
                                     <option value="[% it.itemtype | html %]">[% it.description | html %]</option>
 
                     <li>
                         <label class="required" for="ccode">Collection:</label>
-                        <select id="ccode" name="ccode">
-                            <option value="">LEAVE UNCHANGED</option>
 
+                        [% IF course_item.ccode_enabled %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="ccode" value="1" name="ccode_enabled" id="ccode_enabled" checked="checked" />
+                        [% ELSE %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="ccode" value="1" name="ccode_enabled" id="ccode_enabled" />
+                        [% END %]
+
+                        [% IF course_item.ccode_enabled %]
+                            <select id="ccode" name="ccode">
+                        [% ELSE %]
+                            <select id="ccode" name="ccode" disabled="disabled">
+                        [% END %]
+
+                            <option value=""></option>
                             [% FOREACH c IN ccodes %]
-                                [% IF course_item.ccode.defined && ( ( course.enabled == 'yes' && c.authorised_value == item.ccode ) || ( course.enabled == 'no' && c.authorised_value == course_item.ccode ) ) %]
+                                [% IF c.authorised_value == course_item.ccode %]
                                     <option value="[% c.authorised_value | html %]" selected="selected">[% c.lib | html %]</option>
                                 [% ELSE %]
                                     <option value="[% c.authorised_value | html %]">[% c.lib | html %]</option>
 
                     <li>
                         <label class="required" for="location">Shelving location:</label>
-                        <select id="location" name="location">
-                            <option value="">LEAVE UNCHANGED</option>
 
+                        [% IF course_item.location_enabled %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="location" value="1" name="location_enabled" id="location_enabled" checked="checked" />
+                        [% ELSE %]
+                            <input type="checkbox" class="field-toggle" data-pulldown="location"  value="1" name="location_enabled" id="location_enabled" />
+                        [% END %]
+
+                        [% IF course_item.location_enabled %]
+                            <select id="location" name="location">
+                        [% ELSE %]
+                            <select id="location" name="location" disabled="disabled">
+                        [% END %]
+
+                            <option value=""></option>
                             [% FOREACH s IN locations %]
-                                [% IF course_item.location.defined && ( ( course.enabled == 'yes' && s.authorised_value == item.location ) || ( course.enabled == 'no' && s.authorised_value == course_item.location ) ) %]
+                                [% IF s.authorised_value == course_item.location %]
                                     <option value="[% s.authorised_value | html %]" selected="selected">[% s.lib | html %]</option>
                                 [% ELSE %]
                                     <option value="[% s.authorised_value | html %]">[% s.lib | html %]</option>
 
                     <li>
                         <label class="required" for="holdingbranch">Holding library:</label>
-                        <select id="holdingbranch" name="holdingbranch">
-                            <option value="">LEAVE UNCHANGED</option>
+
+                        [% IF course_item.holdingbranch_enabled %]
+                            <input type="checkbox" value="1" class="field-toggle" data-pulldown="holdingbranch" name="holdingbranch_enabled" id="holdingbranch_enabled" checked="checked" />
+                        [% ELSE %]
+                            <input type="checkbox" value="1" class="field-toggle" data-pulldown="holdingbranch" name="holdingbranch_enabled" id="holdingbranch_enabled" />
+                        [% END %]
+
+                        [% IF course_item.holdingbranch_enabled %]
+                            <select id="holdingbranch" name="holdingbranch">
+                        [% ELSE %]
+                            <select id="holdingbranch" name="holdingbranch" disabled="disabled">
+                        [% END %]
+
+                            <option value=""></option>
                             [% FOREACH b IN Branches.all() %]
-                                [% IF course_item.holdingbranch.defined && ( ( course.enabled == 'yes' && b.branchcode == item.holdingbranch ) || ( course.enabled == 'no' && b.branchcode == course_item.holdingbranch ) ) %]
+                                [% IF b.branchcode == course_item.holdingbranch %]
                                     <option value="[% b.branchcode | html %]" selected="selected">[% b.branchname | html %]</option>
                                 [% ELSE %]
                                     <option value="[% b.branchcode | html %]">[% b.branchname | html %]</option>
     </div>
 </div>
 
+[% MACRO jsinclude BLOCK %]
+    <script type="text/javascript">
+    //<![CDATA[
+        $(document).ready(function(){
+            $('.field-toggle').change(function() {
+                if( this.checked ) {
+                    $('#' + $(this).data('pulldown') ).removeAttr('disabled');
+                } else {
+                    $('#' + $(this).data('pulldown') ).val('');
+                    $('#' + $(this).data('pulldown') ).attr('disabled', 'disabled');
+                }
+            });
+        });
+    //]]>
+    </script>
+[% END %]
+
 [% INCLUDE 'intranet-bottom.inc' %]
index 068c6ba..3fe2e95 100644 (file)
@@ -2,6 +2,8 @@
 [% USE Branches %]
 [% USE ItemTypes %]
 
+[% SET footerjs = 1 %]
+
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Course reserves &rsaquo; Add items</title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -35,8 +37,9 @@
                         [% IF item_level_itypes %]
                         <li>
                             <label class="required" for="itype">Item type:</label>
-                            <select id="itype" name="itype">
-                                <option value="">LEAVE UNCHANGED</option>
+                            <input type="checkbox" class="field-toggle" data-pulldown="itype" value="1" name="itype_enabled" id="itype_enabled" />
+                            <select id="itype" name="itype" disabled="disabled">
+                                <option value=""></option>
 
                                 [% FOREACH it IN ItemTypes.Get() %]
                                     <option value="[% it.itemtype | html %]">[% it.description | html %]</option>
@@ -47,8 +50,9 @@
 
                         <li>
                             <label class="required" for="ccode">Collection:</label>
-                            <select id="ccode" name="ccode">
-                                <option value="">LEAVE UNCHANGED</option>
+                            <input type="checkbox" class="field-toggle" data-pulldown="ccode" value="1" name="ccode_enabled" id="ccode_enabled" />
+                            <select id="ccode" name="ccode" disabled="disabled">
+                                <option value=""></option>
                                 [% FOREACH c IN AuthorisedValues.Get('CCODE') %]
                                     <option value="[% c.authorised_value | html %]">[% c.lib | html %]</option>
                                 [% END %]
@@ -57,8 +61,9 @@
 
                         <li>
                             <label class="required" for="location">Shelving location:</label>
-                            <select id="location" name="location">
-                                <option value="">LEAVE UNCHANGED</option>
+                            <input type="checkbox" class="field-toggle" data-pulldown="location"  value="1" name="location_enabled" id="location_enabled" />
+                            <select id="location" name="location" disabled="disabled">
+                                <option value=""></option>
                                 [% FOREACH s IN AuthorisedValues.Get('LOC') %]
                                     <option value="[% s.authorised_value | html %]">[% s.lib | html %]</option>
                                 [% END %]
@@ -67,8 +72,9 @@
 
                         <li>
                             <label class="required" for="holdingbranch">Holding library:</label>
-                            <select id="holdingbranch" name="holdingbranch">
-                                <option value="">LEAVE UNCHANGED</option>
+                            <input type="checkbox" value="1" class="field-toggle" data-pulldown="holdingbranch" name="holdingbranch_enabled" id="holdingbranch_enabled" />
+                            <select id="holdingbranch" name="holdingbranch" disabled="disabled">
+                                <option value=""></option>
                                 [% FOREACH b IN Branches.all() %]
                                     <option value="[% b.branchcode | html %]">[% b.branchname | html %]</option>
                                 [% END %]
     </div>
 </div>
 
+[% MACRO jsinclude BLOCK %]
+    <script type="text/javascript">
+    //<![CDATA[
+        $(document).ready(function(){
+            $('.field-toggle').change(function() {
+                if( this.checked ) {
+                    $('#' + $(this).data('pulldown') ).removeAttr('disabled');
+                } else {
+                    $('#' + $(this).data('pulldown') ).val('');
+                    $('#' + $(this).data('pulldown') ).attr('disabled', 'disabled');
+                }
+            });
+        });
+    //]]>
+    </script>
+[% END %]
+
 [% INCLUDE 'intranet-bottom.inc' %]
index bc816e1..a1e04a9 100644 (file)
                             <td>[% cr.item.itemcallnumber | html %]</td>
                             [% IF item_level_itypes %]
                             <td>
-                                [% IF cr.course_item.itype.defined %]
+                                [% IF cr.course_item.itype_enabled %]
                                     [% IF cr.course_item.enabled == 'yes' %]
-                                        <strong>[% ItemTypes.GetDescription( cr.item.itype) | html %]</strong>
-                                        [% IF cr.item.itype %]
-                                            ([% ItemTypes.GetDescription( cr.course_item.itype ) | html %])
-                                        [% END %]
+                                        <strong>[% ItemTypes.GetDescription( cr.item.effective_itemtype ) | html %]</strong>
+                                        ([% ItemTypes.GetDescription( cr.course_item.itype_storage ) | html %])
                                     [% ELSE %]
                                         [% ItemTypes.GetDescription( cr.course_item.itype ) | html %]
-                                        [% IF cr.item.itype %]
-                                            (<strong>[% ItemTypes.GetDescription( cr.item.itype) | html %]</strong>)
-                                        [% END %]
+                                        (<strong>[% ItemTypes.GetDescription( cr.item.effective_itemtype) | html %]</strong>)
                                     [% END %]
                                 [% ELSE %]
                                      <i>Unchanged</i>
                             </td>
                             [% END %]
                             <td>
-                                 [% IF cr.course_item.ccode.defined %]
+                                 [% IF cr.course_item.ccode_enabled %]
                                      [% IF cr.course_item.enabled == 'yes' %]
                                         <strong>[% AuthorisedValues.GetByCode( 'CCODE', cr.item.ccode ) | html %]</strong>
                                         [% IF cr.item.ccode %]
-                                            ([% AuthorisedValues.GetByCode( 'CCODE', cr.course_item.ccode ) | html %])
+                                            ([% AuthorisedValues.GetByCode( 'CCODE', cr.course_item.ccode_storage ) | html %])
                                         [% END %]
                                      [% ELSE %]
                                         [% AuthorisedValues.GetByCode( 'CCODE', cr.course_item.ccode ) | html %]
                                  [% END %]
                             </td>
                             <td>
-                                [% IF cr.course_item.location.defined %]
+                                [% IF cr.course_item.location_enabled %]
                                     [% IF cr.course_item.enabled == 'yes' %]
                                         <strong>[% AuthorisedValues.GetByCode( 'LOC', cr.item.permanent_location ) | html %]</strong>
                                         [% IF cr.item.permanent_location %]
-                                            ([% AuthorisedValues.GetByCode( 'LOC', cr.course_item.location ) | html %])
+                                            ([% AuthorisedValues.GetByCode( 'LOC', cr.course_item.location_storage ) | html %])
                                         [% END %]
                                     [% ELSE %]
                                         [% AuthorisedValues.GetByCode( 'LOC', cr.course_item.location ) | html %]
                                 [% END %]
                             </td>
                             <td>
-                                [% IF cr.course_item.holdingbranch.defined %]
+                                [% IF cr.course_item.holdingbranch_enabled %]
                                     [% IF cr.course_item.enabled == 'yes' %]
                                         <strong>[% Branches.GetName( cr.item.holdingbranch ) | html %]</strong>
                                         [% IF cr.item.holdingbranch %]
-                                            ([% Branches.GetName( cr.course_item.holdingbranch ) | html %])
+                                            ([% Branches.GetName( cr.course_item.holdingbranch_storage ) | html %])
                                         [% END %]
                                     [% ELSE %]
                                         [% Branches.GetName( cr.course_item.holdingbranch ) | html %]
index c5ce965..fe375b4 100644 (file)
@@ -38,17 +38,22 @@ create_dependent_objects();
 my ($biblionumber, $itemnumber) = create_bib_and_item();
 
 my $ci_id = ModCourseItem(
-    itemnumber    => $itemnumber,
-    itype         => 'BK_foo',
-    ccode         => 'BOOK',
-    holdingbranch => 'B2',
-    location      => 'TH',
+    itemnumber            => $itemnumber,
+    itype_enabled         => 1,
+    ccode_enabled         => 1,
+    holdingbranch_enabled => 1,
+    location_enabled      => 1,
+    itype                 => 'BK_foo',
+    ccode                 => 'BOOK',
+    holdingbranch         => 'B2',
+    location              => 'TH',
 );
 
 my $course = $builder->build({
     source => 'CourseReserve',
     value => {
         ci_id => $ci_id,
+        enabled => 'no',
     }
 });
 
@@ -60,23 +65,27 @@ my $cr_id = ModCourseReserve(
 );
 
 my $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');
+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->{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->itype, 'BK_foo', 'Item type in course should be BK_foo');
+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->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',
+    itemnumber            => $itemnumber,
+    itype_enabled         => 1,
+    ccode_enabled         => 1,
+    holdingbranch_enabled => 1,
+    location_enabled      => 1,
+    itype                 => 'BK_foo',
+    ccode                 => 'DVD',
+    holdingbranch         => 'B3',
+    location              => 'TH',
 );
 
 ModCourseReserve(
@@ -87,20 +96,20 @@ ModCourseReserve(
 );
 
 $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');
+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->{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->itype, 'BK_foo', 'Item type in course should be BK_foo');
+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->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');
+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->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');
@@ -112,11 +121,15 @@ $item = Koha::Items->find($itemnumber);
 is($item->ccode, '', 'Item ccode should be empty');
 
 my $ci_id2 = ModCourseItem(
-    itemnumber    => $itemnumber,
-    itype         => 'CD_foo',
-    ccode         => 'BOOK',
-    holdingbranch => 'B1',
-    location      => 'HR',
+    itemnumber            => $itemnumber,
+    itype_enabled         => 1,
+    ccode_enabled         => 1,
+    holdingbranch_enabled => 1,
+    location_enabled      => 1,
+    itype                 => 'CD_foo',
+    ccode                 => 'BOOK',
+    holdingbranch         => 'B1',
+    location              => 'HR',
 );
 
 my $cr_id2 = ModCourseReserve(
@@ -130,14 +143,18 @@ $item = Koha::Items->find($itemnumber);
 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');
+is($course_item2->{ccode_storage}, '', 'Course item ccode storage should be empty');
 
 ModCourseItem(
-    itemnumber    => $itemnumber,
-    itype         => 'CD_foo',
-    ccode         => 'DVD',
-    holdingbranch => 'B1',
-    location      => 'HR',
+    itemnumber            => $itemnumber,
+    itype_enabled         => 1,
+    ccode_enabled         => 1,
+    holdingbranch_enabled => 1,
+    location_enabled      => 1,
+    itype                 => 'CD_foo',
+    ccode                 => 'DVD',
+    holdingbranch         => 'B1',
+    location              => 'HR',
 );
 
 ModCourseReserve(
@@ -151,17 +168,21 @@ $item = Koha::Items->find($itemnumber);
 is($item->ccode, 'DVD', 'Item ccode should be DVD');
 
 ModCourseItem(
-    itemnumber    => $itemnumber,
-    itype         => 'BK',
-    ccode         => 'BOOK',
-    holdingbranch => '', # LEAVE UNCHANGED
-    location      => 'TH',
+    itemnumber            => $itemnumber,
+    itype_enabled         => 1,
+    ccode_enabled         => 1,
+    holdingbranch_enabled => 0,             # LEAVE UNCHANGED
+    location_enabled      => 1,
+    itype                 => 'BK',
+    ccode                 => 'BOOK',
+    holdingbranch         => undef,         # LEAVE UNCHANGED
+    location              => 'TH',
 );
 $item = Koha::Items->find($itemnumber);
 is($item->ccode, 'BOOK', 'Item ccode should be BOOK');
 
 $course_item2 = GetCourseItem( ci_id => $ci_id2 );
-is($course_item2->{ccode}, '', 'Course item ccode should be empty');
+is($course_item2->{ccode_storage}, '', 'Course item ccode storage should be empty');
 
 DelCourseReserve( cr_id => $cr_id2 );
 $item = Koha::Items->find($itemnumber);
@@ -195,7 +216,7 @@ subtest 'Ensure item info is preserved' => sub {
     #Remove course reservei
     DelCourseReserve( cr_id => $course_reserve_id );
     my $item_after = Koha::Items->find( $item->itemnumber );
-    is( $item->itype, $item_after->itype, "Itemtype is unchanged after adding to and removing from course reserves for inactive course");
+    is( $item->effective_itemtype, $item_after->itype, "Itemtype is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->location, $item_after->location, "Location is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->holdingbranch, $item_after->holdingbranch, "Holdingbranch is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->ccode, $item_after->ccode, "Collection is unchanged after adding to and removing from course reserves for inactive course");
@@ -225,7 +246,7 @@ subtest 'Ensure item info is preserved' => sub {
     #Remove course reserve
     DelCourseReserve( cr_id => $course_reserve_id );
     $item_after = Koha::Items->find( $item->itemnumber );
-    is( $item->itype, $item_after->itype, "Itemtype is unchanged after adding to and removing from course reserves for inactive course");
+    is( $item->effective_itemtype, $item_after->itype, "Itemtype is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->location, $item_after->location, "Location is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->holdingbranch, $item_after->holdingbranch, "Holdingbranch is unchanged after adding to and removing from course reserves for inactive course");
     is( $item->ccode, $item_after->ccode, "Collection is unchanged after adding to and removing from course reserves for inactive course");