From 022eb725c4329386c61cb7984b5ec4e45a5cdf77 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 7 Nov 2016 16:52:54 +0000 Subject: [PATCH] Bug 17578: GetMemberDetails - Remove flags Same as authflags, a flags key is set containing all the patron flags. It is only used in a few places and it's better to call C4::Members::patronflags when we need it. Test plan: Look at the diff and confirm that the change make sense Use git grep to confirm we do not use the flags somewhere else. --- C4/Circulation.pm | 10 ++++++---- C4/ILSDI/Services.pm | 6 +++--- C4/Members.pm | 16 ---------------- C4/SIP/ILS/Patron.pm | 6 +++--- circ/circulation.pl | 4 ++-- members/deletemem.pl | 4 ++-- members/member-flags.pl | 2 +- 7 files changed, 17 insertions(+), 31 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 782ca7b..eee0954 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -719,14 +719,16 @@ sub CanBookBeIssued { ModDateLastSeen( $item->{'itemnumber'} ); return( { STATS => 1 }, {}); } - if ( ref $borrower->{flags} ) { - if ( $borrower->{flags}->{GNA} ) { + + my $flags = C4::Members::patronflags( $borrower ); + if ( ref $flags ) { + if ( $flags->{GNA} ) { $issuingimpossible{GNA} = 1; } - if ( $borrower->{flags}->{'LOST'} ) { + if ( $flags->{'LOST'} ) { $issuingimpossible{CARD_LOST} = 1; } - if ( $borrower->{flags}->{'DBARRED'} ) { + if ( $flags->{'DBARRED'} ) { $issuingimpossible{DEBARRED} = 1; } } diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 7c5e5f3..807c63b 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -361,14 +361,14 @@ sub GetPatronInfo { # Get Member details my $borrowernumber = $cgi->param('patron_id'); - my $borrower = GetMemberDetails( $borrowernumber ); + my $borrower = GetMember( borrowernumber => $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; # Cleaning the borrower hashref - $borrower->{'charges'} = $borrower->{'flags'}->{'CHARGES'}->{'amount'}; + my $flags = C4::Members::patronflags( $borrowernumber ); + $borrower->{'charges'} = $flags>{'CHARGES'}->{'amount'}; my $library = Koha::Libraries->find( $borrower->{branchcode} ); $borrower->{'branchname'} = $library ? $library->branchname : ''; - delete $borrower->{'flags'}; delete $borrower->{'userid'}; delete $borrower->{'password'}; diff --git a/C4/Members.pm b/C4/Members.pm index f43d052..4360375 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -136,18 +136,6 @@ number. C<$borrower> is a reference-to-hash whose keys are the fields of the borrowers table in the Koha database. In addition, -C<$borrower-E{flags}> is a hash giving more detailed information -about the patron. Its keys act as flags : - - if $borrower->{flags}->{LOST} { - # Patron's card was reported lost - } - -If the state of a flag means that the patron should not be -allowed to borrow any more books, then it will have a C key -with a true value. - -See patronflags for more details. =cut @@ -188,9 +176,6 @@ sub GetMemberDetails { my $borrower = $sth->fetchrow_hashref; return unless $borrower; - my $flags = patronflags( $borrower); - $borrower->{'flags'} = $flags; - $borrower->{'is_expired'} = 0; $borrower->{'is_expired'} = 1 if defined($borrower->{dateexpiry}) && @@ -499,7 +484,6 @@ sub ModMember { my $schema = Koha::Database->new()->schema; my @columns = $schema->source('Borrower')->columns; my $new_borrower = { map { join(' ', @columns) =~ /$_/ ? ( $_ => $data{$_} ) : () } keys(%data) }; - delete $new_borrower->{flags}; $new_borrower->{dateofbirth} ||= undef if exists $new_borrower->{dateofbirth}; $new_borrower->{dateenrolled} ||= undef if exists $new_borrower->{dateenrolled}; diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 26ad963..e1a714c 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -40,9 +40,9 @@ sub new { $kp = GetMemberDetails($kp->{borrowernumber}); $debug and warn "new Patron (GetMemberDetails): " . Dumper($kp); my $pw = $kp->{password}; - my $flags = $kp->{flags}; # or warn "Warning: No flags from patron object for '$patron_id'"; - my $debarred = defined($kp->{flags}->{DBARRED}); - $debug and warn sprintf("Debarred = %s : ", ($debarred||'undef')) . Dumper(%{$kp->{flags}}); + my $flags = C4::Members::patronflags( $kp->{borrowernumber} ); + my $debarred = defined($flags->{DBARRED}); + $debug and warn sprintf("Debarred = %s : ", ($debarred||'undef')) . Dumper(%$flags); my ($day, $month, $year) = (localtime)[3,4,5]; my $today = sprintf '%04d-%02d-%02d', $year+1900, $month+1, $day; my $expired = ($today gt $kp->{dateexpiry}) ? 1 : 0; diff --git a/circ/circulation.pl b/circ/circulation.pl index ded56c4..405a4c9 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -472,7 +472,7 @@ if ($borrowernumber) { } #title -my $flags = $borrower->{'flags'}; +my $flags = C4::Members::patronflags( $borrowernumber ); foreach my $flag ( sort keys %$flags ) { $flags->{$flag}->{'message'} =~ s#\n#
#g; if ( $flags->{$flag}->{'noissues'} ) { @@ -555,7 +555,7 @@ foreach my $flag ( sort keys %$flags ) { } } -my $amountold = $borrower->{flags} ? $borrower->{flags}->{'CHARGES'}->{'message'} || 0 : 0; +my $amountold = $flags ? $flags->{'CHARGES'}->{'message'} || 0 : 0; $amountold =~ s/^.*\$//; # remove upto the $, if any my ( $total, $accts, $numaccts) = GetMemberAccountRecords( $borrowernumber ); diff --git a/members/deletemem.pl b/members/deletemem.pl index b51c721..2917c5c 100755 --- a/members/deletemem.pl +++ b/members/deletemem.pl @@ -76,8 +76,8 @@ if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preferen my $issues = GetPendingIssues($member); # FIXME: wasteful call when really, we only want the count my $countissues = scalar(@$issues); -my ($bor)=GetMemberDetails($member,''); -my $flags=$bor->{flags}; +my $bor = C4::Members::GetMember( borrowernumbe => $member ); +my $flags = C4::Members::patronflags( $member ); my $userenv = C4::Context->userenv; diff --git a/members/member-flags.pl b/members/member-flags.pl index 8ab4ee3..725a9d0 100755 --- a/members/member-flags.pl +++ b/members/member-flags.pl @@ -86,7 +86,7 @@ if ($input->param('newflags')) { print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member"); } else { - my $flags = $bor->{'flags'}; + my $flags = C4::Members::patronflags( $member ); my $accessflags; my $dbh = C4::Context->dbh(); # FIXME This needs to be improved to avoid doing the same query -- 2.1.4