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>
This commit is contained in:
parent
3ecc5d59fe
commit
605d6c9b6e
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
}
|
||||
|
|
@ -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`
|
||||
|
|
|
|||
|
|
@ -117,6 +117,32 @@
|
|||
</select>
|
||||
</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>
|
||||
|
||||
|
|
|
|||
|
|
@ -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>
|
||||
|
|
@ -158,6 +159,26 @@
|
|||
[% END %]
|
||||
[% 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' %]
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
Loading…
Reference in New Issue