Bug 16960 - Patron::Modifications should be fixed

The changes from opac-memberentry do not reach the table, since the
Patron::Modifications object does not work well.

Test Plan:
1) Apply this patch
2) Create some patron modification requests
3) Ensure you can approve and deny modifications
4) Ensure patron self registration works

Signed-off-by: Bob Birchall <bob@calyx.net.au>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Kyle M Hall 2016-07-22 16:50:50 +00:00
parent 29b4ee5c74
commit 1016139132
9 changed files with 111 additions and 322 deletions

View File

@ -27,14 +27,44 @@ use base qw(Koha::Object);
=head1 NAME
Koha::Item - Koha Item object class
=head1 API
Koha::Patron::Modification - Class represents a requrest to modify or create a patron
=head2 Class Methods
=cut
=head2 approve
$m->approve();
Commits the pending modifications to the borrower record and removes
them from the modifications table.
=cut
sub approve {
my ($self) = @_;
my $data = $self->unblessed();
delete $data->{timestamp};
delete $data->{verification_token};
foreach my $key ( keys %$data ) {
delete $data->{$key} unless ( defined( $data->{$key} ) );
}
my $patron = Koha::Patrons->find( $self->borrowernumber );
return unless $patron;
$patron->set($data);
if ( $patron->store() ) {
return $self->delete();
}
}
=head3 type
=cut

View File

