Attachment 79850 Details for Bug 21465 – Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron

[patch] Bug 21465: Don't throw duplicate userid error if userid belongs to the matched patron

Bug-21465-Dont-throw-duplicate-userid-error-if-use.patch (text/plain), 11.10 KB, created by Nick Clemens (kidclamp) on 2018-10-02 20:07:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2018-10-02 20:07:15 UTC

Size: 11.10 KB

patch

obsolete

>From ed1489579e648e5d294489ca661e4802188ea8ea Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Tue, 2 Oct 2018 19:48:02 +0000
>Subject: [PATCH] Bug 21465: Don't throw duplicate userid error if userid
> belongs to the matched patron
>
>To test:
> 1 - Export your patrons
>    a - Create a report 'SELECT * FROM borrowers'
>    b - Run and save the report as csv (check your delimiter)
>    c - Delete the borrowernumebr column
> 2 - Use the Patron Import tool to import the csv from above
> 3 - Set matching to 'cardnumber'
> 4 - Set 'If matching record is already in the borrowers table:' to
>Overwrite
> 5 - Import
> 6 - None are import because of matchign userid (their own)
> 7 - Apply patch
> 8 - Repeat
> 9 - Patrons are successfully overwritten
>10 - prove -v t/db_dependent/Koha/Patrons/Import.t
>11 - prove -v t/db_dependent/Koha/Patrons.t
>---
> Koha/ExternalContent/OverDrive.pm    |  6 +++-
> Koha/Patrons/Import.pm               |  1 +
> t/db_dependent/Koha/Patrons.t        |  3 +-
> t/db_dependent/Koha/Patrons/Import.t | 61 ++++++++++++++++++++++++++++++------
> 4 files changed, 60 insertions(+), 11 deletions(-)
>
>diff --git a/Koha/ExternalContent/OverDrive.pm b/Koha/ExternalContent/OverDrive.pm
>index 628e761..c10418d 100644
>--- a/Koha/ExternalContent/OverDrive.pm
>+++ b/Koha/ExternalContent/OverDrive.pm
>@@ -149,15 +149,19 @@ sub auth_by_code {
> =cut
> 
> sub auth_by_userid {
>+    warn "Here";
>     my $self = shift;
>     my $userid = shift or croak "No user provided";
>     my $password = shift;
>     croak "No password provided" unless ($password || !C4::Context->preference("OverDrivePasswordRequired"));
>     my $website_id = shift or croak "OverDrive Library ID not provided";
>     my $authorization_name = shift or croak "OverDrive Authname not provided";
>-
>+warn "Calling";
>+warn "$userid, $password, $website_id, $authorization_name";
>+warn $self->client->auth_by_user_id($userid, $password, $website_id, $authorization_name);
>     my ($access_token, $access_token_type, $auth_token)
>       = $self->client->auth_by_user_id($userid, $password, $website_id, $authorization_name);
>+    warn "$access_token, $access_token_type, $auth_token";
>     $access_token or die "Invalid OverDrive code returned";
>     $self->set_token_in_koha_session($access_token, $access_token_type);
> 
>diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
>index 1adb32d..fb6a0cd 100644
>--- a/Koha/Patrons/Import.pm
>+++ b/Koha/Patrons/Import.pm
>@@ -207,6 +207,7 @@ sub import_patrons {
>             and exists $borrower{userid}
>             and $borrower{userid}
>             and not Koha::Patron->new( { userid => $borrower{userid} } )->has_valid_userid
>+            and not $borrowernumber
>         ) {
>             push @errors, { duplicate_userid => 1, userid => $borrower{userid} };
>             $invalid++;
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index 6521db1..886e164 100644
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -1285,7 +1285,7 @@ subtest 'get_overdues' => sub {
> };
> 
> subtest 'userid_is_valid' => sub {
>-    plan tests => 8;
>+    plan tests => 9;
> 
>     my $library = $builder->build_object( { class => 'Koha::Libraries' } );
>     my $patron_category = $builder->build_object(
>@@ -1305,6 +1305,7 @@ subtest 'userid_is_valid' => sub {
>     my $expected_userid_patron_1 = 'tomasito.none';
>     my $borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber;
>     my $patron_1       = Koha::Patrons->find($borrowernumber);
>+    is( $patron_1->has_valid_userid, 1, "Should be valid when compared against them self" );
>     is ( $patron_1->userid, $expected_userid_patron_1, 'The userid generated should be the one we expect' );
> 
>     $patron_1->userid( 'tomasito.non' );
>diff --git a/t/db_dependent/Koha/Patrons/Import.t b/t/db_dependent/Koha/Patrons/Import.t
>index b55a5af..cf4ee0e 100644
>--- a/t/db_dependent/Koha/Patrons/Import.t
>+++ b/t/db_dependent/Koha/Patrons/Import.t
>@@ -18,7 +18,7 @@
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use Test::More tests => 124;
>+use Test::More tests => 142;
> use Test::Warn;
> 
> # To be replaced by t::lib::Mock
>@@ -89,6 +89,7 @@ t::lib::Mocks::mock_preference('dateformat', 'us');
> my $csv_headers  = 'cardnumber,surname,firstname,title,othernames,initials,streetnumber,streettype,address,address2,city,state,zipcode,country,email,phone,mobile,fax,dateofbirth,branchcode,categorycode,dateenrolled,dateexpiry,userid,password';
> my $res_header   = 'cardnumber, surname, firstname, title, othernames, initials, streetnumber, streettype, address, address2, city, state, zipcode, country, email, phone, mobile, fax, dateofbirth, branchcode, categorycode, dateenrolled, dateexpiry, userid, password';
> my $csv_one_line = '1000,Nancy,Jenkins,Dr,,NJ,78,Circle,Bunting,El Paso,Henderson,Texas,79984,United States,ajenkins0@sourceforge.net,7-(388)559-6763,3-(373)151-4471,8-(509)286-4001,10/16/1965,CPL,PT,12/28/2014,07/01/2015,jjenkins0,DPQILy';
>+my $csv_one_line_a = '1001,Nancy,Jenkins,Dr,,NJ,78,Circle,Bunting,El Paso,Henderson,Texas,79984,United States,ajenkins0@sourceforge.net,7-(388)559-6763,3-(373)151-4471,8-(509)286-4001,10/16/1965,CPL,PT,12/28/2014,07/01/2015,jjenkins0,DPQILy';
> 
> my $filename_1 = make_csv($temp_dir, $csv_headers, $csv_one_line);
> open(my $handle_1, "<", $filename_1) or die "cannot open < $filename_1: $!";
>@@ -135,7 +136,7 @@ is($result_2->{imported}, 0, 'Got the expected 0 imported result from import_pat
> is($result_2->{invalid}, 1, 'Got the expected 1 invalid result from import_patrons with invalid card number');
> is($result_2->{overwritten}, 0, 'Got the expected 0 overwritten result from import_patrons with invalid card number');
> 
>-# Given ... valid file handle, good matchpoint but same input as prior test.
>+# Given ... valid file handle, good matchpoint that matches should not overwrite when not set.
> my $filename_3 = make_csv($temp_dir, $csv_headers, $csv_one_line);
> open(my $handle_3, "<", $filename_3) or die "cannot open < $filename_3: $!";
> my $params_3 = { file => $handle_3, matchpoint => 'cardnumber', };
>@@ -144,17 +145,59 @@ my $params_3 = { file => $handle_3, matchpoint => 'cardnumber', };
> my $result_3 = $patrons_import->import_patrons($params_3);
> 
> # Then ...
>-is($result_3->{already_in_db}, 0, 'Got the expected 0 already_in_db from import_patrons with duplicate userid');
>-is($result_3->{errors}->[0]->{duplicate_userid}, 1, 'Got the expected duplicate userid error from import patrons with duplicate userid');
>-is($result_3->{errors}->[0]->{userid}, 'jjenkins0', 'Got the expected userid error from import patrons with duplicate userid');
>+is($result_3->{already_in_db}, 1, 'Got the expected 1 already_in_db from import_patrons with duplicate userid');
>+is($result_3->{errors}->[0]->{duplicate_userid}, undef, 'No duplicate userid error from import patrons with duplicate userid (it is our own)');
>+is($result_3->{errors}->[0]->{userid}, undef, 'No duplicate userid error from import patrons with duplicate userid (it is our own)');
> 
>-is($result_3->{feedback}->[0]->{feedback}, 1, 'Got the expected 1 feedback from import_patrons with duplicate userid');
>+is($result_3->{feedback}->[0]->{feedback}, 1, 'Got 1 expected feedback from import_patrons that matched but not overwritten');
> is($result_3->{feedback}->[0]->{name}, 'headerrow', 'Got the expected header row name from import_patrons with duplicate userid');
> is($result_3->{feedback}->[0]->{value}, $res_header, 'Got the expected header row value from import_patrons with duplicate userid');
> 
>-is($result_3->{imported}, 0, 'Got the expected 0 imported result from import_patrons with duplicate userid');
>-is($result_3->{invalid}, 1, 'Got the expected 1 invalid result from import_patrons with duplicate userid');
>-is($result_3->{overwritten}, 0, 'Got the expected 0 overwritten result from import_patrons with duplicate userid');
>+is($result_3->{imported}, 0, 'Got the expected 0 imported result from import_patrons');
>+is($result_3->{invalid}, 0, 'Got the expected 0 invalid result from import_patrons');
>+is($result_3->{overwritten}, 0, 'Got the expected 0 overwritten result from import_patrons that matched');
>+
>+# Given ... valid file handle, good matchpoint that matches should overwrite when set.
>+my $filename_3a = make_csv($temp_dir, $csv_headers, $csv_one_line);
>+open(my $handle_3a, "<", $filename_3a) or die "cannot open < $filename_3: $!";
>+my $params_3a = { file => $handle_3a, matchpoint => 'cardnumber', overwrite_cardnumber => 1};
>+
>+# When ...
>+my $result_3a = $patrons_import->import_patrons($params_3a);
>+
>+# Then ...
>+is($result_3a->{already_in_db}, 0, 'Got the expected 0 already_in_db from import_patrons when matched and overwrite set');
>+is($result_3a->{errors}->[0]->{duplicate_userid}, undef, 'No duplicate userid error from import patrons with duplicate userid (it is our own)');
>+is($result_3a->{errors}->[0]->{userid}, undef, 'No duplicate userid error from import patrons with duplicate userid (it is our own)');
>+
>+is($result_3a->{feedback}->[0]->{feedback}, 1, 'Got 1 expected feedback from import_patrons that matched and overwritten');
>+is($result_3a->{feedback}->[0]->{name}, 'headerrow', 'Got the expected header row name from import_patrons with duplicate userid');
>+is($result_3a->{feedback}->[0]->{value}, $res_header, 'Got the expected header row value from import_patrons with duplicate userid');
>+
>+is($result_3a->{imported}, 0, 'Got the expected 0 imported result from import_patrons');
>+is($result_3a->{invalid}, 0, 'Got the expected 0 invalid result from import_patrons');
>+is($result_3a->{overwritten}, 1, 'Got the expected 1 overwritten result from import_patrons that matched');
>+
>+# Given ... valid file handle, good matchpoint that does not match and conflicting userid.
>+my $filename_3b = make_csv($temp_dir, $csv_headers, $csv_one_line_a);
>+open(my $handle_3b, "<", $filename_3b) or die "cannot open < $filename_3: $!";
>+my $params_3b = { file => $handle_3b, matchpoint => 'cardnumber', };
>+
>+# When ...
>+my $result_3b = $patrons_import->import_patrons($params_3b);
>+
>+# Then ...
>+is($result_3b->{already_in_db}, 0, 'Got the expected 0 already_in_db from import_patrons with duplicate userid');
>+is($result_3b->{errors}->[0]->{duplicate_userid}, 1, 'Got the expected duplicate userid error from import patrons with duplicate userid');
>+is($result_3b->{errors}->[0]->{userid}, 'jjenkins0', 'Got the expected userid error from import patrons with duplicate userid');
>+
>+is($result_3b->{feedback}->[0]->{feedback}, 1, 'Got the expected 1 feedback from import_patrons with duplicate userid');
>+is($result_3b->{feedback}->[0]->{name}, 'headerrow', 'Got the expected header row name from import_patrons with duplicate userid');
>+is($result_3b->{feedback}->[0]->{value}, $res_header, 'Got the expected header row value from import_patrons with duplicate userid');
>+
>+is($result_3b->{imported}, 0, 'Got the expected 0 imported result from import_patrons with duplicate userid');
>+is($result_3b->{invalid}, 1, 'Got the expected 1 invalid result from import_patrons with duplicate userid');
>+is($result_3b->{overwritten}, 0, 'Got the expected 0 overwritten result from import_patrons with duplicate userid');
> 
> # Given ... a new input and mocked C4::Context
> t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 1);
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 21465: 79850 | 79908 | 82877 | 83033