From de0c88f84ab4b3995b7d4ba2e4cde3fe6410dc76 Mon Sep 17 00:00:00 2001 From: Yohann Dufour Date: Fri, 25 Jul 2014 16:14:11 +0200 Subject: [PATCH] Bug 12633: SQLHelper replacement - C4::Members With this patch, the subroutines AddMember and Search uses DBIx::Class instead of C4::SQLHelper. In order to replace SQLHelper in the Search subroutine, 2 subroutines has been added, it seems a lot : replace 1 line with 2 subroutines, but in fact these 2 subroutines replace all the hidden work done by the SQLHelper. Test plan: 1) Apply the patch 12457 in order to get the last unit tests for Members 2) Apply the patch 3) Execute the unit tests by launching : prove t/db_dependent/Members.t 4) The result has to be a success without error or warning : t/db_dependent/Members.t .. ok All tests successful. Files=1, Tests=55, 2 wallclock secs ( 0.04 usr 0.01 sys + 2.27 cusr 0.09 csys = 2.41 CPU) Result: PASS 5) Sign in the intranet. 6) Verify you can add a member in the intranet. 7) Test of members/guarantor_search.pl : Edit a member which belongs to the cateogry type 'C' (Child), click on 'Set to patron' and verify the search of an adult member is correct. 8) Test of patroncards/members-search.pl : Go to Tools > Patron card creator > New batch > "Add item(s)" and verify the search of a member is correct. 9) Test of serials/member-search.pl : Go on the page serials/member-search.pl and verify the search of a member is correct. 10) Test of reserve/request.pl : Go on a biblio details then "Holds" and verify the search of a member is correct. 11) Test of circ/circulation.pl : Go on Circulation > Checkouts and verify the search of a member is correct. --- C4/Members.pm | 112 ++++++++++++++++++++++++++++++++++++++---- members/guarantor_search.pl | 7 ++- patroncards/members-search.pl | 8 +-- serials/member-search.pl | 3 +- 4 files changed, 114 insertions(+), 16 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 326344d..15f6cf8 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -32,7 +32,6 @@ use C4::Reserves; use C4::Accounts; use C4::Biblio; use C4::Letters; -use C4::SQLHelper qw(InsertInTable UpdateInTable SearchInTable); use C4::Members::Attributes qw(SearchIdMatchingAttribute); use C4::NewsChannels; #get slip news use DateTime; @@ -197,9 +196,67 @@ sub _express_member_find { return (undef, $search_on_fields, $searchtype); } +sub _get_cond_for_dbix { + my ($params) = @_; + my $filters = $params->{filters}; + my $exact_search = $params->{exact_search}; + my $search_on_fields = $params->{search_on_fields}; + + my $cond; + if( ref($filters) eq 'ARRAY' ) { + $cond = []; + foreach my $filter (@$filters) { + push @$cond, _get_cond_for_dbix({ + filters => $filter, + exact_search => $exact_search, + search_on_fields => $search_on_fields, + }); + }; + } + elsif( ref($filters) eq 'HASH' ) { + $cond = { %$filters }; + $cond->{'-or'} = _get_columns_cond({ + string => $filters->{''}, + search_on_fields => $search_on_fields, + exact_search => $exact_search, + }) if( exists( $filters->{''} ) ); + delete $cond->{''}; + } + elsif( not ref($filters) ) { + $cond = _get_columns_cond({ + string => $filters, + search_on_fields => $search_on_fields, + exact_search => $exact_search, + }); + } + return $cond; +} + +sub _get_columns_cond { + my ($params) = @_; + my $string = $params->{string}; + my $exact_search = $params->{exact_search} || 1; + my $search_on_fields = $params->{search_on_fields} || undef; + + my $source = Koha::Database->new()->schema->source('Borrower'); + my @columns = $search_on_fields ? @$search_on_fields : $source->columns; + + my $cond = []; + foreach my $column (@columns) { + if( $exact_search ) { + push @$cond, { $column => [ map { $_ } split(' ', $string) ] }; + } + else { + push @$cond, { $column => { like => [ map { $_.'%' } split(' ', $string) ] } }; + } + } + return $cond; +} + sub Search { my ( $filter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype ) = @_; - +use Data::Dumper; +#die Dumper($filter); my $search_string; my $found_borrower; @@ -283,7 +340,25 @@ sub Search { } $searchtype ||= "start_with"; - return SearchInTable( "borrowers", $filter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype ); + my $cond = _get_cond_for_dbix({ + filters => $filter, + exact_search => ($searchtype eq 'exact') ? 1 : 0, + search_on_fields => $search_on_fields, + }); +#die Dumper($filter); +use Data::Dumper; +#die Dumper($cond); + my $rs = Koha::Database->new()->schema->resultset('Borrower'); + $rs = $rs->search( + $cond, + { + ($columns_out) ? { columns => $columns_out } : {}, + order_by => $orderby, + rows => $limit, + }, + ); + $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); + return [ $rs->all ]; } =head2 GetMemberDetails @@ -787,8 +862,18 @@ sub ModMember { } } my $old_categorycode = GetBorrowerCategorycode( $data{borrowernumber} ); - my $execute_success=UpdateInTable("borrowers",\%data); - if ($execute_success) { # only proceed if the update was a success + + # get only the columns of a borrower + 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}; + + my $rs = $schema->resultset('Borrower')->search({ + borrowernumber => $new_borrower->{borrowernumber}, + }); + my $execute_success = $rs->update($new_borrower); + if ($execute_success ne '0E0') { # only proceed if the update was a success # ok if its an adult (type) it may have borrowers that depend on it as a guarantor # so when we update information for an adult we should check for guarantees and update the relevant part # of their records, ie addresses and phone numbers @@ -823,6 +908,7 @@ Returns as undef upon any db error without further processing sub AddMember { my (%data) = @_; my $dbh = C4::Context->dbh; + my $schema = Koha::Database->new()->schema; # generate a proper login if none provided $data{'userid'} = Generate_Userid($data{'borrowernumber'}, $data{'firstname'}, $data{'surname'}) if $data{'userid'} eq ''; @@ -837,9 +923,7 @@ sub AddMember { $data{'dateenrolled'} = C4::Dates->new()->output("iso"); } - my $patron_category = - Koha::Database->new()->schema()->resultset('Category') - ->find( $data{'categorycode'} ); + my $patron_category = $schema->resultset('Category')->find( $data{'categorycode'} ); $data{'privacy'} = $patron_category->default_privacy() eq 'default' ? 1 : $patron_category->default_privacy() eq 'never' ? 2 @@ -848,14 +932,22 @@ sub AddMember { # create a disabled account if no password provided $data{'password'} = ($data{'password'})? hash_password($data{'password'}) : '!'; - $data{'borrowernumber'}=InsertInTable("borrowers",\%data); + $data{'dateofbirth'} = undef if( not $data{'dateofbirth'} ); + + # get only the columns of Borrower + my @columns = $schema->source('Borrower')->columns; + my $new_member = { map { join(' ',@columns) =~ /$_/ ? ( $_ => $data{$_} ) : () } keys(%data) } ; + delete $new_member->{borrowernumber}; + + my $rs = $schema->resultset('Borrower'); + $data{borrowernumber} = $rs->create($new_member)->id; # mysql_insertid is probably bad. not necessarily accurate and mysql-specific at best. logaction("MEMBERS", "CREATE", $data{'borrowernumber'}, "") if C4::Context->preference("BorrowersLog"); AddEnrolmentFeeIfNeeded( $data{categorycode}, $data{borrowernumber} ); - return $data{'borrowernumber'}; + return $data{borrowernumber}; } =head2 Check_Userid diff --git a/members/guarantor_search.pl b/members/guarantor_search.pl index e5b39df..227d526 100755 --- a/members/guarantor_search.pl +++ b/members/guarantor_search.pl @@ -47,6 +47,7 @@ my $orderby = $input->param('orderby'); my $category_type = $input->param('category_type'); $orderby = "surname,firstname" unless $orderby; +my $orderby_array = [ split(',', $orderby) ]; $member =~ s/,//g; #remove any commas from search string $member =~ s/\*/%/g; @@ -61,8 +62,12 @@ my ( $count, $results ); my @resultsdata; if ( $member ne '' ) { + my $rs = Koha::Database->new()->schema->resultset('Category')->search({ category_type => $search_category }, { columns => ['categorycode'] } ); + $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); + my $categorycodes = [ map { $_->{categorycode} } $rs->all ]; + $results = - Search( { '' => $member, category_type => $search_category }, $orderby ); + Search( { '' => $member, categorycode => $categorycodes }, $orderby_array ); $count = $results ? @$results : 0; diff --git a/patroncards/members-search.pl b/patroncards/members-search.pl index ebc53d9..9caa163 100755 --- a/patroncards/members-search.pl +++ b/patroncards/members-search.pl @@ -17,8 +17,7 @@ # with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -use strict; -use warnings; +use Modern::Perl; use CGI; @@ -56,12 +55,13 @@ my ($template, $loggedinuser, $cookie) = get_template_and_user({ debug => 1,}); $orderby = "surname,firstname" unless $orderby; +my $orderby_array = [ split(',', $orderby) ]; $member =~ s/,//g; #remove any commas from search string $member =~ s/\*/%/g; if ($member || $category) { - my $results = $category ? Search({''=>$member, categorycode=>$category}, $orderby) - : Search($member, $orderby); + my $results = $category ? Search({''=>$member, categorycode=>$category}, $orderby_array) + : Search($member, $orderby_array); my $count = $results ? @$results : 0; my @resultsdata = (); diff --git a/serials/member-search.pl b/serials/member-search.pl index dfeb373..53d2933 100755 --- a/serials/member-search.pl +++ b/serials/member-search.pl @@ -96,12 +96,13 @@ $$patron{surname}.="\%" if ($$patron{surname}); my @searchpatron; push @searchpatron, $member if ($member); push @searchpatron, $patron if ( keys %$patron ); +delete $searchpatron[1]->{member}; my $from = ( $startfrom - 1 ) * $resultsperpage; my $to = $from + $resultsperpage; if (@searchpatron) { ($results) = Search( \@searchpatron, - [ { surname => 0 }, { firstname => 0 } ], + [ { -asc => 'surname' }, { -asc => 'firstname' } ], undef, undef, [ "firstname", "surname", "email", "othernames", "cardnumber" ], -- 1.9.1