From 2bf57e27f277e40c424ae9c5c896501da1dddf0d Mon Sep 17 00:00:00 2001 From: Robin Sheat Date: Thu, 23 Jun 2011 21:09:30 +1200 Subject: [PATCH] [SIGNED OFF] Bug 6526 - make the reserves code use the borrowernumber Content-Type: text/plain; charset="utf-8" Previously it used the cardnumber, which caused numerous issues if your users don't have card numbers. --- .../prog/en/modules/reserve/request.tt | 16 ++-- reserve/placerequest.pl | 22 +++--- reserve/request.pl | 77 ++++++++++---------- 3 files changed, 59 insertions(+), 56 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 4b7aa7c..cbcfc55 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -169,7 +169,7 @@ function checkMultiHold() {

Confirm Holds

[% END %] - [% UNLESS ( cardnumber ) %] + [% UNLESS ( borrowernumber ) %] [% IF ( messageborrower ) %]

Patron Not Found

No patron with this name, Please, try another

[% END %] @@ -243,7 +243,7 @@ function checkMultiHold() {
[% END %] - + [% IF ( multi_hold ) %] @@ -262,7 +262,7 @@ function checkMultiHold() { [% END %]
  1. Patron: - [% IF ( cardnumber ) %] + [% IF ( borrowernumber ) %] [% borrowerfirstname %] [% borrowersurname %] ([% cardnumber %]) [% ELSE %] Not defined yet @@ -358,7 +358,7 @@ function checkMultiHold() {
[% UNLESS ( multi_hold ) %]
- [% IF ( cardnumber ) %] + [% IF ( borrowernumber ) %] [% IF ( override_required ) %] [% ELSIF ( none_available ) %] @@ -465,7 +465,7 @@ function checkMultiHold() { [% IF ( bibitemloo.hiddencount ) %] -

Show all items ([% bibitemloo.hiddencount %] hidden)

+

Show all items ([% bibitemloo.hiddencount %] hidden)

[% END %] [% END %] @@ -538,7 +538,7 @@ function checkMultiHold() { [% END %]
- [% IF ( cardnumber ) %] + [% IF ( borrowernumber ) %] [% IF ( override_required ) %] [% ELSIF ( none_available ) %] @@ -552,7 +552,7 @@ function checkMultiHold() {
[% END %] -[% UNLESS ( cardnumber ) %] +[% UNLESS ( borrowernumber ) %] [% IF ( reserveloop ) %]
[% IF ( multi_hold ) %] @@ -631,7 +631,7 @@ function checkMultiHold() { [% IF ( reserveloo.hidename ) %] - [% reserveloo.cardnumber %] + [% reserveloo.cardnumber (reserveloo.borrowernumber) %] [% ELSE %] [% reserveloo.firstname %] [% reserveloo.surname %] [% END %] diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index bc3fc74..19d76be 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -44,14 +44,14 @@ my @bibitems=$input->param('biblioitem'); # and probably remove the reserveconstraint table as well, I never could fill anything in this table. my @reqbib=$input->param('reqbib'); my $biblionumber=$input->param('biblionumber'); -my $borrower=$input->param('member'); +my $borrowernumber=$input->param('borrowernumber'); my $notes=$input->param('notes'); my $branch=$input->param('pickup'); my $startdate=$input->param('reserve_date') || ''; my @rank=$input->param('rank-request'); my $type=$input->param('type'); my $title=$input->param('title'); -my $borrowernumber=GetMember('cardnumber'=>$borrower); +my $borrower=GetMember('borrowernumber'=>$borrowernumber); my $checkitem=$input->param('checkitem'); my $expirationdate = $input->param('expiration_date'); @@ -81,7 +81,7 @@ if ($checkitem ne ''){ } } -if ($type eq 'str8' && $borrowernumber ne ''){ +if ($type eq 'str8' && $borrower){ foreach my $biblionumber (keys %bibinfos) { my $count=@bibitems; @@ -100,18 +100,18 @@ if ($type eq 'str8' && $borrowernumber ne ''){ if ($multi_hold) { my $bibinfo = $bibinfos{$biblionumber}; - AddReserve($branch,$borrowernumber->{'borrowernumber'},$biblionumber,'a',[$biblionumber], + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber], $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); } else { if ($input->param('request') eq 'any'){ # place a request on 1st available - AddReserve($branch,$borrowernumber->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); } elsif ($reqbib[0] ne ''){ # FIXME : elsif probably never reached, (see top of the script) # place a request on a given item - AddReserve($branch,$borrowernumber->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); } else { - AddReserve($branch,$borrowernumber->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); } } } @@ -124,8 +124,10 @@ if ($type eq 'str8' && $borrowernumber ne ''){ } else { print $input->redirect("request.pl?biblionumber=$biblionumber"); } -} elsif ($borrowernumber eq ''){ +} elsif ($borrower eq ''){ print $input->header(); - print "Invalid card number please try again"; - print $input->Dump; + print "Invalid borrower number please try again"; +# Not sure that Dump() does HTML escaping. Use firebug or something to trace +# instead. +# print $input->Dump; } diff --git a/reserve/request.pl b/reserve/request.pl index 0b3bebc..79ff6b5 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -85,7 +85,7 @@ my $CGIbranch = CGI::scrolling_list( my $findborrower = $input->param('findborrower'); $findborrower = '' unless defined $findborrower; $findborrower =~ s|,| |g; -my $cardnumber = $input->param('cardnumber') || ''; +my $borrowernumber_hold = $input->param('borrowernumber') || ''; my $borrowerslist; my $messageborrower; my $warnings; @@ -117,21 +117,19 @@ if ($findborrower) { my @borrowers = @$borrowers; - if ( $#borrowers == -1 ) { - $input->param( 'findborrower', '' ); + if ( !@borrowers ) { $messageborrower = "'$findborrower'"; } - elsif ( $#borrowers == 0 ) { - $input->param( 'cardnumber', $borrowers[0]->{'cardnumber'} ); - $cardnumber = $borrowers[0]->{'cardnumber'}; + elsif ( @borrowers == 1 ) { + $borrowernumber_hold = $borrowers[0]->{'borrowernumber'}; } else { $borrowerslist = \@borrowers; } } -if ($cardnumber) { - my $borrowerinfo = GetMemberDetails( 0, $cardnumber ); +if ($borrowernumber_hold) { + my $borrowerinfo = GetMemberDetails( $borrowernumber_hold ); my $diffbranch; my @getreservloop; my $count_reserv = 0; @@ -143,7 +141,7 @@ if ($cardnumber) { my $number_reserves = GetReserveCount( $borrowerinfo->{'borrowernumber'} ); - if ( C4::Context->preference('maxreserves') && $number_reserves >= C4::Context->preference('maxreserves') ) { + if ( C4::Context->preference('maxreserves') && ($number_reserves >= C4::Context->preference('maxreserves')) ) { $warnings = 1; $maxreserves = 1; } @@ -164,24 +162,25 @@ if ($cardnumber) { } $template->param( - borrowernumber => $borrowerinfo->{'borrowernumber'}, - borrowersurname => $borrowerinfo->{'surname'}, - borrowerfirstname => $borrowerinfo->{'firstname'}, - borrowerstreetaddress => $borrowerinfo->{'address'}, - borrowercity => $borrowerinfo->{'city'}, - borrowerphone => $borrowerinfo->{'phone'}, - borrowermobile => $borrowerinfo->{'mobile'}, - borrowerfax => $borrowerinfo->{'fax'}, - borrowerphonepro => $borrowerinfo->{'phonepro'}, - borroweremail => $borrowerinfo->{'email'}, - borroweremailpro => $borrowerinfo->{'emailpro'}, - borrowercategory => $borrowerinfo->{'category'}, - borrowerreservs => $count_reserv, - maxreserves => $maxreserves, - expiry => $expiry, - diffbranch => $diffbranch, - messages => $messages, - warnings => $warnings + borrowernumber => $borrowerinfo->{'borrowernumber'}, + borrowersurname => $borrowerinfo->{'surname'}, + borrowerfirstname => $borrowerinfo->{'firstname'}, + borrowerstreetaddress => $borrowerinfo->{'address'}, + borrowercity => $borrowerinfo->{'city'}, + borrowerphone => $borrowerinfo->{'phone'}, + borrowermobile => $borrowerinfo->{'mobile'}, + borrowerfax => $borrowerinfo->{'fax'}, + borrowerphonepro => $borrowerinfo->{'phonepro'}, + borroweremail => $borrowerinfo->{'email'}, + borroweremailpro => $borrowerinfo->{'emailpro'}, + borrowercategory => $borrowerinfo->{'category'}, + borrowerreservs => $count_reserv, + cardnumber => $borrowerinfo->{'cardnumber'}, + maxreserves => $maxreserves, + expiry => $expiry, + diffbranch => $diffbranch, + messages => $messages, + warnings => $warnings ); } @@ -200,18 +199,18 @@ if ($borrowerslist) { } @{$borrowerslist} ) { - push @values, $borrower->{cardnumber}; + push @values, $borrower->{borrowernumber}; - $labels{ $borrower->{cardnumber} } = sprintf( + $labels{ $borrower->{borrowernumber} } = sprintf( '%s, %s ... (%s - %s) ... %s', - $borrower->{surname}, $borrower->{firstname}, - $borrower->{cardnumber}, $borrower->{categorycode}, - $borrower->{address}, + $borrower->{surname} ||'', $borrower->{firstname} || '', + $borrower->{cardnumber} || '', $borrower->{categorycode} || '', + $borrower->{address} || '', ); } $CGIselectborrower = CGI::scrolling_list( - -name => 'cardnumber', + -name => 'borrowernumber', -values => \@values, -labels => \%labels, -size => 7, @@ -220,7 +219,7 @@ if ($borrowerslist) { } # FIXME launch another time GetMemberDetails perhaps until -my $borrowerinfo = GetMemberDetails( 0, $cardnumber ); +my $borrowerinfo = GetMemberDetails( $borrowernumber_hold ); my @biblionumbers = (); my $biblionumbers = $input->param('biblionumbers'); @@ -238,7 +237,7 @@ foreach my $biblionumber (@biblionumbers) { my $dat = GetBiblioData($biblionumber); - if ( not CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { + unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { $warnings = 1; $maxreserves = 1; } @@ -429,11 +428,14 @@ foreach my $biblionumber (@biblionumbers) { $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; if ( $branchitemrule->{'holdallowed'} == 0 || - ( $branchitemrule->{'holdallowed'} == 1 && $borrowerinfo->{'branchcode'} ne $item->{'homebranch'} ) ) { + ( $branchitemrule->{'holdallowed'} == 1 && + $borrowerinfo->{'branchcode'} ne $item->{'homebranch'} ) ) { $policy_holdallowed = 0; } - if (IsAvailableForItemLevelRequest($itemnumber) and not $item->{cantreserve} and CanItemBeReserved($borrowerinfo->{borrowernumber}, $itemnumber) ) { + if (IsAvailableForItemLevelRequest($itemnumber) and + not $item->{cantreserve} and + CanItemBeReserved($borrowerinfo->{borrowernumber}, $itemnumber) ) { if ( $policy_holdallowed ) { $item->{available} = 1; $num_available++; @@ -564,7 +566,6 @@ foreach my $biblionumber (@biblionumbers) { date => $date, biblionumber => $biblionumber, findborrower => $findborrower, - cardnumber => $cardnumber, title => $dat->{title}, author => $dat->{author}, holdsview => 1, -- 1.7.2.5