From 43697d5007226a4cb114631b7ad89168c645c1cc Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 23 Feb 2018 12:42:15 -0300 Subject: [PATCH] Bug 20287: Move ModMember to Koha::Patron Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- C4/Members.pm | 108 --------------------------------------- Koha/Patron.pm | 108 ++++++++++++++++++++++++++++++++------- members/memberentry.pl | 2 +- members/members-update-do.pl | 9 ++-- members/update-child.pl | 14 ++--- misc/cronjobs/nl-sync-to-koha.pl | 14 ++--- opac/opac-messaging.pl | 25 +++++---- opac/opac-privacy.pl | 15 +++--- t/db_dependent/Koha/Patrons.t | 16 +++--- t/db_dependent/Letters.t | 2 +- t/db_dependent/Members.t | 54 ++++++++++---------- t/db_dependent/Reserves.t | 4 +- tools/modborrowers.pl | 4 +- 13 files changed, 163 insertions(+), 212 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 080146eb11..dc78638123 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -72,7 +72,6 @@ BEGIN { #Modify data push @EXPORT, qw( - &ModMember &changepassword ); @@ -261,113 +260,6 @@ sub patronflags { return ( \%flags ); } - -=head2 ModMember - - my $success = ModMember(borrowernumber => $borrowernumber, - [ field => value ]... ); - -Modify borrower's data. All date fields should ALREADY be in ISO format. - -return : -true on success, or false on failure - -=cut - -sub ModMember { - my (%data) = @_; - - # trim whitespace from data which has some non-whitespace in it. - foreach my $field_name (keys(%data)) { - if ( defined $data{$field_name} && $data{$field_name} =~ /\S/ ) { - $data{$field_name} =~ s/^\s*|\s*$//g; - } - } - - # test to know if you must update or not the borrower password - if (exists $data{password}) { - if ($data{password} eq '****' or $data{password} eq '') { - delete $data{password}; - } else { - if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) { - # Update the hashed PIN in borrower_sync.hashed_pin, before Koha hashes it - Koha::NorwegianPatronDB::NLUpdateHashedPIN( $data{'borrowernumber'}, $data{password} ); - } - $data{password} = hash_password($data{password}); - } - } - - my $old_categorycode = Koha::Patrons->find( $data{borrowernumber} )->categorycode; - - # 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) }; - - $new_borrower->{dateofbirth} ||= undef if exists $new_borrower->{dateofbirth}; - $new_borrower->{dateenrolled} ||= undef if exists $new_borrower->{dateenrolled}; - $new_borrower->{dateexpiry} ||= undef if exists $new_borrower->{dateexpiry}; - $new_borrower->{debarred} ||= undef if exists $new_borrower->{debarred}; - $new_borrower->{sms_provider_id} ||= undef if exists $new_borrower->{sms_provider_id}; - $new_borrower->{guarantorid} ||= undef if exists $new_borrower->{guarantorid}; - - my $patron = Koha::Patrons->find( $new_borrower->{borrowernumber} ); - - my $borrowers_log = C4::Context->preference("BorrowersLog"); - if ( $borrowers_log && $patron->cardnumber ne $new_borrower->{cardnumber} ) - { - logaction( - "MEMBERS", - "MODIFY", - $data{'borrowernumber'}, - to_json( - { - cardnumber_replaced => { - previous_cardnumber => $patron->cardnumber, - new_cardnumber => $new_borrower->{cardnumber}, - } - }, - { utf8 => 1, pretty => 1 } - ) - ); - } - - delete $new_borrower->{userid} if exists $new_borrower->{userid} and not $new_borrower->{userid}; - - my $execute_success = $patron->store if $patron->set($new_borrower); - - if ($execute_success) { # only proceed if the update was a success - # If the patron changes to a category with enrollment fee, we add a fee - if ( $data{categorycode} and $data{categorycode} ne $old_categorycode ) { - if ( C4::Context->preference('FeeOnChangePatronCategory') ) { - $patron->add_enrolment_fee_if_needed; - } - } - - # If NorwegianPatronDBEnable is enabled, we set syncstatus to something that a - # cronjob will use for syncing with NL - if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) { - my $borrowersync = Koha::Database->new->schema->resultset('BorrowerSync')->find({ - 'synctype' => 'norwegianpatrondb', - 'borrowernumber' => $data{'borrowernumber'} - }); - # Do not set to "edited" if syncstatus is "new". We need to sync as new before - # we can sync as changed. And the "new sync" will pick up all changes since - # the patron was created anyway. - if ( $borrowersync->syncstatus ne 'new' && $borrowersync->syncstatus ne 'delete' ) { - $borrowersync->update( { 'syncstatus' => 'edited' } ); - } - # Set the value of 'sync' - $borrowersync->update( { 'sync' => $data{'sync'} } ); - # Try to do the live sync - Koha::NorwegianPatronDB::NLSync({ 'borrowernumber' => $data{'borrowernumber'} }); - } - - logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ arg: $data{'borrowernumber'})") if $borrowers_log; - } - return $execute_success; -} - =head2 GetAllIssues $issues = &GetAllIssues($borrowernumber, $sortkey, $limit); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fa0b53a55b..ca07ad80b2 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -22,6 +22,7 @@ use Modern::Perl; use Carp; use List::MoreUtils qw( uniq ); +use JSON qw( to_json ); use Module::Load::Conditional qw( can_load ); use Text::Unaccent qw( unac_string ); @@ -155,16 +156,25 @@ sub store { # We are in a transaction but the table is not locked $self->fixup_cardnumber; } - unless ( $self->in_storage ) { #AddMember - unless( $self->category->in_storage ) { - Koha::Exceptions::Object::FKConstraint->throw( - broken_fk => 'categorycode', - value => $self->categorycode, - ); - } + unless( $self->category->in_storage ) { + Koha::Exceptions::Object::FKConstraint->throw( + broken_fk => 'categorycode', + value => $self->categorycode, + ); + } + + $self->trim_whitespaces; - $self->trim_whitespaces; + # We don't want invalid dates in the db (mysql has a bad habit of inserting 0000-00-00) + $self->dateofbirth(undef) unless $self->dateofbirth; + $self->debarred(undef) unless $self->debarred; + + # Set default values if not set + $self->sms_provider_id(undef) unless $self->sms_provider_id; + $self->guarantorid(undef) unless $self->guarantorid; + + unless ( $self->in_storage ) { #AddMember # Generate a valid userid/login if needed $self->userid($self->generate_userid) @@ -201,14 +211,6 @@ sub store { ? Koha::AuthUtils::hash_password( $self->password ) : '!' ); - # We don't want invalid dates in the db (mysql has a bad habit of inserting 0000-00-00) - $self->dateofbirth(undef) unless $self->dateofbirth; - $self->debarred(undef) unless $self->debarred; - - # Set default values if not set - $self->sms_provider_id(undef) unless $self->sms_provider_id; - $self->guarantorid(undef) unless $self->guarantorid; - $self->borrowernumber(undef); $self = $self->SUPER::store; @@ -237,9 +239,78 @@ sub store { if C4::Context->preference("BorrowersLog"); } else { #ModMember + # test to know if you must update or not the borrower password + if ( defined $self->password ) { + if ( $self->password eq '****' or $self->password eq '' ) { + $self->password(undef); + } else { + if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) { + # Update the hashed PIN in borrower_sync.hashed_pin, before Koha hashes it + Koha::NorwegianPatronDB::NLUpdateHashedPIN( $self->borrowernumber, $self->password ); + } + $self->password(Koha::AuthUtils::hash_password($self->password)); + } + } + + # Come from ModMember, but should not be possible (?) + $self->dateenrolled(undef) unless $self->dateenrolled; + $self->dateexpiry(undef) unless $self->dateexpiry; + + if ( C4::Context->preference('FeeOnChangePatronCategory') + and $self->category->categorycode ne + $self->get_from_storage->category->categorycode ) + { + $self->add_enrolment_fee_if_needed; + } + + # If NorwegianPatronDBEnable is enabled, we set syncstatus to something that a + # cronjob will use for syncing with NL + if ( C4::Context->preference('NorwegianPatronDBEnable') + && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) + { + my $borrowersync = Koha::Database->new->schema->resultset('BorrowerSync')->find({ + 'synctype' => 'norwegianpatrondb', + 'borrowernumber' => $self->borrowernumber, + }); + # Do not set to "edited" if syncstatus is "new". We need to sync as new before + # we can sync as changed. And the "new sync" will pick up all changes since + # the patron was created anyway. + if ( $borrowersync->syncstatus ne 'new' && $borrowersync->syncstatus ne 'delete' ) { + $borrowersync->update( { 'syncstatus' => 'edited' } ); + } + # Set the value of 'sync' + # FIXME THIS IS BROKEN # $borrowersync->update( { 'sync' => $data{'sync'} } ); + + # Try to do the live sync + Koha::NorwegianPatronDB::NLSync({ 'borrowernumber' => $self->borrowernumber }); + } + + my $borrowers_log = C4::Context->preference("BorrowersLog"); + my $previous_cardnumber = $self->get_from_storage->cardnumber; + if ( $borrowers_log && $previous_cardnumber ne $self->cardnumber ) + { + logaction( + "MEMBERS", + "MODIFY", + $self->borrowernumber, + to_json( + { + cardnumber_replaced => { + previous_cardnumber => $previous_cardnumber, + new_cardnumber => $self->cardnumber, + } + }, + { utf8 => 1, pretty => 1 } + ) + ); + } + + logaction( "MEMBERS", "MODIFY", $self->borrowernumber, + "UPDATE (executed w/ arg: " . $self->borrowernumber . ")" ) + if $borrowers_log; + $self = $self->SUPER::store; } - } ); return $self; @@ -1162,8 +1233,7 @@ Generate a userid using the $surname and the $firstname (if there is a value in Return the generate userid ($firstname.$surname if there is a $firstname, or $surname if there is no value in $firstname) plus offset (0 if the $userid is unique, or a higher numeric value if not unique). -# Note: Should we set $self->userid with the generated value? -# Certainly yes, but we AddMember and ModMember will be rewritten +# TODO: Set $self->userid with the generated value =cut diff --git a/members/memberentry.pl b/members/memberentry.pl index e97f085c94..26853b042e 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -202,7 +202,7 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' ) } } -# remove keys from %newdata that ModMember() doesn't like +# remove keys from %newdata that is not part of patron's attributes { my @keys_to_delete = ( qr/^BorrowerMandatoryField$/, diff --git a/members/members-update-do.pl b/members/members-update-do.pl index 2f64096941..cc29dbe6ad 100755 --- a/members/members-update-do.pl +++ b/members/members-update-do.pl @@ -22,7 +22,7 @@ use CGI qw ( -utf8 ); use C4::Auth; use C4::Output; use C4::Context; -use C4::Members; +use Koha::Patrons; use Koha::Patron::Modifications; my $query = new CGI; @@ -57,10 +57,9 @@ foreach my $param (@params) { if ($query->param("unset_gna_$borrowernumber")) { # Unset gone no address - ModMember( - borrowernumber => $borrowernumber, - gonenoaddress => undef - ); + # FIXME Looks like this could go to $m->approve + my $patron = Koha::Patrons->find( $borrowernumber ); + $patron->gonenoaddress(undef)->store; } $m->approve() if $m; diff --git a/members/update-child.pl b/members/update-child.pl index cc942e2704..77f73f634a 100755 --- a/members/update-child.pl +++ b/members/update-child.pl @@ -22,7 +22,7 @@ script to update a child member to (usually) an adult member category - if called with op=multi, will return all available non child categories, for selection. - - if called with op=update, script will update member record via ModMember(). + - if called with op=update, script will update member record via Koha::Patron->store. =cut @@ -31,7 +31,6 @@ use CGI qw ( -utf8 ); use C4::Context; use C4::Auth; use C4::Output; -use C4::Members; use Koha::Patrons; use Koha::Patron::Categories; @@ -76,14 +75,9 @@ elsif ( $op eq 'update' ) { my $patron = Koha::Patrons->find( $borrowernumber ); output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); - my $member = $patron->unblessed; - $member->{'guarantorid'} = 0; - $member->{'categorycode'} = $catcode; - my $borcat = Koha::Patron::Categories->find($catcode); - $member->{'category_type'} = $borcat->category_type; - $member->{'description'} = $borcat->description; - delete $member->{password}; - ModMember(%$member); + $patron->guarantorid(undef); + $patron->categorycode($catcode); + $patron->store; if ( $catcode_multi ) { $template->param( diff --git a/misc/cronjobs/nl-sync-to-koha.pl b/misc/cronjobs/nl-sync-to-koha.pl index 84e23788b5..ce25c7e142 100755 --- a/misc/cronjobs/nl-sync-to-koha.pl +++ b/misc/cronjobs/nl-sync-to-koha.pl @@ -12,9 +12,9 @@ nl-sync-to-koha.pl - Sync patrons from the Norwegian national patron database (N =cut -use C4::Members; use C4::Members::Attributes qw( UpdateBorrowerAttribute ); use Koha::NorwegianPatronDB qw( NLCheckSysprefs NLGetChanged ); +use Koha::Patrons; use Koha::Database; use Getopt::Long; use Pod::Usage; @@ -70,17 +70,11 @@ foreach my $patron ( @{ $result->{'kohapatrons'} } ) { # Delete the extra data from the copy of the hashref delete $clean_patron{'_extra'}; # Find the borrowernumber based on cardnumber - my $stored_patron = Koha::Database->new->schema->resultset('Borrower')->find({ - 'cardnumber' => $patron->{'cardnumber'} - }); + my $stored_patron = Koha::Patrons->find({ cardnumber => $patron->{cardnumber} }); my $borrowernumber = $stored_patron->borrowernumber; if ( $run ) { - # Call ModMember - my $success = ModMember( - 'borrowernumber' => $borrowernumber, - %clean_patron, - ); - if ( $success ) { + # FIXME Exceptions must be caught here + if ( $stored_patron->set(\%clean_patron)->store ) { # Get the sync object my $sync = Koha::Database->new->schema->resultset('BorrowerSync')->find({ 'synctype' => 'norwegianpatrondb', diff --git a/opac/opac-messaging.pl b/opac/opac-messaging.pl index 906d56fe4e..d2c403311f 100755 --- a/opac/opac-messaging.pl +++ b/opac/opac-messaging.pl @@ -30,6 +30,7 @@ use C4::Output; use C4::Members; use C4::Members::Messaging; use C4::Form::MessagingPreferences; +use Koha::Patrons; use Koha::SMS::Providers; my $query = CGI->new(); @@ -50,37 +51,35 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +my $patron = Koha::Patrons->find( $borrowernumber ); # FIXME and if borrowernumber is invalid? + my $messaging_options = C4::Members::Messaging::GetMessagingOptions(); if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { my $sms = $query->param('SMSnumber'); my $sms_provider_id = $query->param('sms_provider_id'); - if ( defined $sms && ( $borrower->{'smsalertnumber'} // '' ) ne $sms - or ( $borrower->{sms_provider_id} // '' ) ne $sms_provider_id ) { - ModMember( - borrowernumber => $borrowernumber, + if ( defined $sms && ( $patron->smsalertnumber // '' ) ne $sms + or ( $patron->sms_provider_id // '' ) ne $sms_provider_id ) { + $patron->set({ smsalertnumber => $sms, sms_provider_id => $sms_provider_id, - ); - # FIXME will not be needed when ModMember will be replaced - $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; + })->store; } - C4::Form::MessagingPreferences::handle_form_action($query, { borrowernumber => $borrowernumber }, $template); + C4::Form::MessagingPreferences::handle_form_action($query, { borrowernumber => $patron->borrowernumber }, $template); } -C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $borrower->{'borrowernumber'} }, $template); +C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $patron->borrowernumber }, $template); -$template->param( BORROWER_INFO => $borrower, +$template->param( BORROWER_INFO => $patron->unblessed, messagingview => 1, - SMSnumber => $borrower->{'smsalertnumber'}, + SMSnumber => $patron->smsalertnumber, # FIXME This is already sent 2 lines above SMSSendDriver => C4::Context->preference("SMSSendDriver"), TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification") ); if ( C4::Context->preference("SMSSendDriver") eq 'Email' ) { my @providers = Koha::SMS::Providers->search(); - $template->param( sms_providers => \@providers, sms_provider_id => $borrower->{'sms_provider_id'} ); + $template->param( sms_providers => \@providers, sms_provider_id => $patron->sms_provider_id ); } output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; diff --git a/opac/opac-privacy.pl b/opac/opac-privacy.pl index 8796a7a8dd..50f9a23e6c 100755 --- a/opac/opac-privacy.pl +++ b/opac/opac-privacy.pl @@ -21,7 +21,6 @@ use CGI qw ( -utf8 ); use C4::Auth; # checkauth, getborrowernumber. use C4::Context; -use C4::Members; use C4::Output; use Koha::Patrons; @@ -50,12 +49,14 @@ my $privacy = $query->param("privacy"); my $privacy_guarantor_checkouts = $query->param("privacy_guarantor_checkouts"); if ( $op eq "update_privacy" ) { - ModMember( - borrowernumber => $borrowernumber, - privacy => $privacy, - privacy_guarantor_checkouts => $privacy_guarantor_checkouts, - ); - $template->param( 'privacy_updated' => 1 ); + my $patron = Koha::Patrons->find( $borrowernumber ); + if ( $patron ) { + $patron->set({ + privacy => $privacy, + privacy_guarantor_checkouts => $privacy_guarantor_checkouts, + })->store; + $template->param( 'privacy_updated' => 1 ); + } } elsif ( $op eq "delete_record" ) { diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index d7153b0d32..3908c5b7ff 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -402,16 +402,16 @@ subtest 'add_enrolment_fee_if_needed' => sub { t::lib::Mocks::mock_preference( 'FeeOnChangePatronCategory', 0 ); $borrower_data{categorycode} = 'J'; - C4::Members::ModMember(%borrower_data); + $patron->set(\%borrower_data)->store; $total = $patron->account->balance; is( int($total), int($enrolmentfee_K), "Kid growing and become a juvenile, but shouldn't pay for the upgrade " ); $borrower_data{categorycode} = 'K'; - C4::Members::ModMember(%borrower_data); + $patron->set(\%borrower_data)->store; t::lib::Mocks::mock_preference( 'FeeOnChangePatronCategory', 1 ); $borrower_data{categorycode} = 'J'; - C4::Members::ModMember(%borrower_data); + $patron->set(\%borrower_data)->store; $total = $patron->account->balance; is( int($total), int($enrolmentfee_K + $enrolmentfee_J), "Kid growing and become a juvenile, they should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) ); @@ -1362,13 +1362,13 @@ subtest 'Log cardnumber change' => sub { plan tests => 3; t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); - my $patron = $builder->build( { source => 'Borrower' } ); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $cardnumber = $patron->{cardnumber}; - $patron->{cardnumber} = 'TESTCARDNUMBER'; - ModMember(%$patron); + my $cardnumber = $patron->cardnumber; + $patron->set( { cardnumber => 'TESTCARDNUMBER' }); + $patron->store; - my @logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->{borrowernumber} } ); + my @logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); my $log_info = from_json( $logs[0]->info ); is( $log_info->{cardnumber_replaced}->{new_cardnumber}, 'TESTCARDNUMBER', 'Got correct new cardnumber' ); is( $log_info->{cardnumber_replaced}->{previous_cardnumber}, $cardnumber, 'Got correct old cardnumber' ); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 5a60e1470e..344473c829 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -610,7 +610,7 @@ subtest 'SendQueuedMessages' => sub { is( $@, '', 'SendQueuedMessages should not explode if the patron does not have a sms provider set' ); my $sms_pro = $builder->build_object({ class => 'Koha::SMS::Providers', value => { domain => 'kidclamp.rocks' } }); - ModMember( borrowernumber => $borrowernumber, smsalertnumber => '5555555555', sms_provider_id => $sms_pro->id() ); + $patron->set( { smsalertnumber => '5555555555', sms_provider_id => $sms_pro->id() } )->store; $message_id = C4::Letters::EnqueueLetter($my_message); #using datas set around line 95 and forward C4::Letters::SendQueuedMessages(); my $sms_message_address = $schema->resultset('MessageQueue')->search({ diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index bc0d04902d..71cdec6f48 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -97,9 +97,9 @@ my %data = ( my $addmem=Koha::Patron->new(\%data)->store->borrowernumber; ok($addmem, "Koha::Patron->store()"); -my $member = Koha::Patrons->find( { cardnumber => $CARDNUMBER } ) +my $patron = Koha::Patrons->find( { cardnumber => $CARDNUMBER } ) or BAIL_OUT("Cannot read member with card $CARDNUMBER"); -$member = $member->unblessed; +my $member = $patron->unblessed; ok ( $member->{firstname} eq $FIRSTNAME && $member->{surname} eq $SURNAME && @@ -114,7 +114,7 @@ $member->{firstname} = $CHANGED_FIRSTNAME . q{ }; $member->{email} = $EMAIL; $member->{phone} = $PHONE; $member->{emailpro} = $EMAILPRO; -ModMember(%$member); +$patron->set($member)->store; my $changedmember = Koha::Patrons->find( { cardnumber => $CARDNUMBER } )->unblessed; ok ( $changedmember->{firstname} eq $CHANGED_FIRSTNAME && $changedmember->{email} eq $EMAIL && @@ -152,43 +152,45 @@ is ($checkcardnum, "2", "Card number is too long"); dateenrolled => '', ); my $borrowernumber = Koha::Patron->new( \%data )->store->borrowernumber; -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +$patron = Koha::Patrons->find( $borrowernumber ); +my $borrower = $patron->unblessed; is( $borrower->{dateofbirth}, undef, 'Koha::Patron->store should undef dateofbirth if empty string is given'); is( $borrower->{debarred}, undef, 'Koha::Patron->store should undef debarred if empty string is given'); isnt( $borrower->{dateexpiry}, '0000-00-00', 'Koha::Patron->store should not set dateexpiry to 0000-00-00 if empty string is given'); isnt( $borrower->{dateenrolled}, '0000-00-00', 'Koha::Patron->store should not set dateenrolled to 0000-00-00 if empty string is given'); -ModMember( borrowernumber => $borrowernumber, dateofbirth => '', debarred => '', dateexpiry => '', dateenrolled => '' ); +$patron->set({ dateofbirth => '', debarred => '', dateexpiry => '', dateenrolled => '' })->store; $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; -is( $borrower->{dateofbirth}, undef, 'ModMember should undef dateofbirth if empty string is given'); -is( $borrower->{debarred}, undef, 'ModMember should undef debarred if empty string is given'); -isnt( $borrower->{dateexpiry}, '0000-00-00', 'ModMember should not set dateexpiry to 0000-00-00 if empty string is given'); -isnt( $borrower->{dateenrolled}, '0000-00-00', 'ModMember should not set dateenrolled to 0000-00-00 if empty string is given'); +is( $borrower->{dateofbirth}, undef, 'Koha::Patron->store should undef dateofbirth if empty string is given'); +is( $borrower->{debarred}, undef, 'Koha::Patron->store should undef debarred if empty string is given'); +isnt( $borrower->{dateexpiry}, '0000-00-00', 'Koha::Patron->store should not set dateexpiry to 0000-00-00 if empty string is given'); +isnt( $borrower->{dateenrolled}, '0000-00-00', 'Koha::Patron->store should not set dateenrolled to 0000-00-00 if empty string is given'); -ModMember( borrowernumber => $borrowernumber, dateofbirth => '1970-01-01', debarred => '2042-01-01', dateexpiry => '9999-12-31', dateenrolled => '2015-09-06' ); +$patron->set({ dateofbirth => '1970-01-01', debarred => '2042-01-01', dateexpiry => '9999-12-31', dateenrolled => '2015-09-06' })->store; $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; -is( $borrower->{dateofbirth}, '1970-01-01', 'ModMember should correctly set dateofbirth if a valid date is given'); -is( $borrower->{debarred}, '2042-01-01', 'ModMember should correctly set debarred if a valid date is given'); -is( $borrower->{dateexpiry}, '9999-12-31', 'ModMember should correctly set dateexpiry if a valid date is given'); -is( $borrower->{dateenrolled}, '2015-09-06', 'ModMember should correctly set dateenrolled if a valid date is given'); +is( $borrower->{dateofbirth}, '1970-01-01', 'Koha::Patron->store should correctly set dateofbirth if a valid date is given'); +is( $borrower->{debarred}, '2042-01-01', 'Koha::Patron->store should correctly set debarred if a valid date is given'); +is( $borrower->{dateexpiry}, '9999-12-31', 'Koha::Patron->store should correctly set dateexpiry if a valid date is given'); +is( $borrower->{dateenrolled}, '2015-09-06', 'Koha::Patron->store should correctly set dateenrolled if a valid date is given'); -subtest 'ModMember should not update userid if not true' => sub { +subtest 'Koha::Patron->store should not update userid if not true' => sub { plan tests => 3; $data{ cardnumber } = "234567890"; $data{userid} = 'a_user_id'; $borrowernumber = Koha::Patron->new( \%data )->store->borrowernumber; - $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; + my $patron = Koha::Patrons->find( $borrowernumber ); + my $borrower = $patron->unblessed; - ModMember( borrowernumber => $borrowernumber, firstname => 'Tomas', userid => '' ); + $patron->set( { firstname => 'Tomas', userid => '' } )->store; $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - is ( $borrower->{userid}, $data{userid}, 'ModMember should not update the userid with an empty string' ); - ModMember( borrowernumber => $borrowernumber, firstname => 'Tomas', userid => 0 ); + is ( $borrower->{userid}, $data{userid}, 'Koha::Patron->store should not update the userid with an empty string' ); + $patron->set( { firstname => 'Tomas', userid => 0 } )->store; $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - is ( $borrower->{userid}, $data{userid}, 'ModMember should not update the userid with an 0'); - ModMember( borrowernumber => $borrowernumber, firstname => 'Tomas', userid => undef ); + is ( $borrower->{userid}, $data{userid}, 'Koha::Patron->store should not update the userid with an 0'); + $patron->set( { firstname => 'Tomas', userid => undef } )->store; $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - is ( $borrower->{userid}, $data{userid}, 'ModMember should not update the userid with an undefined value'); + is ( $borrower->{userid}, $data{userid}, 'Koha::Patron->store should not update the userid with an undefined value'); }; #Regression tests for bug 10612 @@ -272,7 +274,7 @@ my @listpatrons = ($bor1inlist, $bor2inlist); AddPatronsToList( { list => $list1, borrowernumbers => \@listpatrons }); my $patstodel = GetBorrowersToExpunge( {patron_list_id => $list1->patron_list_id() } ); is( scalar(@$patstodel),0,'No staff deleted from list of all staff'); -ModMember( borrowernumber => $bor2inlist, categorycode => 'CIVILIAN' ); +Koha::Patrons->find($bor2inlist)->set({ categorycode => 'CIVILIAN' })->store; $patstodel = GetBorrowersToExpunge( {patron_list_id => $list1->patron_list_id()} ); ok( scalar(@$patstodel)== 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Staff patron not deleted from list'); $patstodel = GetBorrowersToExpunge( {branchcode => $library3->{branchcode},patron_list_id => $list1->patron_list_id() } ); @@ -282,8 +284,8 @@ ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inli $patstodel = GetBorrowersToExpunge( {not_borrowed_since => '2016-01-02', patron_list_id => $list1->patron_list_id() } ); ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Staff patron not deleted by last issue date'); -ModMember( borrowernumber => $bor1inlist, categorycode => 'CIVILIAN' ); -ModMember( borrowernumber => $guarantee->{borrowernumber} ,guarantorid=>$bor1inlist ); +Koha::Patrons->find($bor1inlist)->set({ categorycode => 'CIVILIAN' })->store; +Koha::Patrons->find($guarantee->{borrowernumber})->set({ guarantorid => $bor1inlist })->store; $patstodel = GetBorrowersToExpunge( {patron_list_id => $list1->patron_list_id()} ); ok( scalar(@$patstodel)== 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted from list'); @@ -293,7 +295,7 @@ $patstodel = GetBorrowersToExpunge( {expired_before => '2015-01-02', patron_list ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted by expirationdate and list'); $patstodel = GetBorrowersToExpunge( {not_borrowed_since => '2016-01-02', patron_list_id => $list1->patron_list_id() } ); ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted by last issue date'); -ModMember( borrowernumber => $guarantee->{borrowernumber}, guarantorid=>'' ); +Koha::Patrons->find($guarantee->{borrowernumber})->set({ guarantorid => '' })->store; $builder->build({ source => 'Issue', diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index af5f064865..0b12de6de9 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -526,13 +526,13 @@ is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Res #Set the dateofbirth for the Borrower making them "too young". $borrower->{dateofbirth} = DateTime->now->add( years => -15 ); -C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $borrower->{dateofbirth} ); +Koha::Patrons->find( $borrowernumber )->set({ dateofbirth => $borrower->{dateofbirth} })->store; is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'ageRestricted', "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); #Set the dateofbirth for the Borrower making them "too old". $borrower->{dateofbirth} = DateTime->now->add( years => -30 ); -C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $borrower->{dateofbirth} ); +Koha::Patrons->find( $borrowernumber )->set({ dateofbirth => $borrower->{dateofbirth} })->store; is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); #### diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl index e53e3a30d7..7b8587ad04 100755 --- a/tools/modborrowers.pl +++ b/tools/modborrowers.pl @@ -295,8 +295,8 @@ if ( $op eq 'do' ) { # If at least one field are filled, we want to modify the borrower if ( defined $infos ) { $infos->{borrowernumber} = $borrowernumber; - my $success = ModMember(%$infos); - if (!$success) { + eval { Koha::Patrons->find( $borrowernumber )->set($infos)->store; }; + if ( $@ ) { # FIXME We could provide better error handling here my $patron = Koha::Patrons->find( $borrowernumber ); $infos->{cardnumber} = $patron ? $patron->cardnumber || '' : ''; push @errors, { error => "can_not_update", borrowernumber => $infos->{borrowernumber}, cardnumber => $infos->{cardnumber} }; -- 2.11.0