@ -25,80 +25,20 @@ Koha::Patron::Modifications
use Modern::Perl;
use C4::Context;
use C4::Debug;
use Koha::Patron::Modification;
use base qw(Koha::Objects);
=head2 AddModifications
=head2 pending_count
Koha::Patron::Modifications->AddModifications( $data );
$count = Koha::Patron::Modifications->pending_count();
Adds or updates modifications for a patron
Requires either the key borrowernumber, or verification_token
to be part of the passed in hash.
Returns the number of pending modifications for existing patrons.
=cut
sub AddModifications {
my ( $self, $data ) = @_;
delete $data->{borrowernumber};
if( $self->{borrowernumber} ) {
return if( not keys %$data );
$data->{borrowernumber} = $self->{borrowernumber};
$data->{verification_token} = '';
}
elsif( $self->{verification_token} ) {
$data->{verification_token} = $self->{verification_token};
$data->{borrowernumber} = 0;
}
else {
return;
}
my $rs = Koha::Database->new()->schema->resultset('BorrowerModification');
return $rs->update_or_create($data, { key => 'primary' } );
}
=head2 Verify
$verified = Koha::Patron::Modifications->Verify( $verification_token );
Returns true if the passed in token is valid.
=cut
sub Verify {
my ( $self, $verification_token ) = @_;
$verification_token =
($verification_token)
? $verification_token
: $self->{'verification_token'};
my $dbh = C4::Context->dbh;
my $query = "
SELECT COUNT(*) AS count
FROM borrower_modifications
WHERE verification_token = ?
";
my $sth = $dbh->prepare($query);
$sth->execute($verification_token);
my $result = $sth->fetchrow_hashref();
return $result->{'count'};
}
=head2 GetPendingModificationsCount
$count = Koha::Patron::Modifications->GetPendingModificationsCount();
Returns the number of pending modifications for existing patron.
=cut
sub GetPendingModificationsCount {
sub pending_count {
my ( $self, $branchcode ) = @_;
my $dbh = C4::Context->dbh;
@ -119,18 +59,18 @@ sub GetPendingModificationsCount {
$sth->execute(@params);
my $result = $sth->fetchrow_hashref();
return $result->{'count'};
return $result->{count};
}
=head2 GetPendingModifications
=head2 pending
$arrayref = Koha::Patron::Modifications->GetPendingModifications();
$arrayref = Koha::Patron::Modifications->pending();
Returns an arrayref of hashrefs for all pending modifications for existing patrons.
=cut
sub GetPendingModifications {
sub pending {
my ( $self, $branchcode ) = @_;
my $dbh = C4::Context->dbh;
@ -162,144 +102,6 @@ sub GetPendingModifications {
return \@m;
}
=head2 ApproveModifications
Koha::Patron::Modifications->ApproveModifications( $borrowernumber );
Commits the pending modifications to the borrower record and removes
them from the modifications table.
=cut
sub ApproveModifications {
my ( $self, $borrowernumber ) = @_;
$borrowernumber =
($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'};
return unless $borrowernumber;
my $data = $self->GetModifications( { borrowernumber => $borrowernumber } );
delete $data->{timestamp};
delete $data->{verification_token};
my $rs = Koha::Database->new()->schema->resultset('Borrower')->search({
borrowernumber => $data->{borrowernumber},
});
if( $rs->update($data) ) {
$self->DelModifications( { borrowernumber => $borrowernumber } );
}
}
=head2 DenyModifications
Koha::Patron::Modifications->DenyModifications( $borrowernumber );
Removes the modifications from the table for the given patron,
without committing the changes to the patron record.
=cut
sub DenyModifications {
my ( $self, $borrowernumber ) = @_;
$borrowernumber =
($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'};
return unless $borrowernumber;
return $self->DelModifications( { borrowernumber => $borrowernumber } );
}
=head2 DelModifications
Koha::Patron::Modifications->DelModifications({
[ borrowernumber => $borrowernumber ],
[ verification_token => $verification_token ]
});
Deletes the modifications for the given borrowernumber or verification token.
=cut
sub DelModifications {
my ( $self, $params ) = @_;
my ( $field, $value );
if ( $params->{'borrowernumber'} ) {
$field = 'borrowernumber';
$value = $params->{'borrowernumber'};
}
elsif ( $params->{'verification_token'} ) {
$field = 'verification_token';
$value = $params->{'verification_token'};
}
return unless $value;
my $dbh = C4::Context->dbh;
$field = $dbh->quote_identifier($field);
my $query = "
DELETE
FROM borrower_modifications
WHERE $field = ?
";
my $sth = $dbh->prepare($query);
return $sth->execute($value);
}
=head2 GetModifications
$hashref = Koha::Patron::Modifications->GetModifications({
[ borrowernumber => $borrowernumber ],
[ verification_token => $verification_token ]
});
Gets the modifications for the given borrowernumber or verification token.
=cut
sub GetModifications {
my ( $self, $params ) = @_;
my ( $field, $value );
if ( defined( $params->{'borrowernumber'} ) ) {
$field = 'borrowernumber';
$value = $params->{'borrowernumber'};
}
elsif ( defined( $params->{'verification_token'} ) ) {
$field = 'verification_token';
$value = $params->{'verification_token'};
}
return unless $value;
my $dbh = C4::Context->dbh;
$field = $dbh->quote_identifier($field);
my $query = "
SELECT *
FROM borrower_modifications
WHERE $field = ?
";
my $sth = $dbh->prepare($query);
$sth->execute($value);
my $data = $sth->fetchrow_hashref();
foreach my $key ( keys %$data ) {
delete $data->{$key} unless ( defined( $data->{$key} ) );
}
return $data;
}
sub _type {
return 'BorrowerModification';
}

View File

@ -65,8 +65,7 @@ my $branch =
my $pendingcomments = numberofreviews(0);
my $pendingtags = get_count_by_tag_status(0);
my $pendingsuggestions = CountSuggestion("ASKED");
my $pending_borrower_modifications =
Koha::Patron::Modifications->GetPendingModificationsCount( $branch );
my $pending_borrower_modifications = Koha::Patron::Modifications->pending_count( $branch );
my $pending_discharge_requests = Koha::Patron::Discharge::count({ pending => 1 });
$template->param(

View File

@ -89,7 +89,7 @@ else {
my $pending_borrower_modifications =
Koha::Patron::Modifications->GetPendingModificationsCount( $branch );
Koha::Patron::Modifications->pending_count( $branch );
$template->param(
no_add => $no_add,

View File

@ -50,14 +50,14 @@ foreach my $param (@params) {
my $action = $query->param($param);
if ( $action eq 'approve' ) {
Koha::Patron::Modifications->ApproveModifications( $borrowernumber );
my $m = Koha::Patron::Modifications->find( { borrowernumber => $borrowernumber } );
$m->approve() if $m;
}
elsif ( $action eq 'deny' ) {
Koha::Patron::Modifications->DenyModifications( $borrowernumber );
}
elsif ( $action eq 'ignore' ) {
my $m = Koha::Patron::Modifications->find( { borrowernumber => $borrowernumber } );
$m->delete() if $m;
}
# elsif ( $action eq 'ignore' ) { }
}
}

View File

@ -49,7 +49,7 @@ my $branch =
: undef;
my $pending_modifications =
Koha::Patron::Modifications->GetPendingModifications($branch);
Koha::Patron::Modifications->pending($branch);
my $borrowers;
foreach my $pm (@$pending_modifications) {

View File

@ -27,6 +27,7 @@ use C4::Output;
use C4::Members;
use C4::Form::MessagingPreferences;
use Koha::Patrons;
use Koha::Patron::Modification;
use Koha::Patron::Modifications;
use C4::Branch qw(GetBranchesLoop);
use C4::Scrubber;
@ -127,11 +128,11 @@ if ( $action eq 'create' ) {
$template->param( 'email' => $borrower{'email'} );
my $verification_token = md5_hex( \%borrower );
$borrower{'password'} = random_string("..........");
Koha::Patron::Modifications->new(
verification_token => $verification_token )
->AddModifications(\%borrower);
$borrower{password} = random_string("..........");
$borrower{verification_token} = $verification_token;
Koha::Patron::Modification->new( \%borrower )->store();
#Send verification email
my $letter = C4::Letters::GetPreparedLetter(
@ -224,12 +225,10 @@ elsif ( $action eq 'update' ) {
}
);
my $m =
Koha::Patron::Modifications->new(
borrowernumber => $borrowernumber );
$borrower_changes{borrowernumber} = $borrowernumber;
my $m = Koha::Patron::Modification->new( \%borrower_changes )->store();
$m->DelModifications;
$m->AddModifications(\%borrower_changes);
$template->param(
borrower => GetMember( borrowernumber => $borrowernumber ),
);

View File

@ -34,10 +34,10 @@ unless ( C4::Context->preference('PatronSelfRegistration') ) {
}
my $token = $cgi->param('token');
my $m = Koha::Patron::Modifications->new( verification_token => $token );
my $m = Koha::Patron::Modifications->find( { verification_token => $token } );
my ( $template, $borrowernumber, $cookie );
if ( $m->Verify() ) {
if ( $m ) {
( $template, $borrowernumber, $cookie ) = get_template_and_user(
{
template_name => "opac-registration-confirmation.tt",
@ -50,13 +50,13 @@ if ( $m->Verify() ) {
$template->param(
OpacPasswordChange => C4::Context->preference('OpacPasswordChange') );
my $borrower = Koha::Patron::Modifications->GetModifications({ verification_token => $token });
my $borrower = $m->unblessed();
my $password;
( $borrowernumber, $password ) = AddMember_Opac(%$borrower);
if ($borrowernumber) {
Koha::Patron::Modifications->DelModifications({ verification_token => $token });
$m->delete();
C4::Form::MessagingPreferences::handle_form_action($cgi, { borrowernumber => $borrowernumber }, $template, 1, C4::Context->preference('PatronSelfRegistrationDefaultCategory') ) if C4::Context->preference('EnhancedMessagingPreferences');
$template->param( password_cleartext => $password );

View File

@ -1,133 +1,92 @@
#!/usr/bin/perl
use Modern::Perl;
use Test::More tests => 14;
use Test::More tests => 8;
use C4::Context;
use t::lib::TestBuilder;
use C4::Members;
use Koha::Patron::Modifications;
BEGIN {
use_ok('Koha::Patron::Modification');
use_ok('Koha::Patron::Modifications');
}
my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin;
my $dbh = C4::Context->dbh;
$dbh->{RaiseError} = 1;
$dbh->{AutoCommit} = 0;
$dbh->do("DELETE FROM borrower_modifications");
## Create new pending modification
Koha::Patron::Modifications->new( verification_token => '1234567890' )
->AddModifications( { surname => 'Hall', firstname => 'Kyle' } );
Koha::Patron::Modification->new(
{
verification_token => '1234567890',
surname => 'Hall',
firstname => 'Kyle'
}
)->store();
## Get the new pending modification
my $borrower = Koha::Patron::Modifications->GetModifications(
{ verification_token => '1234567890' } );
my $borrower =
Koha::Patron::Modifications->find( { verification_token => '1234567890' } );
## Verify we get the same data
ok( $borrower->{'surname'} = 'Hall',
'Test AddModifications() and GetModifications()' );
## Check the Verify method
ok(
Koha::Patron::Modifications->Verify('1234567890'),
'Test that Verify() succeeds with a valid token'
);
## Delete the pending modification
$borrower = Koha::Patron::Modifications->DelModifications(
{ verification_token => '1234567890' } );
## Verify it's no longer in the database
$borrower = Koha::Patron::Modifications->GetModifications(
{ verification_token => '1234567890' } );
ok( !defined( $borrower->{'surname'} ), 'Test DelModifications()' );
## Check the Verify method
ok(
!Koha::Patron::Modifications->Verify('1234567890'),
'Test that Verify() method fails for a bad token'
);
is( $borrower->surname, 'Hall', 'Found modification has matching surname' );
## Create new pending modification for a patron
my $builder = t::lib::TestBuilder->new;
my $borr1 = $builder->build({ source => 'Borrower' })->{borrowernumber};
Koha::Patron::Modifications->new( borrowernumber => $borr1 )
->AddModifications( { surname => 'Hall', firstname => 'Kyle' } );
my $borr1 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
my $m1 = Koha::Patron::Modification->new(
{
borrowernumber => $borr1,
surname => 'Hall',
firstname => 'Kyle'
}
)->store();
## Test the counter
ok( Koha::Patron::Modifications->GetPendingModificationsCount() == 1,
'Test GetPendingModificationsCount()' );
is( Koha::Patron::Modifications->pending_count,
1, 'Test pending_count()' );
## Create new pending modification for another patron
my $borr2 = $builder->build({ source => 'Borrower' })->{borrowernumber};
Koha::Patron::Modifications->new( borrowernumber => $borr2 )
->AddModifications( { surname => 'Smith', firstname => 'Sandy' } );
my $borr2 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
my $m2 = Koha::Patron::Modification->new(
{
borrowernumber => $borr2,
surname => 'Smith',
firstname => 'Sandy'
}
)->store();
## Test the counter
ok(
Koha::Patron::Modifications->GetPendingModificationsCount() == 2,
'Add a new pending modification and test GetPendingModificationsCount() again'
is(
Koha::Patron::Modifications->pending_count(), 2,
'Add a new pending modification and test pending_count() again'
);
## Check GetPendingModifications
my $pendings = Koha::Patron::Modifications->GetPendingModifications();
my @firstnames_mod = sort ( $pendings->[0]->{firstname}, $pendings->[1]->{firstname} );
ok( $firstnames_mod[0] eq 'Kyle', 'Test GetPendingModifications()' );
ok( $firstnames_mod[1] eq 'Sandy', 'Test GetPendingModifications() again' );
my $pendings = Koha::Patron::Modifications->pending;
my @firstnames_mod =
sort ( $pendings->[0]->{firstname}, $pendings->[1]->{firstname} );
ok( $firstnames_mod[0] eq 'Kyle', 'Test pending()' );
ok( $firstnames_mod[1] eq 'Sandy', 'Test pending() again' );
## This should delete the row from the table
Koha::Patron::Modifications->DenyModifications( $borr2 );
## Test the counter
ok( Koha::Patron::Modifications->GetPendingModificationsCount() == 1,
'Test DenyModifications()' );
$m2->delete();
## Save a copy of the borrowers original data
my $old_borrower = GetMember( borrowernumber => $borr1 );
## Apply the modifications
Koha::Patron::Modifications->ApproveModifications( $borr1 );
## Test the counter
ok(
Koha::Patron::Modifications->GetPendingModificationsCount() == 0,
'Test ApproveModifications() removes pending modification from db'
);
$m1->approve();
## Get a copy of the borrowers current data
my $new_borrower = GetMember( borrowernumber => $borr1 );
## Check to see that the approved modifications were saved
ok( $new_borrower->{'surname'} eq 'Hall',
'Test ApproveModifications() applys modification to borrower' );
## Now let's put it back the way it was
Koha::Patron::Modifications->new( borrowernumber => $borr1 )->AddModifications(
{
surname => $old_borrower->{'surname'},
firstname => $old_borrower->{'firstname'}
}
);
## Test the counter
ok( Koha::Patron::Modifications->GetPendingModificationsCount() == 1,
'Test GetPendingModificationsCount()' );
## Apply the modifications
Koha::Patron::Modifications->ApproveModifications( $borr1 );
## Test the counter
ok(
Koha::Patron::Modifications->GetPendingModificationsCount() == 0,
'Test ApproveModifications() removes pending modification from db, again'
);
$new_borrower = GetMember( borrowernumber => $borr1 );
## Test to verify the borrower has been updated with the original values
ok(
$new_borrower->{'surname'} eq $old_borrower->{'surname'},
'Test ApproveModifications() applys modification to borrower, again'
);
'Test approve() applys modification to borrower' );
$dbh->rollback();