Bug 27064: Only allow transferring a hold from the transfers page

These patches replace the 'Waiting' button on the transfers page with a 'Transfer' button
and correct some other related problem by passing the hold object to the template and
using that to fetch patron info as well as passing the reserve_id through to ensure the correct hold is
affected at all times

To test:
 1 - Place a hold for delivery at Library B
 2 - Sign in at Library A
 3 - Browse to Circulation->Transfers
 4 - Attempt to transfer an item on the title with the hold to Library B
 5 - You get a notice that hold was found (missing patron/branch info)
 6 - You have the option to set the hold waiting - click it
 7 - The transfer is generated and marked completed
 8 - The hold is marked as waiting, but the item is still at Library A and no transfer is active
 9 - The patron is notified that the hold is waiting
10 - Revert the hold or cancel and place a new one
11 - Apply patches
12 - Attempt transfer again
13 - You now have the option to transfer the hold
14 - Click that
15 - Hold is in transit and transfer is generated correctly
16 - Transfer again and choose 'cancel'
17 - Confirm hold is cancelled and transfer generated

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Nick Clemens 2020-11-20 11:34:11 +00:00 committed by Kyle M Hall
parent 9d70d1eaec
commit 12b1925cd5
2 changed files with 34 additions and 26 deletions

View file

