Bugzilla – Attachment 76912 Details for
Bug 20287
Move AddMember and ModMember to Koha::Patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20287: Move ModMember to Koha::Patron
Bug-20287-Move-ModMember-to-KohaPatron.patch (text/plain), 34.94 KB, created by
Jonathan Druart
on 2018-07-12 18:49:57 UTC
(
hide
)
Description:
Bug 20287: Move ModMember to Koha::Patron
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2018-07-12 18:49:57 UTC
Size:
34.94 KB
patch
obsolete
>From 43697d5007226a4cb114631b7ad89168c645c1cc Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 23 Feb 2018 12:42:15 -0300 >Subject: [PATCH] Bug 20287: Move ModMember to Koha::Patron > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20287
:
73436
|
76901
|
76902
|
76903
|
76904
|
76905
|
76906
|
76907
|
76908
|
76909
|
76910
|
76911
| 76912 |
76913
|
76914
|
76915
|
76916
|
76917
|
76918
|
76919
|
76920
|
76921
|
76922
|
76923
|
76924
|
76927
|
77221