Bug 41882: Enforce pickup location rules
To test: 1- Set ReservesControlBranch to 'item's home library' 2- Set Main library's 'Holds and bookings policies by item type' so that PASS items can only be picked up at Main 3- Place a hold on a bib whose only item is a PASS belonging to Main 4- Use the Batch hold modification tool to identify the hold you just placed 5- Update the pickup location to North. Note that the update is successful, and there's no indication that the pickup location is now a problem. 6- Check in Main's PASS, and note that the hold doesn't trigger to fill 7- APPLY PATCH, restart_all 8- Try again, this time the hold should fail to update at all. 9- You should see a new table with the 'Failed hold' in it, it should also include a 'Result' column which explains why the hold failed (New pickup location is invalid). 10- Try modifying a hold to an invalid pickup location, but also attempt to modify something else on the hold at the same time, like expiration date. 11- The hold should not modify anything. If the pickup location is invalid all other modifications should NOT happen on that hold. 12 - Play with the Table settings for the holds_modified tables, ensuring you can hide/show all the columns. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Lucas Gass
parent
63f8c65b36
commit
cc3b7cebf5
|
|
@ -2701,6 +2701,8 @@ modules:
|
|||
columnname: suspended_until
|
||||
-
|
||||
columnname: notes
|
||||
-
|
||||
columnname: result
|
||||
logviewer:
|
||||
logst:
|
||||
default_display_length: 20
|
||||
|
|
|
|||
|
|
@ -103,63 +103,133 @@
|
|||
</div>
|
||||
<!-- /#modify_holds_search -->
|
||||
[% ELSIF view == "report" %]
|
||||
<h1>Modified holds</h1>
|
||||
<div id="modified_holds_results-wrapper">
|
||||
<div id="modified_holds_results" class="page-section">
|
||||
<table id="modified_holds">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Title</th>
|
||||
<th>Patron</th>
|
||||
<th>Pickup location</th>
|
||||
<th>Priority</th>
|
||||
<th>Expiration date</th>
|
||||
<th>Status</th>
|
||||
<th>Suspended</th>
|
||||
<th>Suspended until</th>
|
||||
<th>Notes</th>
|
||||
<!-- Add other columns as needed -->
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
[% FOREACH hold IN updated_holds_obj %]
|
||||
<h1>Modified holds report</h1>
|
||||
[% IF updated_holds_obj %]
|
||||
<div id="modified_holds_results-wrapper">
|
||||
<div id="modified_holds_results" class="page-section">
|
||||
<h2>Updated holds ([% total_updated | html %])</h2>
|
||||
<table id="modified_holds">
|
||||
<thead>
|
||||
<tr>
|
||||
<td>[% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %]</td>
|
||||
<td>[% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %]</td>
|
||||
<td>[% hold.branch.branchname | html %]</td>
|
||||
<td>[% hold.priority | html %]</td>
|
||||
<td>[% hold.expirationdate | $KohaDates %]</td>
|
||||
<td>
|
||||
[% IF hold.found == 'F' %]
|
||||
<span>Fulfilled</span>
|
||||
[% ELSIF hold.cancellationdate %]
|
||||
<span>Cancelled</span>
|
||||
[% ELSIF hold.found == 'W' %]
|
||||
[% IF hold.cancellation_requests.count == 0 %]
|
||||
<span>Waiting</span>
|
||||
[% ELSE %]
|
||||
<span>Cancelled</span>
|
||||
[% END %]
|
||||
[% ELSIF hold.found == 'T' %]
|
||||
<span>In transit</span>
|
||||
[% ELSE %]
|
||||
<span>Pending</span>
|
||||
[% END %]
|
||||
</td>
|
||||
<td>[% IF hold.suspend %]Yes[% ELSE %]No[% END %]</td>
|
||||
<td>[% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %]</td>
|
||||
<td>[% hold.reservenotes | html %]</td>
|
||||
<th>Title</th>
|
||||
<th>Patron</th>
|
||||
<th>Pickup location</th>
|
||||
<th>Priority</th>
|
||||
<th>Expiration date</th>
|
||||
<th>Status</th>
|
||||
<th>Suspended</th>
|
||||
<th>Suspended until</th>
|
||||
<th>Notes</th>
|
||||
<th>Result</th>
|
||||
<!-- Add other columns as needed -->
|
||||
</tr>
|
||||
[% END %]
|
||||
</tbody>
|
||||
</table>
|
||||
</thead>
|
||||
<tbody>
|
||||
[% FOREACH hold IN updated_holds_obj %]
|
||||
<tr>
|
||||
<td>[% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %]</td>
|
||||
<td>[% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %]</td>
|
||||
<td>[% hold.branch.branchname | html %]</td>
|
||||
<td>[% hold.priority | html %]</td>
|
||||
<td>[% hold.expirationdate | $KohaDates %]</td>
|
||||
<td>
|
||||
[% IF hold.found == 'F' %]
|
||||
<span>Fulfilled</span>
|
||||
[% ELSIF hold.cancellationdate %]
|
||||
<span>Cancelled</span>
|
||||
[% ELSIF hold.found == 'W' %]
|
||||
[% IF hold.cancellation_requests.count == 0 %]
|
||||
<span>Waiting</span>
|
||||
[% ELSE %]
|
||||
<span>Cancelled</span>
|
||||
[% END %]
|
||||
[% ELSIF hold.found == 'T' %]
|
||||
<span>In transit</span>
|
||||
[% ELSE %]
|
||||
<span>Pending</span>
|
||||
[% END %]
|
||||
</td>
|
||||
<td>[% IF hold.suspend %]Yes[% ELSE %]No[% END %]</td>
|
||||
<td>[% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %]</td>
|
||||
<td>[% hold.reservenotes | html %]</td>
|
||||
<td><span class="badge rounded-pill bg-success">Success</span></td>
|
||||
</tr>
|
||||
[% END %]
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
<!-- /#modified_holds_results -->
|
||||
</div>
|
||||
<!-- /#modified_holds_results -->
|
||||
<fieldset class="action">
|
||||
<a href="/cgi-bin/koha/tools/batch_modify_holds.pl">Return to batch hold modification</a>
|
||||
</fieldset>
|
||||
</div>
|
||||
<!-- /#modified_holds_results-wrapper -->
|
||||
<!-- /#modified_holds_results-wrapper -->
|
||||
[% END %]
|
||||
[% IF failed_holds_obj %]
|
||||
<div id="failed_holds_results-wrapper">
|
||||
<div id="failed_holds_results" class="page-section">
|
||||
<h2>Failed holds ([% total_failed | html %])</h2>
|
||||
<div class="alert alert-warning">Important: The following holds were not modified.</div>
|
||||
<table id="failed_holds">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Title</th>
|
||||
<th>Patron</th>
|
||||
<th>Pickup location</th>
|
||||
<th>Priority</th>
|
||||
<th>Expiration date</th>
|
||||
<th>Status</th>
|
||||
<th>Suspended</th>
|
||||
<th>Suspended until</th>
|
||||
<th>Notes</th>
|
||||
<th>Result</th>
|
||||
<!-- Add other columns as needed -->
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
[% FOREACH failure IN failed_holds_obj %]
|
||||
[% SET hold = failure.hold %]
|
||||
<tr>
|
||||
<td>[% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %]</td>
|
||||
<td>[% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %]</td>
|
||||
<td>[% hold.branch.branchname | html %]</td>
|
||||
<td>[% hold.priority | html %]</td>
|
||||
<td>[% hold.expirationdate | $KohaDates %]</td>
|
||||
<td>
|
||||
[% IF hold.found == 'F' %]
|
||||
<span>Fulfilled</span>
|
||||
[% ELSIF hold.cancellationdate %]
|
||||
<span>Cancelled</span>
|
||||
[% ELSIF hold.found == 'W' %]
|
||||
[% IF hold.cancellation_requests.count == 0 %]
|
||||
<span>Waiting</span>
|
||||
[% ELSE %]
|
||||
<span>Cancelled</span>
|
||||
[% END %]
|
||||
[% ELSIF hold.found == 'T' %]
|
||||
<span>In transit</span>
|
||||
[% ELSE %]
|
||||
<span>Pending</span>
|
||||
[% END %]
|
||||
</td>
|
||||
<td>[% IF hold.suspend %]Yes[% ELSE %]No[% END %]</td>
|
||||
<td>[% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %]</td>
|
||||
<td>[% hold.reservenotes | html %]</td>
|
||||
<td>
|
||||
<span class="badge rounded-pill bg-warning text-dark">Failed</span>
|
||||
[% IF failure.reason == 'pickupLocationNotAllowed' %]
|
||||
New pickup location is invalid
|
||||
[% END %]
|
||||
</td>
|
||||
</tr>
|
||||
[% END %]
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
<!-- /#failed_holds_results -->
|
||||
</div>
|
||||
<!-- /#failed_holds_results-wrapper -->
|
||||
[% END %]
|
||||
<fieldset class="action">
|
||||
<a href="/cgi-bin/koha/tools/batch_modify_holds.pl">Return to batch hold modification</a>
|
||||
</fieldset>
|
||||
[% END %]
|
||||
<div id="modify_holds_results-wrapper" style="display:none;">
|
||||
<div id="toolbar" class="btn-toolbar">
|
||||
|
|
@ -634,14 +704,21 @@
|
|||
return true;
|
||||
});
|
||||
|
||||
//Modified holds table
|
||||
$("#modified_holds").kohaTable(
|
||||
{
|
||||
//Modified holds table and failed holds table share table settings
|
||||
var table_settings = [% TablesSettings.GetTableSettings( 'circ', 'holds', 'modified_holds', 'json' ) | $raw %];
|
||||
[% IF updated_holds_obj %]
|
||||
$("#modified_holds").kohaTable({
|
||||
pagingType: "full",
|
||||
autoWidth: false,
|
||||
},
|
||||
modified_holds_table_settings
|
||||
);
|
||||
autoWidth: false
|
||||
}, table_settings);
|
||||
[% END %]
|
||||
[% IF failed_holds_obj %]
|
||||
$("#failed_holds").kohaTable({
|
||||
pagingType: "full",
|
||||
autoWidth: false
|
||||
}, table_settings);
|
||||
[% END %]
|
||||
|
||||
});
|
||||
</script>
|
||||
[% END %]
|
||||
|
|
|
|||
|
|
@ -60,57 +60,84 @@ if ( $op eq 'cud-form' ) {
|
|||
my $clear_hold_notes = $input->param('clear_hold_notes');
|
||||
|
||||
@hold_ids = $input->multi_param('hold_id');
|
||||
my @holds_data = ();
|
||||
my @holds_data = ();
|
||||
my @failed_holds = ();
|
||||
|
||||
my $holds_to_update =
|
||||
Koha::Holds->search( { reserve_id => { -in => \@hold_ids } }, { join => [ "item", "biblio" ] } );
|
||||
|
||||
while ( my $hold = $holds_to_update->next ) {
|
||||
my $hold_before_mod = $hold->unblessed;
|
||||
|
||||
if ($new_expiration_date) {
|
||||
$hold->expirationdate($new_expiration_date)->store;
|
||||
}
|
||||
|
||||
my $hold_failed = 0;
|
||||
if ( $new_pickup_loc && ( $hold->branchcode ne $new_pickup_loc ) ) {
|
||||
my $patron = $hold->patron;
|
||||
my $item = $hold->item;
|
||||
my $locations;
|
||||
|
||||
if ($item) {
|
||||
|
||||
#item level pickup location allowed?
|
||||
$locations = $item->pickup_locations( { patron => $patron } );
|
||||
} else {
|
||||
|
||||
#record level pickup location allowed?
|
||||
$locations = $hold->biblio->pickup_locations( { patron => $patron } );
|
||||
}
|
||||
|
||||
unless ( $locations->search( { branchcode => $new_pickup_loc } )->count ) {
|
||||
push @failed_holds, { hold => $hold, reason => 'pickupLocationNotAllowed' };
|
||||
$hold_failed = 1;
|
||||
next;
|
||||
}
|
||||
|
||||
$hold->branchcode($new_pickup_loc)->store;
|
||||
}
|
||||
|
||||
if ( $new_suspend_status ne "" ) {
|
||||
if ( $new_suspend_status && !$hold->is_found ) {
|
||||
$hold->suspend(1)->store;
|
||||
if ($new_suspend_date) {
|
||||
#only continue to update the hold if it hasn't failed
|
||||
unless ($hold_failed) {
|
||||
if ( $new_suspend_status ne "" ) {
|
||||
if ( $new_suspend_status && !$hold->is_found ) {
|
||||
$hold->suspend(1)->store;
|
||||
if ($new_suspend_date) {
|
||||
$hold->suspend_until($new_suspend_date)->store;
|
||||
} else {
|
||||
$hold->suspend_until(undef)->store;
|
||||
}
|
||||
} elsif ( !$new_suspend_status && $new_suspend_date ) {
|
||||
$hold->suspend(1)->store;
|
||||
$hold->suspend_until($new_suspend_date)->store;
|
||||
} else {
|
||||
$hold->suspend(0)->store;
|
||||
$hold->suspend_until(undef)->store;
|
||||
}
|
||||
} elsif ( !$new_suspend_status && $new_suspend_date ) {
|
||||
$hold->suspend(1)->store;
|
||||
$hold->suspend_until($new_suspend_date)->store;
|
||||
} else {
|
||||
$hold->suspend(0)->store;
|
||||
$hold->suspend_until(undef)->store;
|
||||
}
|
||||
|
||||
if ($new_expiration_date) {
|
||||
$hold->expirationdate($new_expiration_date)->store;
|
||||
}
|
||||
|
||||
if ($new_hold_note) {
|
||||
$hold->reservenotes($new_hold_note)->store;
|
||||
}
|
||||
|
||||
if ($clear_hold_notes) {
|
||||
$hold->reservenotes(undef)->store;
|
||||
}
|
||||
|
||||
push @holds_data, $hold;
|
||||
|
||||
logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $hold_before_mod )
|
||||
if C4::Context->preference('HoldsLog');
|
||||
}
|
||||
|
||||
if ($new_hold_note) {
|
||||
$hold->reservenotes($new_hold_note)->store;
|
||||
}
|
||||
|
||||
if ($clear_hold_notes) {
|
||||
$hold->reservenotes(undef)->store;
|
||||
}
|
||||
|
||||
logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $hold_before_mod )
|
||||
if C4::Context->preference('HoldsLog');
|
||||
|
||||
push @holds_data, $hold;
|
||||
}
|
||||
|
||||
$template->param(
|
||||
updated_holds => to_json( \@hold_ids ),
|
||||
updated_holds_obj => \@holds_data,
|
||||
failed_holds_obj => \@failed_holds,
|
||||
total_updated => scalar @holds_data,
|
||||
total_failed => scalar @failed_holds,
|
||||
view => 'report',
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue