Attachment 30115 Details for Bug 12633 – Bug 12633: SQLHelper replacement - C4::Members

[patch] Bug 12633: SQLHelper replacement - C4::Members

Bug-12633-SQLHelper-replacement---C4Members.patch (text/plain), 11.01 KB, created by Yohann Dufour on 2014-07-25 14:22:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Yohann Dufour

Created: 2014-07-25 14:22:00 UTC

Size: 11.01 KB

patch

obsolete

>From de0c88f84ab4b3995b7d4ba2e4cde3fe6410dc76 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] 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

Actions: View | Diff | Splinter Review

Attachments on bug 12633: 30115 | 30119 | 30547 | 30937 | 37751 | 38272 | 38284 | 38285 | 38697 | 38698 | 57270