Attachment 2330 Details for Bug 4946 – Proposed fix

[patch] Proposed fix

0001-Fix-for-Bug-4946-hold-warning-needs-rewording.patch (text/plain), 9.33 KB, created by Owen Leonard on 2010-07-08 14:48:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2010-07-08 14:48:08 UTC

Size: 9.33 KB

patch

obsolete

>From 47e3e9e08a2fe9ea86982bf279996de924b1f670 Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Wed, 7 Jul 2010 16:44:41 -0400
>Subject: [PATCH] Fix for Bug 4946 - hold warning needs rewording
>
>This patch improves the phrasing of several messages by
>breaking the message variable into distinct parts for more
>natural-sounding warnings when:
>
>- Checking out an item on hold for another patron
>- Checking out an item which is waiting for another patron
>- Checking out an item which is checked out to another patron
>- Checking out an item which is checked out to this patron
>- Checking out to a patron who has too many checked out
>
>I would appreciate special attention to my changes to the
>TooMany function in Circulation.pm to make sure I handled
>it correctly.
>---
> C4/Circulation.pm | 42 ++++++++++++++------
> C4/Reserves.pm | 1 +
> circ/circulation.pl | 1 +
> .../prog/en/modules/circ/circulation.tmpl | 10 ++--
> 4 files changed, 37 insertions(+), 17 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 034a8d6..aee335c 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -31,6 +31,7 @@ use C4::Dates;
> use C4::Calendar;
> use C4::Accounts;
> use C4::ItemCirculationAlertPreference;
>+use C4::Dates qw(format_date);
> use C4::Message;
> use C4::Debug;
> use Date::Calc qw(
>@@ -412,7 +413,7 @@ sub TooMany {
> 
> my $max_loans_allowed = $issuing_rule->{'maxissueqty'};
> if ($current_loan_count >= $max_loans_allowed) {
>- return "$current_loan_count / $max_loans_allowed";
>+ return ($current_loan_count, $max_loans_allowed);
> }
> }
> 
>@@ -440,7 +441,7 @@ sub TooMany {
> 
> my $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
> if ($current_loan_count >= $max_loans_allowed) {
>- return "$current_loan_count / $max_loans_allowed";
>+ return ($current_loan_count, $max_loans_allowed);
> }
> }
> 
>@@ -747,12 +748,16 @@ sub CanBookBeIssued {
> #
> # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
> #
>-	my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );
>- # if TooMany return / 0, then the user has no permission to check out this book
>- if ($toomany =~ /\/ 0/) {
>+	my ($current_loan_count, $max_loans_allowed) = TooMany( $borrower, $item->{biblionumber}, $item );
>+ # if TooMany max_loans_allowed returns 0 the user doesn't have permission to check out this book
>+ if ($max_loans_allowed eq 0) {
> $needsconfirmation{PATRON_CANT} = 1;
> } else {
>- $needsconfirmation{TOO_MANY} = $toomany if $toomany;
>+ if($max_loans_allowed){
>+ $needsconfirmation{TOO_MANY} = 1;
>+ $needsconfirmation{current_loan_count} = $current_loan_count;
>+ $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
>+ }
> }
> 
> #
>@@ -835,8 +840,11 @@ sub CanBookBeIssued {
> my $currborinfo = C4::Members::GetMemberDetails( $issue->{borrowernumber} );
> 
> # warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
>- $needsconfirmation{ISSUED_TO_ANOTHER} =
>-"$currborinfo->{'reservedate'} : $currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
>+ $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
>+ $needsconfirmation{issued_firstname} = $currborinfo->{'firstname'};
>+ $needsconfirmation{issued_surname} = $currborinfo->{'surname'};
>+ $needsconfirmation{issued_cardnumber} = $currborinfo->{'cardnumber'};
>+ $needsconfirmation{issued_borrowernumber} = $currborinfo->{'borrowernumber'};
> }
> 
> # See if the item is on reserve.
>@@ -850,13 +858,23 @@ sub CanBookBeIssued {
> {
> # The item is on reserve and waiting, but has been
> # reserved by some other patron.
>- $needsconfirmation{RESERVE_WAITING} =
>-"$resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'}, $branchname)";
>+ $needsconfirmation{RESERVE_WAITING} = 1;
>+ $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
>+ $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
>+ $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
>+ $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
>+ $needsconfirmation{'resbranchname'} = $branchname;
>+ $needsconfirmation{'reswaitingdate'} = format_date($res->{'waitingdate'});
> }
> elsif ( $restype eq "Reserved" ) {
> # The item is on reserve for someone else.
>- $needsconfirmation{RESERVED} =
>-"$res->{'reservedate'} : $resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'})";
>+ $needsconfirmation{RESERVED} = 1;
>+ $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
>+ $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
>+ $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
>+ $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
>+ $needsconfirmation{'resbranchname'} = $branchname;
>+ $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'});
> }
> }
> 	return ( \%issuingimpossible, \%needsconfirmation );
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 9b7014e..f6ffd83 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -1677,6 +1677,7 @@ sub _Findgroupreserve {
> SELECT reserves.biblionumber AS biblionumber,
> reserves.borrowernumber AS borrowernumber,
> reserves.reservedate AS reservedate,
>+ reserves.waitingdate AS waitingdate,
> reserves.branchcode AS branchcode,
> reserves.cancellationdate AS cancellationdate,
> reserves.found AS found,
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index 4b67920..2762b4c 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -298,6 +298,7 @@ if ($barcode) {
> $template->param(
> $needsconfirmation => $$question{$needsconfirmation},
> getTitleMessageIteminfo => $getmessageiteminfo->{'title'},
>+ getBarcodeMessageIteminfo => $getmessageiteminfo->{'barcode'},
> NEEDSCONFIRMATION => 1
> );
> $confirm_required = 1;
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tmpl
>index 980d195..9ab95a3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tmpl
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tmpl
>@@ -212,23 +212,23 @@ function refocus(calendar) {
> 
> 
> 
>- <li>Item is currently checked out to this patron. Renew?</li>
>+ <li>Item  () is currently checked out to this patron. Renew?</li>
> 
> 
> 
>- <li>Item is consigned for </li>
>+ <li>Item  () has been waiting for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber="> </a> () at  since </li>
> 
> 
> 
>- <li>Item is on hold for </li>
>+ <li>Item  () has been on hold for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber="> </a> () at  since </li>
> 
> 
> 
>- <li>Item (  ) checked out to . Check in and check out?</li>
>+ <li>Item  () is checked out to <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber="> </a> (). Check in and check out?</li>
> 
> 
> 
>- <li>Too many checked out (already checked out / max : )</li>
>+ <li>Too many checked out.  checked out, only  are allowed.</li>
> 
> 
> 
>-- 
>1.7.0.4
>

Actions: View | Diff | Splinter Review

Attachments on bug 4946: 2325 | 2326 | 2327 | 2328 | 2329 | 2330