From 3f1da7cdbc20af13e2f05b0137c0b66bbf0399f4 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 8 Nov 2016 11:03:54 +0100 Subject: [PATCH] [SIGNED-OFF] Bug 17578: GetMemberDetails - Remove GetMemberDetails All the values different from the ones GetMember returned has been managed outside of GetMemberDetails. It looks safe to replace all the occurrences of GetMemberDetails with GetMember. Signed-off-by: Josef Moravec --- C4/Circulation.pm | 14 ++++----- C4/ILSDI/Services.pm | 12 ++++---- C4/Members.pm | 51 --------------------------------- C4/SIP/ILS/Patron.pm | 4 +-- circ/circulation.pl | 4 +-- members/purchase-suggestions.pl | 2 +- members/routing-lists.pl | 2 +- members/statistics.pl | 2 +- misc/export_borrowers.pl | 7 +++-- opac/opac-account-pay-paypal-return.pl | 2 +- opac/opac-readingrecord.pl | 2 +- opac/opac-renew.pl | 2 +- opac/opac-reserve.pl | 4 +-- opac/opac-showreviews.pl | 4 +-- opac/opac-suggestions.pl | 2 +- opac/opac-user.pl | 2 +- opac/sco/sco-main.pl | 6 ++-- t/db_dependent/Acquisition/OrderUsers.t | 2 +- t/db_dependent/Members.t | 4 +-- t/db_dependent/rollingloans.t | 2 +- tools/viewlog.pl | 4 +-- 21 files changed, 42 insertions(+), 92 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2191fc9..d4f3cdb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -570,7 +570,7 @@ C<$issuingimpossible> and C<$needsconfirmation> are some hashref. =over 4 -=item C<$borrower> hash with borrower informations (from GetMember or GetMemberDetails) +=item C<$borrower> hash with borrower informations (from GetMember) =item C<$barcode> is the bar code of the book being issued. @@ -1230,7 +1230,7 @@ Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this =over 4 -=item C<$borrower> is a hash with borrower informations (from GetMember or GetMemberDetails). +=item C<$borrower> is a hash with borrower informations (from GetMember). =item C<$barcode> is the barcode of the item being issued. @@ -1828,7 +1828,7 @@ sub AddReturn { my $issue = GetItemIssue($itemnumber); if ($issue and $issue->{borrowernumber}) { - $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber}) + $borrower = C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} ) or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '$issue->{borrowernumber}'\n" . Dumper($issue) . "\n"; } else { @@ -2984,7 +2984,7 @@ sub AddRenewal { # Send a renewal slip according to checkout alert preferencei if ( C4::Context->preference('RenewalSendNotice') eq '1' ) { - $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 ); + $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ); my $circulation_alert = 'C4::ItemCirculationAlertPreference'; my %conditions = ( branchcode => $branch, @@ -3093,7 +3093,7 @@ sub GetSoonestRenewDate { my $itemissue = GetItemIssue($itemnumber) or return; $borrowernumber ||= $itemissue->{borrowernumber}; - my $borrower = C4::Members::GetMemberDetails($borrowernumber) + my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return; my $branchcode = _GetCircControlBranch( $item, $borrower ); @@ -3757,7 +3757,7 @@ sub LostItem{ # If a borrower lost the item, add a replacement cost to the their record if ( my $borrowernumber = $issues->{borrowernumber} ){ - my $borrower = C4::Members::GetMemberDetails( $borrowernumber ); + my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ); if (C4::Context->preference('WhenLostForgiveFine')){ my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, 1, 0); # 1, 0 = exemptfine, no-dropbox @@ -3853,7 +3853,7 @@ sub ProcessOfflineReturn { sub ProcessOfflineIssue { my $operation = shift; - my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber + my $borrower = C4::Members::GetMember( cardnumber => $operation->{cardnumber} ); if ( $borrower->{borrowernumber} ) { my $itemnumber = C4::Items::GetItemnumberFromBarcode( $operation->{barcode} ); diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 33d15cb..e8ae7c0 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -456,7 +456,7 @@ sub GetPatronStatus { # Get Member details my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Return the results @@ -485,7 +485,7 @@ sub GetServices { # Get the member, or return an error code if not found my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Get the item, or return an error code if not found @@ -557,7 +557,7 @@ sub RenewLoan { # Get borrower infos or return an error code my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Get the item, or return an error code @@ -607,7 +607,7 @@ sub HoldTitle { # Get the borrower or return an error code my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Get the biblio record, or return an error code @@ -675,7 +675,7 @@ sub HoldItem { # Get the borrower or return an error code my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Get the biblio or return an error code @@ -742,7 +742,7 @@ sub CancelHold { # Get the borrower or return an error code my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Get the reserve or return an error code diff --git a/C4/Members.pm b/C4/Members.pm index f00b291..586761b 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -60,7 +60,6 @@ BEGIN { @ISA = qw(Exporter); #Get data push @EXPORT, qw( - &GetMemberDetails &GetMember &GetMemberIssuesAndFines @@ -126,56 +125,6 @@ This module contains routines for adding, modifying and deleting members/patrons =head1 FUNCTIONS -=head2 GetMemberDetails - -($borrower) = &GetMemberDetails($borrowernumber, $cardnumber); - -Looks up a patron and returns information about him or her. If -C<$borrowernumber> is true (nonzero), C<&GetMemberDetails> looks -up the borrower by number; otherwise, it looks up the borrower by card -number. - -C<$borrower> is a reference-to-hash whose keys are the fields of the -borrowers table in the Koha database. In addition, - -=cut - -sub GetMemberDetails { - my ( $borrowernumber, $cardnumber ) = @_; - my $dbh = C4::Context->dbh; - my $query; - my $sth; - if ($borrowernumber) { - $sth = $dbh->prepare(" - SELECT borrowers.*, - category_type, - categories.description, - FROM borrowers - LEFT JOIN categories ON borrowers.categorycode=categories.categorycode - WHERE borrowernumber = ? - "); - $sth->execute($borrowernumber); - } - elsif ($cardnumber) { - $sth = $dbh->prepare(" - SELECT borrowers.*, - category_type, - categories.description, - FROM borrowers - LEFT JOIN categories ON borrowers.categorycode = categories.categorycode - WHERE cardnumber = ? - "); - $sth->execute($cardnumber); - } - else { - return; - } - my $borrower = $sth->fetchrow_hashref; - return unless $borrower; - - return ($borrower); -} - =head2 patronflags $flags = &patronflags($patron); diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 59aedab..0012776 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -37,8 +37,8 @@ sub new { syslog("LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id); return; } - $kp = GetMemberDetails($kp->{borrowernumber}); - $debug and warn "new Patron (GetMemberDetails): " . Dumper($kp); + $kp = GetMember( borrowernumber => $kp->{borrowernumber}); + $debug and warn "new Patron (GetMember): " . Dumper($kp); my $pw = $kp->{password}; my $flags = C4::Members::patronflags( $kp ); my $debarred = defined($flags->{DBARRED}); diff --git a/circ/circulation.pl b/circ/circulation.pl index a816f96..e92fbab 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -265,7 +265,7 @@ if ($findborrower) { my $patron; if ($borrowernumber) { $patron = Koha::Patrons->find( $borrowernumber ); - $borrower = GetMemberDetails( $borrowernumber, 0 ); + $borrower = GetMember( borrowernumber => $borrowernumber ); my ( $od, $issue, $fines ) = GetMemberIssuesAndFines( $borrowernumber ); # Warningdate is the date that the warning starts appearing @@ -454,7 +454,7 @@ if (@$barcodes) { # reload the borrower info for the sake of reseting the flags..... if ($borrowernumber) { - $borrower = GetMemberDetails( $borrowernumber, 0 ); + $borrower = GetMember( borrowernumber => $borrowernumber ); } ################################################################################## diff --git a/members/purchase-suggestions.pl b/members/purchase-suggestions.pl index 2f7ce49..f967b10 100755 --- a/members/purchase-suggestions.pl +++ b/members/purchase-suggestions.pl @@ -43,7 +43,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $borrowernumber = $input->param('borrowernumber'); # Set informations for the patron -my $borrower = GetMemberDetails( $borrowernumber, 0 ); +my $borrower = GetMember( borrowernumber => $borrowernumber ); foreach my $key ( keys %$borrower ) { $template->param( $key => $borrower->{$key} ); } diff --git a/members/routing-lists.pl b/members/routing-lists.pl index 7c99170..63cc3da 100755 --- a/members/routing-lists.pl +++ b/members/routing-lists.pl @@ -51,7 +51,7 @@ my $branch = C4::Context->userenv->{'branch'}; # get the borrower information..... my $borrower; if ($borrowernumber) { - $borrower = GetMemberDetails( $borrowernumber, 0 ); + $borrower = GetMember( borrowernumber => $borrowernumber ); } diff --git a/members/statistics.pl b/members/statistics.pl index f8ead98..1ea7f15 100755 --- a/members/statistics.pl +++ b/members/statistics.pl @@ -48,7 +48,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $borrowernumber = $input->param('borrowernumber'); # Set informations for the patron -my $borrower = GetMemberDetails( $borrowernumber, 0 ); +my $borrower = GetMember( borrowernumber => $borrowernumber ); if ( not defined $borrower ) { $template->param (unknowuser => 1); output_html_with_http_headers $input, $cookie, $template->output; diff --git a/misc/export_borrowers.pl b/misc/export_borrowers.pl index 6f0985a..fd9d616 100755 --- a/misc/export_borrowers.pl +++ b/misc/export_borrowers.pl @@ -41,7 +41,7 @@ $0 [--field=FIELD [--field=FIELD [...]]] [--separator=CHAR] [--show-header] [--w $0 -h -f, --field=FIELD Field to export. It is repeatable and has to match - keys returned by the GetMemberDetails function. + keys returned by the GetMember function. If no field is specified, then all fields will be exported. -s, --separator=CHAR This character will be used to separate fields. @@ -99,7 +99,8 @@ my $csv = Text::CSV->new( { sep_char => $separator, binary => 1 } ); # If the user did not specify any field to export, we assume he wants them all # We retrieve the first borrower informations to get field names my ($borrowernumber) = $sth->fetchrow_array or die "No borrower to export"; -my $member = GetMemberDetails($borrowernumber); +my $member = GetMember($borrowernumber); # FIXME Now is_expired is no longer available + # We will have to use Koha::Patron and allow method calls @fields = keys %$member unless (@fields); if ($show_header) { @@ -120,7 +121,7 @@ die "Invalid character at borrower $borrowernumber: [" print $csv->string . "\n"; while ( my $borrowernumber = $sth->fetchrow_array ) { - $member = GetMemberDetails($borrowernumber); + $member = GetMember( borrowernumber => $borrowernumber ); $csv->combine( map { ( defined $member->{$_} and !ref $member->{$_} ) diff --git a/opac/opac-account-pay-paypal-return.pl b/opac/opac-account-pay-paypal-return.pl index 742f5ae..77b50d8 100755 --- a/opac/opac-account-pay-paypal-return.pl +++ b/opac/opac-account-pay-paypal-return.pl @@ -109,7 +109,7 @@ else { } $template->param( - borrower => GetMemberDetails($borrowernumber), + borrower => GetMember( borrowernumber => $borrowernumber ), accountview => 1 ); diff --git a/opac/opac-readingrecord.pl b/opac/opac-readingrecord.pl index ca37612..47de094 100755 --- a/opac/opac-readingrecord.pl +++ b/opac/opac-readingrecord.pl @@ -51,7 +51,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( ); # get borrower information .... -my ( $borr ) = GetMemberDetails( $borrowernumber ); +my ( $borr ) = GetMember( borrowernumber => $borrowernumber ); $template->param(%{$borr}); diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl index 64e95aa..03c4a71 100755 --- a/opac/opac-renew.pl +++ b/opac/opac-renew.pl @@ -70,7 +70,7 @@ else { $branchcode = $item->{'homebranch'}; } elsif ( $renewalbranch eq 'patronhomebranch' ) { - my $borrower = GetMemberDetails($borrowernumber); + my $borrower = GetMember( borrowernumber => $borrowernumber ); $branchcode = $borrower->{'branchcode'}; } elsif ( $renewalbranch eq 'checkoutbranch' ) { diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 7363236..d11e9af 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -70,7 +70,7 @@ sub get_out { } # get borrower information .... -my ( $borr ) = GetMemberDetails( $borrowernumber ); +my ( $borr ) = GetMember( borrowernumber => $borrowernumber ); my $patron = Koha::Patrons->find( $borrowernumber ); # check if this user can place a reserve, -1 means use sys pref, 0 means dont block, 1 means block @@ -449,7 +449,7 @@ foreach my $biblioNum (@biblionumbers) { # checking reserve my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum); - my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0); + my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); # the item could be reserved for this borrower vi a host record, flag this $reservedfor //= ''; diff --git a/opac/opac-showreviews.pl b/opac/opac-showreviews.pl index 839a536..f85a833 100755 --- a/opac/opac-showreviews.pl +++ b/opac/opac-showreviews.pl @@ -27,7 +27,7 @@ use C4::Koha; use C4::Output; use C4::Circulation; use C4::Biblio; -use C4::Members qw/GetMemberDetails/; +use C4::Members qw/GetMember/; use Koha::DateUtils; use Koha::Reviews; use POSIX qw(ceil floor strftime); @@ -92,7 +92,7 @@ for my $result (@$reviews){ my $bib = &GetBiblioData($biblionumber); my $record = GetMarcBiblio($biblionumber); my $frameworkcode = GetFrameworkCode($biblionumber); - my ( $borr ) = GetMemberDetails( $result->{borrowernumber} ); + my ( $borr ) = GetMember( borrowernumber => $result->{borrowernumber} ); $result->{normalized_upc} = GetNormalizedUPC($record,$marcflavour); $result->{normalized_ean} = GetNormalizedEAN($record,$marcflavour); $result->{normalized_oclc} = GetNormalizedOCLCNumber($record,$marcflavour); diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl index cd8816a..d300769 100755 --- a/opac/opac-suggestions.pl +++ b/opac/opac-suggestions.pl @@ -195,7 +195,7 @@ my $patron_reason_loop = GetAuthorisedValues("OPAC_SUG"); # Is the person allowed to choose their branch if ( C4::Context->preference("AllowPurchaseSuggestionBranchChoice") ) { - my ( $borr ) = GetMemberDetails( $borrowernumber ); + my ( $borr ) = GetMember( borrowernumber => $borrowernumber ); # pass the pickup branch along.... my $userbranch = ''; diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 020e0f8..04a94c0 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -85,7 +85,7 @@ if (!$borrowernumber) { } # get borrower information .... -my ( $borr ) = GetMemberDetails( $borrowernumber ); +my ( $borr ) = GetMember( borrowernumber => $borrowernumber ); my ( $today_year, $today_month, $today_day) = Today(); my ($warning_year, $warning_month, $warning_day) = split /-/, $borr->{'dateexpiry'}; diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index 6d2eb57..17557a8 100755 --- a/opac/sco/sco-main.pl +++ b/opac/sco/sco-main.pl @@ -106,14 +106,14 @@ my ($op, $patronid, $patronlogin, $patronpw, $barcode, $confirmed) = ( my $issuenoconfirm = 1; #don't need to confirm on issue. #warn "issuerid: " . $issuerid; -my $issuer = GetMemberDetails($issuerid); +my $issuer = GetMember( borrowernumber => $issuerid ); my $item = GetItem(undef,$barcode); if (C4::Context->preference('SelfCheckoutByLogin') && !$patronid) { my $dbh = C4::Context->dbh; my $resval; ($resval, $patronid) = checkpw($dbh, $patronlogin, $patronpw); } -my $borrower = GetMemberDetails(undef,$patronid); +my $borrower = GetMember( cardnumber => $patronid ); my $currencySymbol = ""; if ( my $active_currency = Koha::Acquisition::Currencies->get_active ) { @@ -131,7 +131,7 @@ if ($op eq "logout") { elsif ( $op eq "returnbook" && $allowselfcheckreturns ) { my ($doreturn) = AddReturn( $barcode, $branch ); #warn "returnbook: " . $doreturn; - $borrower = GetMemberDetails(undef,$patronid); + $borrower = GetMember( cardnumber => $patronid ); } elsif ( $op eq "checkout" ) { my $impossible = {}; diff --git a/t/db_dependent/Acquisition/OrderUsers.t b/t/db_dependent/Acquisition/OrderUsers.t index 51e7877..2d09ef5 100644 --- a/t/db_dependent/Acquisition/OrderUsers.t +++ b/t/db_dependent/Acquisition/OrderUsers.t @@ -76,7 +76,7 @@ my $borrowernumber = C4::Members::AddMember( userid => 'TESTUSERID' ); -my $borrower = C4::Members::GetMemberDetails( $borrowernumber ); +my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ); C4::Acquisition::ModOrderUsers( $ordernumber, $borrowernumber ); diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 27c3c63..c68d329 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -95,7 +95,7 @@ testAgeAccessors(\%data); #Age accessor tests don't touch the db so it is safe t my $addmem=AddMember(%data); ok($addmem, "AddMember()"); -my $member=GetMemberDetails("",$CARDNUMBER) +my $member = GetMember( cardnumber => $CARDNUMBER ) or BAIL_OUT("Cannot read member with card $CARDNUMBER"); ok ( $member->{firstname} eq $FIRSTNAME && @@ -112,7 +112,7 @@ $member->{email} = $EMAIL; $member->{phone} = $PHONE; $member->{emailpro} = $EMAILPRO; ModMember(%$member); -my $changedmember=GetMemberDetails("",$CARDNUMBER); +my $changedmember = GetMember( cardnumber => $CARDNUMBER ); ok ( $changedmember->{firstname} eq $CHANGED_FIRSTNAME && $changedmember->{email} eq $EMAIL && $changedmember->{phone} eq $PHONE && diff --git a/t/db_dependent/rollingloans.t b/t/db_dependent/rollingloans.t index 2b6ca50..5274808 100644 --- a/t/db_dependent/rollingloans.t +++ b/t/db_dependent/rollingloans.t @@ -41,7 +41,7 @@ SKIP: { sub try_issue { my ($cardnumber, $item ) = @_; my $issuedate = '2011-05-16'; - my $borrower = GetMemberDetails(0, $cardnumber); + my $borrower = GetMember( cardnumber => $cardnumber ); my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( $borrower, $item ); my $issue = AddIssue($borrower, $item, undef, 0, $issuedate); return dt_from_string( $issue->due_date() ); diff --git a/tools/viewlog.pl b/tools/viewlog.pl index 3c37162..b7ddb6a 100755 --- a/tools/viewlog.pl +++ b/tools/viewlog.pl @@ -136,7 +136,7 @@ if ($do_it) { #always add firstname and surname for librarian/user if ( $result->{'user'} ) { - my $userdetails = C4::Members::GetMemberDetails( $result->{'user'} ); + my $userdetails = C4::Members::GetMember( borrowernumber => $result->{'user'} ); if ($userdetails) { $result->{'userfirstname'} = $userdetails->{'firstname'}; $result->{'usersurname'} = $userdetails->{'surname'}; @@ -146,7 +146,7 @@ if ($do_it) { #add firstname and surname for borrower, when using the CIRCULATION, MEMBERS, FINES if ( $result->{module} eq "CIRCULATION" || $result->{module} eq "MEMBERS" || $result->{module} eq "FINES" ) { if ( $result->{'object'} ) { - my $borrowerdetails = C4::Members::GetMemberDetails( $result->{'object'} ); + my $borrowerdetails = C4::Members::GetMember( borrowernumber => $result->{'object'} ); if ($borrowerdetails) { $result->{'borrowerfirstname'} = $borrowerdetails->{'firstname'}; $result->{'borrowersurname'} = $borrowerdetails->{'surname'}; -- 2.1.4