@ -77,7 +77,7 @@ my $hold_transferred;
my $hold_processed; my $hold_processed;
my $reqmessage; my $reqmessage;
my $cancelled; my $cancelled;
my $setwaiting; my $settransit;
my $request = $query->param('request') || ''; my $request = $query->param('request') || '';
my $borrowernumber = $query->param('borrowernumber') || 0; my $borrowernumber = $query->param('borrowernumber') || 0;
@ -98,21 +98,20 @@ if ( $request eq "KillWaiting" ) {
$reqmessage = 1; $reqmessage = 1;
} # FIXME else? } # FIXME else?
} }
elsif ( $request eq "SetWaiting" ) { elsif ( $request eq "SetTransit" ) {
my $item = $query->param('itemnumber'); my $item = $query->param('itemnumber');
ModReserveAffect( $item, $borrowernumber ); my $reserve_id = $query->param('reserve_id');
ModReserveAffect( $item, $borrowernumber, 1, $reserve_id );
$ignoreRs = 1; $ignoreRs = 1;
$setwaiting = 1; $settransit = 1;
$reqmessage = 1; $reqmessage = 1;
} }
elsif ( $request eq 'KillReserved' ) { elsif ( $request eq 'KillReserved' ) {
my $biblionumber = $query->param('biblionumber'); my $biblionumber = $query->param('biblionumber');
my $holds = Koha::Holds->search({ my $reserve_id = $query->param('reserve_id');
biblionumber => $biblionumber, my $hold = Koha::Holds->find({ reserve_id => $reserve_id });
borrowernumber => $borrowernumber if ( $hold ) {
}); $hold->cancel;
if ( $holds->count ) {
$holds->next->cancel;
$cancelled = 1; $cancelled = 1;
$reqmessage = 1; $reqmessage = 1;
} # FIXME else? } # FIXME else?
@ -170,20 +169,24 @@ my $biblionumber;
##################### #####################
if ($found) { my $hold;
my $res = $messages->{'ResFound'}; if ($found){
$itemnumber = $res->{'itemnumber'}; $hold = Koha::Holds->find(
$borrowernumber = $res->{'borrowernumber'}; { reserve_id => $found->{reserve_id} },
{ prefetch => ['item','patron'] }
);
$itemnumber = $found->{'itemnumber'};
$borrowernumber = $found->{'borrowernumber'};
if ( $res->{'ResFound'} eq "Waiting" ) { if ( $found->{'ResFound'} eq "Waiting" ) {
$waiting = 1; $waiting = 1;
} elsif ( $res->{'ResFound'} eq "Transferred" ) { } elsif ( $found->{'ResFound'} eq "Transferred" ) {
$hold_transferred = 1; $hold_transferred = 1;
} elsif ( $res->{'ResFound'} eq "Processing" ) { } elsif ( $found->{'ResFound'} eq "Processing" ) {
$hold_processed = 1; $hold_processed = 1;
} elsif ( $res->{'ResFound'} eq "Reserved" ) { } elsif ( $found->{'ResFound'} eq "Reserved" ) {
$reserved = 1; $reserved = 1;
$biblionumber = $res->{'biblionumber'}; $biblionumber = $found->{'biblionumber'};
} }
} }
@ -221,6 +224,7 @@ foreach my $code ( keys %$messages ) {
$template->param( $template->param(
found => $found, found => $found,
hold => $hold,
reserved => $reserved, reserved => $reserved,
waiting => $waiting, waiting => $waiting,
transferred => $hold_transferred, transferred => $hold_transferred,
@ -232,7 +236,7 @@ $template->param(
tobranchcd => $tobranchcd, tobranchcd => $tobranchcd,
reqmessage => $reqmessage, reqmessage => $reqmessage,
cancelled => $cancelled, cancelled => $cancelled,
setwaiting => $setwaiting, settransit => $settransit,
trsfitemloop => \@trsfitemloop, trsfitemloop => \@trsfitemloop,
errmsgloop => \@errmsgloop, errmsgloop => \@errmsgloop,
PatronAutoComplete => C4::Context->preference("PatronAutoComplete"), PatronAutoComplete => C4::Context->preference("PatronAutoComplete"),

View file

@ -40,7 +40,7 @@
<table> <table>
<caption> <caption>
[% IF ( reserved ) %] [% IF ( reserved ) %]
Reserve found for [% name | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>). Reserve found for [% INCLUDE 'patron-title.inc' patron => hold.patron | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>).
[% END %] [% END %]
[% IF ( waiting ) %] [% IF ( waiting ) %]
Item is marked waiting at [% branchname | html %] for [% name | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>). Item is marked waiting at [% branchname | html %] for [% name | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>).
@ -51,7 +51,9 @@
</caption> </caption>
<tr> <tr>
<th> <th>
[% IF ( reserved ) %]Set reserve to waiting and transfer book to [% branchname | html %]: [% END %] [% IF ( reserved ) %]
Set reserve to transit and transfer book to [% Branches.GetName( hold.branchcode ) | html %]:
[% END %]
[% IF ( waiting or transferred ) %]Cancel reservation and then attempt transfer: [% END %] [% IF ( waiting or transferred ) %]Cancel reservation and then attempt transfer: [% END %]
</th> </th>
<td> <td>
@ -61,6 +63,7 @@
<input type="hidden" name="fb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.frombrcd | html %]" /> <input type="hidden" name="fb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.frombrcd | html %]" />
<input type="hidden" name="tb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.tobrcd | html %]" /> <input type="hidden" name="tb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.tobrcd | html %]" />
[% END %] [% END %]
<input type="hidden" name="reserve_id" value="[% hold.reserve_id | html %]" />
<input type="hidden" name="itemnumber" value="[% itemnumber | html %]" /> <input type="hidden" name="itemnumber" value="[% itemnumber | html %]" />
<input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" /> <input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" />
<input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" /> <input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" />
@ -70,8 +73,8 @@
<input type="submit" value="Cancel" /> <input type="submit" value="Cancel" />
[% END %] [% END %]
[% IF ( reserved ) %] [% IF ( reserved ) %]
<input type="hidden" name="request" value="SetWaiting" /> <input type="hidden" name="request" value="SetTransit" />
<input type="submit" value="Waiting" /> <input type="submit" value="Transfer" />
[% END %] [% END %]
</form> </form>
</td> </td>
@ -86,6 +89,7 @@
<input type="hidden" name="fb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.frombrcd | html %]" /> <input type="hidden" name="fb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.frombrcd | html %]" />
<input type="hidden" name="tb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.tobrcd | html %]" /> <input type="hidden" name="tb-[% trsfitemloo.counter | html %]" value="[% trsfitemloo.tobrcd | html %]" />
[% END %] [% END %]
<input type="hidden" name="reserve_id" value="[% hold.reserve_id | html %]" />
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" /> <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
<input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" /> <input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" />
<input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" /> <input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" />
@ -120,8 +124,8 @@
[% IF ( cancelled ) %] [% IF ( cancelled ) %]
<li>Reserve cancelled</li> <li>Reserve cancelled</li>
[% END %] [% END %]
[% IF ( setwaiting ) %] [% IF ( settransit ) %]
<li>Item should now be waiting at library: [% reqbrchname | html %]</li> <li>Item is now in transit to [% Branches.GetName(tobranchcd) | html %]</li>
[% END %] [% END %]
</ul> </ul>
</div> </div>