Bugzilla – Attachment 99272 Details for
Bug 23727
Editing course reserve items is broken
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23727: Editing course reserve items is broken
Bug-23727-Editing-course-reserve-items-is-broken.patch (text/plain), 44.12 KB, created by
Kyle M Hall (khall)
on 2020-02-19 20:00:26 UTC
(
hide
)
Description:
Bug 23727: Editing course reserve items is broken
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2020-02-19 20:00:26 UTC
Size:
44.12 KB
patch
obsolete
>From 9e7a4c870abb762351bc1d6812117c7688acbc64 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 14 Feb 2020 15:22:17 -0500 >Subject: [PATCH] Bug 23727: Editing course reserve items is broken > >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 >--- > C4/CourseReserves.pm | 167 +++++++++--------- > Koha/Course/Instructors.pm | 2 +- > Koha/Course/Item.pm | 36 ++++ > Koha/Course/Items.pm | 2 +- > Koha/Course/Reserves.pm | 2 +- > course_reserves/add_items.pl | 32 +++- > course_reserves/batch_add_items.pl | 19 +- > .../course_reserves/add_items-step2.tt | 89 ++++++++-- > .../course_reserves/batch_add_items.tt | 39 +++- > .../modules/course_reserves/course-details.tt | 24 ++- > t/db_dependent/CourseReserves/CourseItems.t | 102 ++++++----- > 11 files changed, 340 insertions(+), 174 deletions(-) > >diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm >index 2c130af192..7bbda18cd6 100644 >--- a/C4/CourseReserves.pm >+++ b/C4/CourseReserves.pm >@@ -23,6 +23,11 @@ use C4::Context; > use C4::Items qw(ModItem); > 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 { >@@ -79,19 +84,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( {} ); > >@@ -305,7 +308,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 >@@ -493,23 +496,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 >@@ -525,57 +525,34 @@ 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); >+ $params{holdingbranch} ||= undef; # Can't be empty string, FK constraint > >- my %mod_params = >- map { >- defined $params{$_} && $params{$_} ne '' >- ? ( $_ => $params{$_} ) >- : () >- } @FIELDS; >- >- ModItem( \%mod_params, undef, $course_item->{'itemnumber'} ); >-} >- >-=head2 _ModStoredFields >- >- _ModStoredFields( %params ); >- >- Updates the values for the 'original' fields in course_items >- for a given ci_id >- >-=cut >+ return unless ( $ci_id || $course_item ); > >-sub _ModStoredFields { >- my (%params) = @_; >- warn identify_myself(%params) if $DEBUG; >+ $course_item = Koha::Course::Items->find( $ci_id || $course_item->{ci_id} ); > >- return unless ( $params{'ci_id'} ); >+ my %data = map { $_ => $params{$_} } @FIELDS; >+ my %enabled = map { $_ . "_enabled" => $params{ $_ . "_enabled" } } @FIELDS; > >- my ( @fields_to_update, @values_to_update ); >+ $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; > >- foreach (@FIELDS) { >- if ( defined($params{$_}) ) { >- push( @fields_to_update, $_ ); >- push( @values_to_update, $params{$_} ); >- } >+ ModItem( $item_fields, undef, $course_item->itemnumber ) if keys %$item_fields; > } > >- 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 { >@@ -584,17 +561,23 @@ 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 = Koha::Course::Items->find( $ci_id ); > >- my $mod_item_params; >- foreach my $field ( @FIELDS ) { >- next unless defined $course_item->{$field}; >- $mod_item_params->{$field} = $course_item->{$field}; >- } >+ 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; > >- ModItem( $mod_item_params, undef, $course_item->{'itemnumber'} ) if $mod_item_params && %$mod_item_params; >+ ModItem( $item_fields, undef, $course_item->itemnumber ) 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 >@@ -604,23 +587,41 @@ 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 %course_item_fields; >- my %item_fields; >- foreach (@FIELDS) { >- if ( defined( $course_item->{$_} ) ) { >- $course_item_fields{$_} = $course_item->{$_}; >- $item_fields{$_} = $item->$_ || q{}; >- } >+ 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; >+ >+ ModItem( $item_fields, undef, $course_item->itemnumber ) 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; >+ >+ ModItem( $item_fields, undef, $course_item->itemnumber ) 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(); > } >- >- ModItem( \%course_item_fields, undef, $course_item->{'itemnumber'} ) if %course_item_fields; >- _ModStoredFields( %item_fields, ci_id => $ci_id ); > } > > =head2 GetCourseItems { >diff --git a/Koha/Course/Instructors.pm b/Koha/Course/Instructors.pm >index 418ec4f717..68211eb793 100644 >--- a/Koha/Course/Instructors.pm >+++ b/Koha/Course/Instructors.pm >@@ -19,7 +19,7 @@ use Modern::Perl; > > use Carp; > >-use Koha::Course; >+use Koha::Course::Instructor; > > use base qw(Koha::Objects); > >diff --git a/Koha/Course/Item.pm b/Koha/Course/Item.pm >index 7f9cd917d6..948a90c84b 100644 >--- a/Koha/Course/Item.pm >+++ b/Koha/Course/Item.pm >@@ -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 >diff --git a/Koha/Course/Items.pm b/Koha/Course/Items.pm >index efd06084b9..7fc713dee1 100644 >--- a/Koha/Course/Items.pm >+++ b/Koha/Course/Items.pm >@@ -19,7 +19,7 @@ use Modern::Perl; > > use Carp; > >-use Koha::Course; >+use Koha::Course::Item; > > use base qw(Koha::Objects); > >diff --git a/Koha/Course/Reserves.pm b/Koha/Course/Reserves.pm >index c23a7a95ee..631ba49a93 100644 >--- a/Koha/Course/Reserves.pm >+++ b/Koha/Course/Reserves.pm >@@ -19,7 +19,7 @@ use Modern::Perl; > > use Carp; > >-use Koha::Course; >+use Koha::Course::Reserve; > > use base qw(Koha::Objects); > >diff --git a/course_reserves/add_items.pl b/course_reserves/add_items.pl >index 42416b3a63..edae03b891 100755 >--- a/course_reserves/add_items.pl >+++ b/course_reserves/add_items.pl >@@ -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( >diff --git a/course_reserves/batch_add_items.pl b/course_reserves/batch_add_items.pl >index 76d0caa0b5..2a924b6f35 100755 >--- a/course_reserves/batch_add_items.pl >+++ b/course_reserves/batch_add_items.pl >@@ -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( >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 919c7d57c6..2e3fb93c86 100644 >--- 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 >@@ -1,4 +1,5 @@ > [% USE Branches %] >+[% SET footerjs = 1 %] > [% INCLUDE 'doc-head-open.inc' %] > <title>Koha › Course reserves ›[% 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 %]" /> >@@ -39,11 +40,22 @@ > [% 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> >@@ -55,11 +67,22 @@ > > <li> > <label class="required" for="ccode">Collection code:</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> >@@ -70,11 +93,22 @@ > > <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> >@@ -85,10 +119,22 @@ > > <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> >@@ -119,4 +165,21 @@ > </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' %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/batch_add_items.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/batch_add_items.tt >index ce67aab3ff..c4adebce66 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/batch_add_items.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/batch_add_items.tt >@@ -2,6 +2,8 @@ > [% USE Branches %] > [% USE ItemTypes %] > >+[% SET footerjs = 1 %] >+ > [% INCLUDE 'doc-head-open.inc' %] > <title>Koha › Course reserves › 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 code:</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 %] >@@ -132,4 +138,21 @@ > </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' %] >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 ab08e01654..7542892803 100644 >--- 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 >@@ -101,17 +101,13 @@ > <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> >@@ -122,11 +118,11 @@ > </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 %] >@@ -142,11 +138,11 @@ > [% 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 %] >@@ -162,11 +158,11 @@ > [% 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 %] >diff --git a/t/db_dependent/CourseReserves/CourseItems.t b/t/db_dependent/CourseReserves/CourseItems.t >index c5ce9652ff..4e4d10f649 100644 >--- a/t/db_dependent/CourseReserves/CourseItems.t >+++ b/t/db_dependent/CourseReserves/CourseItems.t >@@ -1,4 +1,5 @@ > #!/usr/bin/perl >+use Carp::Always; > > # This file is part of Koha. > # >@@ -38,17 +39,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 +66,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 +97,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 +122,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 +144,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 +169,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 +217,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 +247,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"); >-- >2.21.1 (Apple Git-122.3)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=99272">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23727
:
95104
|
98943
|
98944
|
98945
|
99269
|
99270
|
99271
|
99272
|
99273
|
99274
|
99275
|
99276
|
99277
|
99327
|
99328
|
99329
|
99330
|
99331
|
99332
|
99333
|
99334
|
99335
|
99336
|
103126
|
103127
|
103128
|
103131
|
103132
|
103133
|
103134
|
103135
|
103136
|
104782
|
104783
|
104784
|
104785
|
104786
|
104787