Bugzilla – Attachment 60992 Details for
Bug 17755
Introduce Koha::Patron::Attribute::Type(s)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17755: (QA followup) Return $self when appropriate
Bug-17755-QA-followup-Return-self-when-appropriate.patch (text/plain), 10.16 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-03-10 13:50:02 UTC
(
hide
)
Description:
Bug 17755: (QA followup) Return $self when appropriate
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-03-10 13:50:02 UTC
Size:
10.16 KB
patch
obsolete
>From c5c1c53b548a9bde78076304f450c8630269e24d Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Thu, 19 Jan 2017 15:09:15 -0300 >Subject: [PATCH] Bug 17755: (QA followup) Return $self when appropriate > >As failure situations raise exceptions that should be handled outside >the object class, methods should return $self so successive calls can be >chained nicely. > >This patch makes methods return $self and adjusts the tests to reflect >this change. > >Make sure tests pass: >- Run: > $ prove t/db_dependent/Koha/Patron/Attribute/Types.t >=> SUCCESS: Tests return green >- Sign off :-D >--- > Koha/Object/Limit/Library.pm | 33 +++++++-- > t/db_dependent/Koha/Patron/Attribute/Types.t | 106 ++++++++++++++++++--------- > 2 files changed, 97 insertions(+), 42 deletions(-) > >diff --git a/Koha/Object/Limit/Library.pm b/Koha/Object/Limit/Library.pm >index 678782b..7d86b94 100644 >--- a/Koha/Object/Limit/Library.pm >+++ b/Koha/Object/Limit/Library.pm >@@ -19,6 +19,7 @@ use Modern::Perl; > > use Koha::Database; > use Koha::Exceptions; >+use Koha::Libraries; > > use Try::Tiny; > >@@ -50,6 +51,10 @@ my $limits = $object->library_limits(); > > $object->library_limits( \@branchcodes ); > >+Accessor method for library limits. When updating library limits, it accepts >+a list of branchcodes. If requested to return the current library limits >+it returns a Koha::Libraries object with the corresponding libraries. >+ > =cut > > sub library_limits { >@@ -67,6 +72,9 @@ sub library_limits { > > my $limits = $object->get_library_limits(); > >+Returns the current library limits in the form of a Koha::Libraries iterator object. >+It returns undef if no library limits defined. >+ > =cut > > sub get_library_limits { >@@ -77,7 +85,12 @@ sub get_library_limits { > { $self->_library_limits->{id} => $self->id } ) > ->get_column( $self->_library_limits->{library} )->all(); > >- return \@branchcodes; >+ return unless @branchcodes; >+ >+ my $filter = [ map { { branchcode => $_ } } @branchcodes ]; >+ my $libraries = Koha::Libraries->search( $filter ); >+ >+ return $libraries; > } > > =head3 add_library_limit >@@ -93,9 +106,8 @@ sub add_library_limit { > "Required parameter 'branchcode' missing") > unless $branchcode; > >- my $limitation; > try { >- $limitation = $self->_library_limit_rs->update_or_create( >+ $self->_library_limit_rs->update_or_create( > { $self->_library_limits->{id} => $self->id, > $self->_library_limits->{library} => $branchcode > } >@@ -105,7 +117,7 @@ sub add_library_limit { > Koha::Exceptions::CannotAddLibraryLimit->throw( $_->{msg} ); > }; > >- return $limitation ? 1 : undef; >+ return $self; > } > > =head3 del_library_limit >@@ -145,14 +157,19 @@ $object->replace_library_limits( \@branchcodes ); > sub replace_library_limits { > my ( $self, $branchcodes ) = @_; > >- $self->_library_limit_rs->search( >- { $self->_library_limits->{id} => $self->id } )->delete; >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ $self->_library_limit_rs->search( >+ { $self->_library_limits->{id} => $self->id } )->delete; > >- my @return_values = map { $self->add_library_limit($_) } @$branchcodes; >+ map { $self->add_library_limit($_) } @$branchcodes; >+ } >+ ); > >- return \@return_values; >+ return $self; > } > >+ > =head3 Koha::Objects->_library_limit_rs > > Returns the internal resultset for the branch limitation table or creates it if undefined >diff --git a/t/db_dependent/Koha/Patron/Attribute/Types.t b/t/db_dependent/Koha/Patron/Attribute/Types.t >index 8fa73cc..876e908 100644 >--- a/t/db_dependent/Koha/Patron/Attribute/Types.t >+++ b/t/db_dependent/Koha/Patron/Attribute/Types.t >@@ -72,7 +72,7 @@ subtest 'new() tests' => sub { > > subtest 'library_limits() tests' => sub { > >- plan tests => 5; >+ plan tests => 13; > > $schema->storage->txn_begin; > >@@ -89,43 +89,68 @@ subtest 'library_limits() tests' => sub { > my $library = $builder->build( { source => 'Branch' } )->{branchcode}; > > my $library_limits = $attribute_type->library_limits(); >- is_deeply( $library_limits, [], >+ is( $library_limits, undef, > 'No branch limitations defined for attribute type' ); > > my $print_error = $dbh->{PrintError}; > $dbh->{PrintError} = 0; > > throws_ok { >- $library_limits = $attribute_type->library_limits( ['fake'] ); >+ $attribute_type->library_limits( ['fake'] ); > } > 'Koha::Exceptions::CannotAddLibraryLimit', > 'Exception thrown on single invalid branchcode'; >+ $library_limits = $attribute_type->library_limits(); >+ is( $library_limits, undef, >+ 'No branch limitations defined for attribute type' ); > > throws_ok { >- $library_limits >- = $attribute_type->library_limits( [ 'fake', $library ] ); >+ $attribute_type->library_limits( [ 'fake', $library ] ); > } > 'Koha::Exceptions::CannotAddLibraryLimit', > 'Exception thrown on invalid branchcode present'; > >+ $library_limits = $attribute_type->library_limits(); >+ is( $library_limits, undef, >+ 'No branch limitations defined for attribute type' ); >+ > $dbh->{PrintError} = $print_error; > >- $library_limits = $attribute_type->library_limits( [$library] ); >- is_deeply( $library_limits, [1], 'Library limits correctly set' ); >+ $attribute_type->library_limits( [$library] ); >+ $library_limits = $attribute_type->library_limits; >+ is( $library_limits->count, 1, 'Library limits correctly set (count)' ); >+ my $limit_library = $library_limits->next; >+ ok( $limit_library->isa('Koha::Library'), >+ 'Library limits correctly set (type)' >+ ); >+ is( $limit_library->branchcode, >+ $library, 'Library limits correctly set (value)' ); > > my $another_library > = $builder->build( { source => 'Branch' } )->{branchcode}; >- >- $library_limits >- = $attribute_type->library_limits( [ $library, $another_library ] ); >- is_deeply( $library_limits, [ 1, 1 ], 'Library limits correctly set' ); >+ my @branchcodes_list = ( $library, $another_library ); >+ >+ $attribute_type->library_limits( \@branchcodes_list ); >+ $library_limits = $attribute_type->library_limits; >+ is( $library_limits->count, 2, 'Library limits correctly set (count)' ); >+ >+ while ( $limit_library = $library_limits->next ) { >+ ok( $limit_library->isa('Koha::Library'), >+ 'Library limits correctly set (type)' >+ ); >+ ok( eval { >+ grep { $limit_library->branchcode eq $_ } @branchcodes_list; >+ }, >+ 'Library limits correctly set (values)' >+ ); >+ } > > $schema->storage->txn_rollback; > }; > > subtest 'add_library_limit() tests' => sub { > >- plan tests => 3; >+ plan tests => 4; > > $schema->storage->txn_begin; > >@@ -140,12 +165,15 @@ subtest 'add_library_limit() tests' => sub { > )->store(); > > throws_ok { $attribute_type->add_library_limit() } >- 'Koha::Exceptions::MissingParameter', >- 'branchcode parameter is mandatory'; >+ 'Koha::Exceptions::MissingParameter', 'branchcode parameter is mandatory'; > > my $library = $builder->build( { source => 'Branch' } )->{branchcode}; >- is( $attribute_type->add_library_limit($library), >- 1, 'Library limit successfully added' ); >+ $attribute_type->add_library_limit($library); >+ my $rs = Koha::Database->schema->resultset('BorrowerAttributeTypesBranch') >+ ->search( { bat_code => 'code' } ); >+ is( $rs->count, 1, 'Library limit successfully added (count)' ); >+ is( $rs->next->b_branchcode->branchcode, >+ $library, 'Library limit successfully added (value)' ); > > my $print_error = $dbh->{PrintError}; > $dbh->{PrintError} = 0; >@@ -209,7 +237,7 @@ subtest 'del_library_limit() tests' => sub { > > subtest 'replace_library_limits() tests' => sub { > >- plan tests => 6; >+ plan tests => 10; > > $schema->storage->txn_begin; > >@@ -223,26 +251,36 @@ subtest 'replace_library_limits() tests' => sub { > } > )->store(); > >- is_deeply( $attribute_type->replace_library_limits( [] ), >- [], 'Replacing with empty array returns an empty array as expected' ); >- >- is_deeply( $attribute_type->library_limits(), >- [], 'Replacing with empty array yields no library limits' ); >+ $attribute_type->replace_library_limits( [] ); >+ my $library_limits = $attribute_type->library_limits; >+ is( $library_limits, undef, 'Replacing with empty array yields no library limits' ); > > my $library_1 = $builder->build({ source => 'Branch'})->{branchcode}; > my $library_2 = $builder->build({ source => 'Branch'})->{branchcode}; >- >- is_deeply( $attribute_type->replace_library_limits( [$library_1] ), >- [ 1 ], 'Successfully adds a single library limit' ); >- >- is_deeply( $attribute_type->library_limits(), >- [ $library_1 ], 'Library limit correctly set' ); >- >- is_deeply( $attribute_type->replace_library_limits( [$library_1, $library_2] ), >- [ 1, 1 ], 'Successfully adds two library limit' ); >- >- is_deeply( $attribute_type->library_limits(), >- [ $library_1, $library_2 ], 'Library limit correctly set' ); >+ my $library_3 = $builder->build({ source => 'Branch'})->{branchcode}; >+ >+ $attribute_type->replace_library_limits( [$library_1] ); >+ $library_limits = $attribute_type->library_limits; >+ is( $library_limits->count, 1, 'Successfully adds a single library limit' ); >+ my $library_limit = $library_limits->next; >+ is( $library_limit->branchcode, $library_1, 'Library limit correctly set' ); >+ >+ >+ my @branchcodes_list = ($library_1, $library_2, $library_3); >+ $attribute_type->replace_library_limits( [$library_1, $library_2, $library_3] ); >+ $library_limits = $attribute_type->library_limits; >+ is( $library_limits->count, 3, 'Successfully adds two library limit' ); >+ >+ while ( my $limit_library = $library_limits->next ) { >+ ok( $limit_library->isa('Koha::Library'), >+ 'Library limits correctly set (type)' >+ ); >+ ok( eval { >+ grep { $limit_library->branchcode eq $_ } @branchcodes_list; >+ }, >+ 'Library limits correctly set (values)' >+ ); >+ } > > $schema->storage->txn_rollback; > }; >-- >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 17755
:
58142
|
58143
|
58144
|
58145
|
58167
|
58495
|
58496
|
58497
|
58498
|
59354
|
60989
|
60990
|
60991
|
60992
|
60995
|
61000
|
61001
|
61548
|
61549
|
61550
|
61551
|
61552