Bugzilla – Attachment 58360 Details for
Bug 17767
Let Koha::Patron::Modification handle extended attributes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17767: Make Koha::Patron::Modification handle extended attributes
Bug-17767-Make-KohaPatronModification-handle-exten.patch (text/plain), 9.33 KB, created by
Tomás Cohen Arazi (tcohen)
on 2016-12-21 16:02:51 UTC
(
hide
)
Description:
Bug 17767: Make Koha::Patron::Modification handle extended attributes
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2016-12-21 16:02:51 UTC
Size:
9.33 KB
patch
obsolete
>From 7d92c07495115401a038d95c1b29b634fd1063ea Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 13 Dec 2016 17:45:41 -0300 >Subject: [PATCH] Bug 17767: Make Koha::Patron::Modification handle extended > attributes > >This patch makes Koha::Patron::Modification aware of the new extended_attributes >column, which is expected to contain valid JSON data. > >The ->store method is modified so it validates the field value (i.e. the content >is decoded using the JSON library) and raises a convenient exception in case of >failure. > >This behaviour change is covered by the provided unit tests. > >To test: >- Apply the patchset >- Run: > $ prove t/db_dependent/Koha/Patron/Modifications.t >=> SUCCESS: Tests make sense, and they pass >- Sign off :-D >--- > Koha/Exceptions/Patron/Modification.pm | 4 ++ > Koha/Patron/Modification.pm | 73 +++++++++++++++++++++--- > Koha/Patron/Modifications.pm | 5 ++ > t/db_dependent/Koha_borrower_modifications.t | 84 ++++++++++++++++++++++++++-- > 4 files changed, 153 insertions(+), 13 deletions(-) > >diff --git a/Koha/Exceptions/Patron/Modification.pm b/Koha/Exceptions/Patron/Modification.pm >index 4cf5cfa..d9ba650 100644 >--- a/Koha/Exceptions/Patron/Modification.pm >+++ b/Koha/Exceptions/Patron/Modification.pm >@@ -10,6 +10,10 @@ use Exception::Class ( > 'Koha::Exceptions::Patron::Modification::DuplicateVerificationToken' => { > isa => 'Koha::Exceptions::Patron::Modification', > description => "The verification token given already exists" >+ }, >+ 'Koha::Exceptions::Patron::Modification::InvalidData' => { >+ isa => 'Koha::Exceptions::Patron::Modification', >+ description => "Some passed data is invalid" > } > ); > >diff --git a/Koha/Patron/Modification.pm b/Koha/Patron/Modification.pm >index d1e033b..696f7e8 100644 >--- a/Koha/Patron/Modification.pm >+++ b/Koha/Patron/Modification.pm >@@ -22,9 +22,13 @@ use Modern::Perl; > use Carp; > > use Koha::Database; >- >-use Koha::Patron::Modifications; > use Koha::Exceptions::Patron::Modification; >+use Koha::Patron::Modifications; >+# TODO: Remove once Koha::Patron::Attribute(s) is implemented >+use C4::Members::Attributes qw( SetBorrowerAttributes ); >+ >+use JSON; >+use Try::Tiny; > > use base qw(Koha::Object); > >@@ -44,16 +48,32 @@ sub store { > my ($self) = @_; > > if ( $self->verification_token ) { >- if ( Koha::Patron::Modifications->search( { verification_token => $self->verification_token } )->count() ) { >- Koha::Exceptions::Patron::Modification::DuplicateVerificationToken->throw( >- "Duplicate verification token " . $self->verification_token >- ); >+ if (Koha::Patron::Modifications->search( >+ { verification_token => $self->verification_token } >+ )->count() >+ ) >+ { >+ Koha::Exceptions::Patron::Modification::DuplicateVerificationToken >+ ->throw( >+ "Duplicate verification token " . $self->verification_token ); >+ } >+ } >+ >+ if ( $self->extended_attributes ) { >+ try { >+ my $json_parser = JSON->new; >+ $json_parser->decode( $self->extended_attributes ); > } >+ catch { >+ Koha::Exceptions::Patron::Modification::InvalidData->throw( >+ 'The passed extended_attributes is not valid JSON'); >+ }; > } > > return $self->SUPER::store(); > } > >+ > =head2 approve > > $m->approve(); >@@ -70,6 +90,7 @@ sub approve { > > delete $data->{timestamp}; > delete $data->{verification_token}; >+ delete $data->{extended_attributes}; > > foreach my $key ( keys %$data ) { > delete $data->{$key} unless ( defined( $data->{$key} ) ); >@@ -81,11 +102,47 @@ sub approve { > > $patron->set($data); > >- if ( $patron->store() ) { >- return $self->delete(); >+ # Take care of extended attributes >+ if ( $self->extended_attributes ) { >+ our $extended_attributes >+ = try { decode_json( $self->extended_attributes ) } >+ catch { >+ Koha::Exceptions::Patron::Modification::InvalidData->throw( >+ 'The passed extended_attributes is not valid JSON'); >+ }; > } >+ >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ try { >+ $patron->store(); >+ >+ # Take care of extended attributes >+ if ( $self->extended_attributes ) { >+ my $extended_attributes >+ = decode_json( $self->extended_attributes ); >+ SetBorrowerAttributes( $patron->borrowernumber, >+ $extended_attributes ); >+ } >+ } >+ catch { >+ if ( $_->isa('DBIx::Class::Exception') ) { >+ Koha::Exceptions::Patron::Modification->throw( >+ $_->{msg} ); >+ } >+ else { >+ Koha::Exceptions::Patron::Modification->throw($@); >+ } >+ }; >+ } >+ ); >+ >+ return $self->delete(); > } > >+ >+ >+ > =head3 type > > =cut >diff --git a/Koha/Patron/Modifications.pm b/Koha/Patron/Modifications.pm >index 088f7a0..3ecef11 100644 >--- a/Koha/Patron/Modifications.pm >+++ b/Koha/Patron/Modifications.pm >@@ -28,6 +28,8 @@ use C4::Context; > > use Koha::Patron::Modification; > >+use JSON; >+ > use base qw(Koha::Objects); > > =head2 pending_count >@@ -93,6 +95,9 @@ sub pending { > my @m; > while ( my $row = $sth->fetchrow_hashref() ) { > foreach my $key ( keys %$row ) { >+ if ( defined $row->{$key} && $key eq 'extended_attributes' ) { >+ $row->{$key} = from_json($row->{$key}); >+ } > delete $row->{$key} unless defined $row->{$key}; > } > >diff --git a/t/db_dependent/Koha_borrower_modifications.t b/t/db_dependent/Koha_borrower_modifications.t >index 0a03821..febe2f9 100755 >--- a/t/db_dependent/Koha_borrower_modifications.t >+++ b/t/db_dependent/Koha_borrower_modifications.t >@@ -1,11 +1,30 @@ > #!/usr/bin/perl > >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. >+ > use Modern::Perl; >-use Test::More tests => 10; >-use Try::Tiny; >+ >+use Test::More tests => 11; >+use Test::Exception; > > use t::lib::TestBuilder; > >+use String::Random qw( random_string ); >+use Try::Tiny; >+ > use C4::Context; > use C4::Members; > >@@ -14,7 +33,61 @@ BEGIN { > use_ok('Koha::Patron::Modifications'); > } > >-my $schema = Koha::Database->new->schema; >+my $schema = Koha::Database->new->schema; >+my $builder = t::lib::TestBuilder->new; >+ >+ >+subtest 'store( extended_attributes ) tests' => sub { >+ >+ plan tests => 4; >+ >+ $schema->storage->txn_begin; >+ >+ Koha::Patron::Modifications->search->delete; >+ >+ my $patron >+ = $builder->build( { source => 'Borrower' } )->{borrowernumber}; >+ my $verification_token = random_string(".........."); >+ my $valid_json_text = '[{"code":"CODE","value":"VALUE"}]'; >+ my $invalid_json_text = '[{'; >+ >+ Koha::Patron::Modification->new( >+ { verification_token => $verification_token, >+ borrowernumber => $patron, >+ surname => 'Hall', >+ extended_attributes => $valid_json_text >+ } >+ )->store(); >+ >+ my $patron_modification >+ = Koha::Patron::Modifications->search( { borrowernumber => $patron } ) >+ ->next; >+ >+ is( $patron_modification->surname, >+ 'Hall', 'Patron modification correctly stored with valid JSON data' ); >+ is( $patron_modification->extended_attributes, >+ $valid_json_text, >+ 'Patron modification correctly stored with valid JSON data' ); >+ >+ $verification_token = random_string(".........."); >+ throws_ok { >+ Koha::Patron::Modification->new( >+ { verification_token => $verification_token, >+ borrowernumber => $patron, >+ surname => 'Hall', >+ extended_attributes => $invalid_json_text >+ } >+ )->store(); >+ } >+ 'Koha::Exceptions::Patron::Modification::InvalidData', >+ 'Trying to store invalid JSON in extended_attributes field raises exception'; >+ >+ is( $@, 'The passed extended_attributes is not valid JSON' ); >+ >+ $schema->storage->txn_rollback; >+}; >+ >+ > $schema->storage->txn_begin; > > my $dbh = C4::Context->dbh; >@@ -52,7 +125,6 @@ my $borrower = > 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}; > > my $m1 = Koha::Patron::Modification->new( >@@ -106,4 +178,6 @@ my $new_borrower = GetMember( borrowernumber => $borr1 ); > ok( $new_borrower->{'surname'} eq 'Hall', > 'Test approve() applies modification to borrower' ); > >-$dbh->rollback(); >+$schema->storage->txn_rollback; >+ >+1; >-- >2.7.4
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 17767
:
58256
|
58257
|
58258
|
58259
|
58260
|
58358
|
58359
|
58360
|
58361
|
58362
|
58365
|
58366
|
58367
|
58368
|
58369
|
58437
|
58438
|
58439
|
58440
|
58441