Bugzilla – Attachment 30547 Details for
Bug 12633
SQLHelper replacement - C4::Members
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[Signed-off] Bug 12633: SQLHelper replacement - C4::Members
Signed-off-Bug-12633-SQLHelper-replacement---C4Mem.patch (text/plain), 11.06 KB, created by
Marc Véron
on 2014-08-05 19:44:08 UTC
(
hide
)
Description:
[Signed-off] Bug 12633: SQLHelper replacement - C4::Members
Filename:
MIME Type:
Creator:
Marc Véron
Created:
2014-08-05 19:44:08 UTC
Size:
11.06 KB
patch
obsolete
>From ca4a0342ab2aca2f9d4ea509edf5c8487a65030e Mon Sep 17 00:00:00 2001 >From: Yohann Dufour <dufour.yohann@gmail.com> >Date: Fri, 25 Jul 2014 16:14:11 +0200 >Subject: [PATCH] [Signed-off] Bug 12633: SQLHelper replacement - C4::Members >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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. > >Followed test plan. Patch behaves as expected. >Signed-off-by: Marc Véron <veron@veron.ch> >--- > C4/Members.pm | 107 +++++++++++++++++++++++++++++++++++++---- > members/guarantor_search.pl | 7 ++- > patroncards/members-search.pl | 8 +-- > serials/member-search.pl | 3 +- > 4 files changed, 110 insertions(+), 15 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index 326344d..4aa1e92 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,6 +196,63 @@ 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 ) = @_; > >@@ -283,7 +339,23 @@ 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, >+ }); >+ >+ 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 +859,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 +905,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 +920,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 +929,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.7.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=30547">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12633
:
30115
|
30119
|
30547
|
30937
|
37751
|
38272
|
38284
|
38285
|
38697
|
38698
|
57270