From 3f2ade46ace082e69cb9f40f7cf241853e1897f8 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- 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 678782bf56..7d86b94d63 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 8fa73cc99b..876e908910 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.39.5