Bugzilla – Attachment 172112 Details for
Bug 37902
Timezone ignored when passing rfc3339 formatted date (search 'q')
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37902: (QA follow-up) Tidy Query.pm & Object.t
Bug-37902-QA-follow-up-Tidy-Querypm--Objectt.patch (text/plain), 56.64 KB, created by
Kyle M Hall (khall)
on 2024-09-27 13:25:51 UTC
(
hide
)
Description:
Bug 37902: (QA follow-up) Tidy Query.pm & Object.t
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2024-09-27 13:25:51 UTC
Size:
56.64 KB
patch
obsolete
>From 28fee854a5c6c507d41d07fcd235bcaad970e463 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 27 Sep 2024 13:23:21 +0000 >Subject: [PATCH] Bug 37902: (QA follow-up) Tidy Query.pm & Object.t > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > Koha/REST/Plugin/Query.pm | 176 +++++++------- > t/db_dependent/Koha/Object.t | 453 ++++++++++++++++++----------------- > 2 files changed, 322 insertions(+), 307 deletions(-) > >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index fe3d936df97..99a7906a699 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -19,8 +19,8 @@ use Modern::Perl; > > use Mojo::Base 'Mojolicious::Plugin'; > use List::MoreUtils qw( any ); >-use Scalar::Util qw( reftype ); >-use JSON qw( decode_json ); >+use Scalar::Util qw( reftype ); >+use JSON qw( decode_json ); > > use Koha::Exceptions; > use Koha::Exceptions::REST; >@@ -58,17 +58,15 @@ Generates the DBIC query from the query parameters. > my $filtered_params; > my $path_params; > >- my $reserved_words = _reserved_words(); >- my @query_param_names = keys %{$c->req->params->to_hash}; >+ my $reserved_words = _reserved_words(); >+ my @query_param_names = keys %{ $c->req->params->to_hash }; > > foreach my $param ( keys %{$params} ) { > if ( grep { $param eq $_ } @{$reserved_words} ) { > $reserved_params->{$param} = $params->{$param}; >- } >- elsif ( grep { $param eq $_ } @query_param_names ) { >+ } elsif ( grep { $param eq $_ } @query_param_names ) { > $filtered_params->{$param} = $params->{$param}; >- } >- else { >+ } else { > $path_params->{$param} = $params->{$param}; > } > } >@@ -97,11 +95,12 @@ Generates the DBIC order_by attributes based on I<$params>, and merges into I<$a > ); > my @order_by_params; > >- foreach my $order_by_style ( @order_by_styles ) { >- if ( defined $args->{params}->{$order_by_style} and ref($args->{params}->{$order_by_style}) eq 'ARRAY' ) { >- push( @order_by_params, @{$args->{params}->{$order_by_style} }); >- } >- else { >+ foreach my $order_by_style (@order_by_styles) { >+ if ( defined $args->{params}->{$order_by_style} >+ and ref( $args->{params}->{$order_by_style} ) eq 'ARRAY' ) >+ { >+ push( @order_by_params, @{ $args->{params}->{$order_by_style} } ); >+ } else { > push @order_by_params, $args->{params}->{$order_by_style} > if defined $args->{params}->{$order_by_style}; > } >@@ -109,18 +108,18 @@ Generates the DBIC order_by attributes based on I<$params>, and merges into I<$a > > my @THE_order_by; > >- foreach my $order_by_param ( @order_by_params ) { >+ foreach my $order_by_param (@order_by_params) { > my $order_by; >- $order_by = [ split(/,/, $order_by_param) ] >- if ( !reftype($order_by_param) && index(',',$order_by_param) == -1); >+ $order_by = [ split( /,/, $order_by_param ) ] >+ if ( !reftype($order_by_param) && index( ',', $order_by_param ) == -1 ); > > if ($order_by) { > if ( reftype($order_by) and reftype($order_by) eq 'ARRAY' ) { >- my @order_by = map { _build_order_atom({ string => $_, result_set => $result_set }) } @{ $order_by }; >- push( @THE_order_by, @order_by); >- } >- else { >- push @THE_order_by, _build_order_atom({ string => $order_by, result_set => $result_set }); >+ my @order_by = >+ map { _build_order_atom( { string => $_, result_set => $result_set } ) } @{$order_by}; >+ push( @THE_order_by, @order_by ); >+ } else { >+ push @THE_order_by, _build_order_atom( { string => $order_by, result_set => $result_set } ); > } > } > } >@@ -145,16 +144,16 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in > my ( $c, $args ) = @_; > my $attributes = $args->{attributes}; > my $result_set = $args->{result_set}; >- my $embed = $c->stash('koha.embed'); >+ my $embed = $c->stash('koha.embed'); > return unless defined $embed; > > my @prefetches; >- foreach my $key (sort keys(%{$embed})) { >- my $parsed = _parse_prefetch($key, $embed, $result_set); >+ foreach my $key ( sort keys( %{$embed} ) ) { >+ my $parsed = _parse_prefetch( $key, $embed, $result_set ); > push @prefetches, $parsed if defined $parsed; > } > >- if(scalar(@prefetches)) { >+ if ( scalar(@prefetches) ) { > $attributes->{prefetch} = \@prefetches; > } > } >@@ -195,24 +194,20 @@ is raised. > my $match = $reserved_params->{_match} // 'contains'; > > foreach my $param ( keys %{$filtered_params} ) { >- if ( $match eq 'exact' || $filtered_params->{$param} =~ m[^\d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2}]) { >+ if ( $match eq 'exact' || $filtered_params->{$param} =~ m[^\d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2}] ) { > $params->{$param} = $filtered_params->{$param}; >- } >- elsif ( $match eq 'contains' ) { >+ } elsif ( $match eq 'contains' ) { > $params->{$param} = >- { like => '%' . $filtered_params->{$param} . '%' }; >- } >- elsif ( $match eq 'starts_with' ) { >+ { like => '%' . $filtered_params->{$param} . '%' }; >+ } elsif ( $match eq 'starts_with' ) { > $params->{$param} = { like => $filtered_params->{$param} . '%' }; >- } >- elsif ( $match eq 'ends_with' ) { >+ } elsif ( $match eq 'ends_with' ) { > $params->{$param} = { like => '%' . $filtered_params->{$param} }; >- } >- else { >+ } else { >+ > # We should never reach here, because the OpenAPI plugin should > # prevent invalid params to be passed >- Koha::Exceptions::WrongParameter->throw( >- "Invalid value for _match param ($match)"); >+ Koha::Exceptions::WrongParameter->throw("Invalid value for _match param ($match)"); > } > } > >@@ -235,10 +230,10 @@ Merges parameters from $q_params into $filtered_params. > > $q_params = decode_json($q_params) unless reftype $q_params; > >- my $params = _parse_dbic_query($q_params, $result_set); >+ my $params = _parse_dbic_query( $q_params, $result_set ); > >- return $params unless scalar(keys %{$filtered_params}); >- return {'-and' => [$params, $filtered_params ]}; >+ return $params unless scalar( keys %{$filtered_params} ); >+ return { '-and' => [ $params, $filtered_params ] }; > } > ); > >@@ -264,23 +259,21 @@ Unwraps and stashes the x-koha-embed headers for use later query construction > next unless $param->{name} eq 'x-koha-embed'; > $embed_spec = $param->{items}->{enum}; > } >- Koha::Exceptions::BadParameter->throw( >- "Embedding objects is not allowed on this endpoint.") >- unless defined($embed_spec); >+ Koha::Exceptions::BadParameter->throw("Embedding objects is not allowed on this endpoint.") >+ unless defined($embed_spec); > > if ($embed_header) { > my $THE_embed = {}; > foreach my $embed_req ( split /\s*,\s*/, $embed_header ) { > if ( $embed_req eq '+strings' ) { # special case > $c->stash( 'koha.strings' => 1 ); >- } >- else { >+ } else { > _merge_embed( _parse_embed($embed_req), $THE_embed ); > } > } > > $c->stash( 'koha.embed' => $THE_embed ) >- if $THE_embed; >+ if $THE_embed; > } > > return $c; >@@ -308,7 +301,7 @@ reference: https://metacpan.org/changes/distribution/JSON-Validator#L14 > $app->helper( > 'stash_overrides' => sub { > >- my ( $c ) = @_; >+ my ($c) = @_; > > my $override_header = $c->req->headers->header('x-koha-override') || q{}; > >@@ -349,27 +342,28 @@ according to the following rules: > =cut > > sub _build_order_atom { >- my ( $args ) = @_; >+ my ($args) = @_; > my $string = $args->{string}; > my $result_set = $args->{result_set}; > > my $param = $string; > $param =~ s/^(\+|\-|\s)//; >- if ( $result_set ) { >- my $model_param = _from_api_param($param, $result_set); >+ if ($result_set) { >+ my $model_param = _from_api_param( $param, $result_set ); > $param = $model_param if defined $model_param; > } > >- if ( $string =~ m/^\+/ or >- $string =~ m/^\s/ ) { >+ if ( $string =~ m/^\+/ >+ or $string =~ m/^\s/ ) >+ { > # asc order operator present > return { -asc => $param }; >- } >- elsif ( $string =~ m/^\-/ ) { >+ } elsif ( $string =~ m/^\-/ ) { >+ > # desc order operator present > return { -desc => $param }; >- } >- else { >+ } else { >+ > # no order operator present > return $param; > } >@@ -390,19 +384,16 @@ sub _parse_embed { > my $result; > my ( $curr, $next ) = split /\s*\.\s*/, $string, 2; > >- if ( $next ) { >- $result->{$curr} = { children => _parse_embed( $next ) }; >- } >- else { >+ if ($next) { >+ $result->{$curr} = { children => _parse_embed($next) }; >+ } else { > if ( $curr =~ m/^(?<relation>.*)[\+|:]count/ ) { > my $key = $+{relation} . "_count"; > $result->{$key} = { is_count => 1 }; >- } >- elsif ( $curr =~ m/^(?<relation>.*)\+strings/ ) { >+ } elsif ( $curr =~ m/^(?<relation>.*)\+strings/ ) { > my $key = $+{relation}; > $result->{$key} = { strings => 1 }; >- } >- else { >+ } else { > $result->{$curr} = {}; > } > } >@@ -421,20 +412,21 @@ Merges the hash referenced by I<$parsed_embed> into I<$global_embed>. > sub _merge_embed { > my ( $structure, $embed ) = @_; > >- my ($root) = keys %{ $structure }; >+ my ($root) = keys %{$structure}; >+ >+ if ( any { $root eq $_ } keys %{$embed} ) { > >- if ( any { $root eq $_ } keys %{ $embed } ) { > # Recurse > _merge_embed( $structure->{$root}, $embed->{$root} ); >- } >- else { >+ } else { >+ > # Embed > $embed->{$root} = $structure->{$root}; > } > } > > sub _parse_prefetch { >- my ( $key, $embed, $result_set) = @_; >+ my ( $key, $embed, $result_set ) = @_; > > my $pref_key = $key; > $pref_key =~ s/_count$// if $embed->{$key}->{is_count}; >@@ -444,26 +436,26 @@ sub _parse_prefetch { > return $pref_key unless defined $embed->{$key}->{children} && defined $ko_class; > > my @prefetches; >- foreach my $child (sort keys(%{$embed->{$key}->{children}})) { >- my $parsed = _parse_prefetch($child, $embed->{$key}->{children}, $ko_class->new); >+ foreach my $child ( sort keys( %{ $embed->{$key}->{children} } ) ) { >+ my $parsed = _parse_prefetch( $child, $embed->{$key}->{children}, $ko_class->new ); > push @prefetches, $parsed if defined $parsed; > } > > return $pref_key unless scalar(@prefetches); > >- return {$pref_key => $prefetches[0]} if scalar(@prefetches) eq 1; >+ return { $pref_key => $prefetches[0] } if scalar(@prefetches) eq 1; > >- return {$pref_key => \@prefetches}; >+ return { $pref_key => \@prefetches }; > } > > sub _from_api_param { >- my ($key, $result_set) = @_; >+ my ( $key, $result_set ) = @_; > >- if($key =~ /\./) { >+ if ( $key =~ /\./ ) { > >- my ($curr, $next) = split /\s*\.\s*/, $key, 2; >+ my ( $curr, $next ) = split /\s*\.\s*/, $key, 2; > >- return $curr.'.'._from_api_param($next, $result_set) if $curr eq 'me'; >+ return $curr . '.' . _from_api_param( $next, $result_set ) if $curr eq 'me'; > > my $ko_class = $result_set->prefetch_whitelist->{$curr}; > >@@ -472,10 +464,16 @@ sub _from_api_param { > > $result_set = $ko_class->new; > >- if ($next =~ /\./) { >- return _from_api_param($next, $result_set); >+ if ( $next =~ /\./ ) { >+ return _from_api_param( $next, $result_set ); > } else { >- return $curr.'.'.($result_set->from_api_mapping && defined $result_set->from_api_mapping->{$next} ? $result_set->from_api_mapping->{$next}:$next); >+ return >+ $curr . '.' >+ . ( >+ $result_set->from_api_mapping && defined $result_set->from_api_mapping->{$next} >+ ? $result_set->from_api_mapping->{$next} >+ : $next >+ ); > } > } else { > return defined $result_set->from_api_mapping->{$key} ? $result_set->from_api_mapping->{$key} : $key; >@@ -483,21 +481,21 @@ sub _from_api_param { > } > > sub _parse_dbic_query { >- my ($q_params, $result_set) = @_; >+ my ( $q_params, $result_set ) = @_; > >- if(reftype($q_params) && reftype($q_params) eq 'HASH') { >+ if ( reftype($q_params) && reftype($q_params) eq 'HASH' ) { > my $parsed_hash; >- foreach my $key (keys %{$q_params}) { >- if($key =~ /-?(not_?)?bool/i ) { >- $parsed_hash->{$key} = _from_api_param($q_params->{$key}, $result_set); >+ foreach my $key ( keys %{$q_params} ) { >+ if ( $key =~ /-?(not_?)?bool/i ) { >+ $parsed_hash->{$key} = _from_api_param( $q_params->{$key}, $result_set ); > next; > } >- my $k = _from_api_param($key, $result_set); >- $parsed_hash->{$k} = _parse_dbic_query($q_params->{$key}, $result_set); >+ my $k = _from_api_param( $key, $result_set ); >+ $parsed_hash->{$k} = _parse_dbic_query( $q_params->{$key}, $result_set ); > } > return $parsed_hash; >- } elsif (reftype($q_params) && reftype($q_params) eq 'ARRAY') { >- my @mapped = map{ _parse_dbic_query($_, $result_set) } @$q_params; >+ } elsif ( reftype($q_params) && reftype($q_params) eq 'ARRAY' ) { >+ my @mapped = map { _parse_dbic_query( $_, $result_set ) } @$q_params; > return \@mapped; > } else { > return $q_params; >diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t >index fe2c4ff54df..9f751c80b3b 100755 >--- a/t/db_dependent/Koha/Object.t >+++ b/t/db_dependent/Koha/Object.t >@@ -23,7 +23,7 @@ use Test::Warn; > > use C4::Context; > use C4::Circulation qw( AddIssue ); >-use C4::Biblio qw( AddBiblio ); >+use C4::Biblio qw( AddBiblio ); > > use Koha::Database; > >@@ -56,12 +56,12 @@ subtest 'is_changed / make_column_dirty' => sub { > > $schema->storage->txn_begin; > >- my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; >- my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >+ my $categorycode = $builder->build( { source => 'Category' } )->{categorycode}; >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > > my $object = Koha::Patron->new(); >- $object->categorycode( $categorycode ); >- $object->branchcode( $branchcode ); >+ $object->categorycode($categorycode); >+ $object->branchcode($branchcode); > $object->surname("Test Surname"); > $object->store->discard_changes; > is( $object->is_changed(), 0, "Object is unchanged" ); >@@ -73,15 +73,15 @@ subtest 'is_changed / make_column_dirty' => sub { > $object->store(); > is( $object->is_changed(), 0, "Object no longer marked as changed after being stored" ); > >- $object->set({ firstname => 'Test Firstname' }); >+ $object->set( { firstname => 'Test Firstname' } ); > is( $object->is_changed(), 1, "Object is changed after Set" ); > $object->store(); > is( $object->is_changed(), 0, "Object no longer marked as changed after being stored" ); > > # Test make_column_dirty > is( $object->make_column_dirty('firstname'), '', 'make_column_dirty returns empty string on success' ); >- is( $object->make_column_dirty('firstname'), 1, 'make_column_dirty returns 1 if already dirty' ); >- is( $object->is_changed, 1, "Object is changed after make dirty" ); >+ is( $object->make_column_dirty('firstname'), 1, 'make_column_dirty returns 1 if already dirty' ); >+ is( $object->is_changed, 1, "Object is changed after make dirty" ); > $object->store; > is( $object->is_changed, 0, "Store clears dirty mark" ); > $object->make_column_dirty('firstname'); >@@ -96,13 +96,13 @@ subtest 'in_storage' => sub { > > $schema->storage->txn_begin; > >- my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; >- my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >+ my $categorycode = $builder->build( { source => 'Category' } )->{categorycode}; >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > > my $object = Koha::Patron->new(); > is( $object->in_storage, 0, "Object is not in storage" ); >- $object->categorycode( $categorycode ); >- $object->branchcode( $branchcode ); >+ $object->categorycode($categorycode); >+ $object->branchcode($branchcode); > $object->surname("Test Surname"); > $object->store(); > is( $object->in_storage, 1, "Object is now stored" ); >@@ -110,12 +110,12 @@ subtest 'in_storage' => sub { > is( $object->in_storage, 1 ); > > my $borrowernumber = $object->borrowernumber; >- my $patron = $schema->resultset('Borrower')->find( $borrowernumber ); >+ my $patron = $schema->resultset('Borrower')->find($borrowernumber); > is( $patron->surname(), "Test Surname", "Object found in database" ); > > $object->delete(); >- $patron = $schema->resultset('Borrower')->find( $borrowernumber ); >- ok( ! $patron, "Object no longer found in database" ); >+ $patron = $schema->resultset('Borrower')->find($borrowernumber); >+ ok( !$patron, "Object no longer found in database" ); > is( $object->in_storage, 0, "Object is not in storage" ); > > $schema->storage->txn_rollback; >@@ -126,10 +126,10 @@ subtest 'id' => sub { > > $schema->storage->txn_begin; > >- my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; >- my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >+ my $categorycode = $builder->build( { source => 'Category' } )->{categorycode}; >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > >- my $patron = Koha::Patron->new({categorycode => $categorycode, branchcode => $branchcode })->store; >+ my $patron = Koha::Patron->new( { categorycode => $categorycode, branchcode => $branchcode } )->store; > is( $patron->id, $patron->borrowernumber ); > > $schema->storage->txn_rollback; >@@ -140,11 +140,14 @@ subtest 'get_column' => sub { > > $schema->storage->txn_begin; > >- my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; >- my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >+ my $categorycode = $builder->build( { source => 'Category' } )->{categorycode}; >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > >- my $patron = Koha::Patron->new({categorycode => $categorycode, branchcode => $branchcode })->store; >- is( $patron->get_column('borrowernumber'), $patron->borrowernumber, 'get_column should retrieve the correct value' ); >+ my $patron = Koha::Patron->new( { categorycode => $categorycode, branchcode => $branchcode } )->store; >+ is( >+ $patron->get_column('borrowernumber'), $patron->borrowernumber, >+ 'get_column should retrieve the correct value' >+ ); > > $schema->storage->txn_rollback; > }; >@@ -173,20 +176,25 @@ subtest 'TO_JSON tests' => sub { > > $schema->storage->txn_begin; > >- my $dt = dt_from_string(); >+ my $dt = dt_from_string(); > my $borrowernumber = $builder->build( >- { source => 'Borrower', >- value => { lost => 1, >- sms_provider_id => undef, >- gonenoaddress => 0, >- updated_on => $dt, >- lastseen => $dt, } })->{borrowernumber}; >- >- my $patron = Koha::Patrons->find($borrowernumber); >- my $lost = $patron->TO_JSON()->{lost}; >+ { >+ source => 'Borrower', >+ value => { >+ lost => 1, >+ sms_provider_id => undef, >+ gonenoaddress => 0, >+ updated_on => $dt, >+ lastseen => $dt, >+ } >+ } >+ )->{borrowernumber}; >+ >+ my $patron = Koha::Patrons->find($borrowernumber); >+ my $lost = $patron->TO_JSON()->{lost}; > my $gonenoaddress = $patron->TO_JSON->{gonenoaddress}; >- my $updated_on = $patron->TO_JSON->{updated_on}; >- my $lastseen = $patron->TO_JSON->{lastseen}; >+ my $updated_on = $patron->TO_JSON->{updated_on}; >+ my $lastseen = $patron->TO_JSON->{lastseen}; > > ok( $lost->isa('JSON::PP::Boolean'), 'Boolean attribute type is correct' ); > is( $lost, 1, 'Boolean attribute value is correct (true)' ); >@@ -196,7 +204,7 @@ subtest 'TO_JSON tests' => sub { > > is( $patron->TO_JSON->{sms_provider_id}, undef, 'Undef values should not be casted to 0' ); > >- ok( !isvstring($patron->borrowernumber), 'Integer values are not coded as strings' ); >+ ok( !isvstring( $patron->borrowernumber ), 'Integer values are not coded as strings' ); > > my $rfc3999_regex = qr/ > (?<year>\d{4}) >@@ -212,11 +220,11 @@ subtest 'TO_JSON tests' => sub { > (?<second>\d{2}) > (([Zz])|([\+|\-]([01][0-9]|2[0-3]):[0-5][0-9])) > /xms; >- like( $updated_on, $rfc3999_regex, "Date-time $updated_on formatted correctly"); >- like( $lastseen, $rfc3999_regex, "Date-time $lastseen formatted correctly"); >+ like( $updated_on, $rfc3999_regex, "Date-time $updated_on formatted correctly" ); >+ like( $lastseen, $rfc3999_regex, "Date-time $lastseen formatted correctly" ); > > # Test JSON doesn't receive strings >- my $order = $builder->build_object({ class => 'Koha::Acquisition::Orders' }); >+ my $order = $builder->build_object( { class => 'Koha::Acquisition::Orders' } ); > $order = Koha::Acquisition::Orders->find( $order->ordernumber ); > is_deeply( $order->TO_JSON, decode_json( encode_json( $order->TO_JSON ) ), 'Orders are similar' ); > >@@ -229,7 +237,7 @@ subtest "to_api() tests" => sub { > > $schema->storage->txn_begin; > >- my $city = $builder->build_object({ class => 'Koha::Cities' }); >+ my $city = $builder->build_object( { class => 'Koha::Cities' } ); > > # THE mapping > # cityid => 'city_id', >@@ -248,7 +256,8 @@ subtest "to_api() tests" => sub { > > # Lets emulate an undef > my $city_class = Test::MockModule->new('Koha::City'); >- $city_class->mock( 'to_api_mapping', >+ $city_class->mock( >+ 'to_api_mapping', > sub { > return { > cityid => 'city_id', >@@ -262,45 +271,41 @@ subtest "to_api() tests" => sub { > > $api_city = $city->to_api; > >- is( $api_city->{city_id}, $city->cityid, 'Attribute translated correctly' ); >- is( $api_city->{country}, $city->city_country, 'Attribute translated correctly' ); >- is( $api_city->{name}, $city->city_name, 'Attribute translated correctly' ); >- is( $api_city->{state}, $city->city_state, 'Attribute translated correctly' ); >+ is( $api_city->{city_id}, $city->cityid, 'Attribute translated correctly' ); >+ is( $api_city->{country}, $city->city_country, 'Attribute translated correctly' ); >+ is( $api_city->{name}, $city->city_name, 'Attribute translated correctly' ); >+ is( $api_city->{state}, $city->city_state, 'Attribute translated correctly' ); > ok( !exists $api_city->{postal_code}, 'Attribute removed' ); > > # Pick a class that won't have a mapping for the API >- my $action_log = $builder->build_object({ class => 'Koha::ActionLogs' }); >+ my $action_log = $builder->build_object( { class => 'Koha::ActionLogs' } ); > is_deeply( $action_log->to_api, $action_log->TO_JSON, 'If no overloaded to_api_mapping method, return TO_JSON' ); > > my $biblio = $builder->build_sample_biblio(); >- my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); >- my $hold = $builder->build_object({ class => 'Koha::Holds', value => { itemnumber => $item->itemnumber } }); >+ my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); >+ my $hold = $builder->build_object( { class => 'Koha::Holds', value => { itemnumber => $item->itemnumber } } ); > > my $embeds = { 'items' => {} }; > >- my $biblio_api = $biblio->to_api({ embed => $embeds }); >+ my $biblio_api = $biblio->to_api( { embed => $embeds } ); > >- ok(exists $biblio_api->{items}, 'Items where embedded in biblio results'); >- is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item matches'); >- ok(!exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet'); >+ ok( exists $biblio_api->{items}, 'Items where embedded in biblio results' ); >+ is( $biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item matches' ); >+ ok( !exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet' ); > > $embeds = ( > { >- 'items' => { >- 'children' => { >- 'holds' => {} >- } >- }, >+ 'items' => { 'children' => { 'holds' => {} } }, > 'biblioitem' => {} > } > ); >- $biblio_api = $biblio->to_api({ embed => $embeds }); >+ $biblio_api = $biblio->to_api( { embed => $embeds } ); > >- ok(exists $biblio_api->{items}, 'Items where embedded in biblio results'); >- is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item still matches'); >- ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded'); >- is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->reserve_id, 'Hold matches'); >- is_deeply($biblio_api->{biblioitem}, $biblio->biblioitem->to_api, 'More than one root'); >+ ok( exists $biblio_api->{items}, 'Items where embedded in biblio results' ); >+ is( $biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item still matches' ); >+ ok( exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded' ); >+ is( $biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->reserve_id, 'Hold matches' ); >+ is_deeply( $biblio_api->{biblioitem}, $biblio->biblioitem->to_api, 'More than one root' ); > > my $_strings = { > location => { >@@ -319,13 +324,9 @@ subtest "to_api() tests" => sub { > } > ); > >- my $hold_api = $hold->to_api( >- { >- embed => { 'item' => { strings => 1 } } >- } >- ); >+ my $hold_api = $hold->to_api( { embed => { 'item' => { strings => 1 } } } ); > >- is( ref($hold_api->{item}), 'HASH', 'Single nested object works correctly' ); >+ is( ref( $hold_api->{item} ), 'HASH', 'Single nested object works correctly' ); > is( $hold_api->{item}->{item_id}, $item->itemnumber, 'Object embedded correctly' ); > is_deeply( > $hold_api->{item}->{_strings}, >@@ -334,27 +335,28 @@ subtest "to_api() tests" => sub { > ); > > # biblio with no items >- my $new_biblio = $builder->build_sample_biblio; >- my $new_biblio_api = $new_biblio->to_api({ embed => $embeds }); >+ my $new_biblio = $builder->build_sample_biblio; >+ my $new_biblio_api = $new_biblio->to_api( { embed => $embeds } ); > > is_deeply( $new_biblio_api->{items}, [], 'Empty list if no items' ); > > my $biblio_class = Test::MockModule->new('Koha::Biblio'); > $biblio_class->mock( 'undef_result', sub { return; } ); > >- $new_biblio_api = $new_biblio->to_api({ embed => ( { 'undef_result' => {} } ) }); >+ $new_biblio_api = $new_biblio->to_api( { embed => ( { 'undef_result' => {} } ) } ); > ok( exists $new_biblio_api->{undef_result}, 'If a method returns undef, then the attribute is defined' ); > is( $new_biblio_api->{undef_result}, undef, 'If a method returns undef, then the attribute is undef' ); > >- $biblio_class->mock( 'items', >- sub { return [ bless { itemnumber => 1 }, 'Somethings' ]; } ); >+ $biblio_class->mock( >+ 'items', >+ sub { return [ bless { itemnumber => 1 }, 'Somethings' ]; } >+ ); > > throws_ok { >- $new_biblio_api = $new_biblio->to_api( >- { embed => { 'items' => { children => { asd => {} } } } } ); >+ $new_biblio_api = $new_biblio->to_api( { embed => { 'items' => { children => { asd => {} } } } } ); > } > 'Koha::Exception', >-"An exception is thrown if a blessed object to embed doesn't implement to_api"; >+ "An exception is thrown if a blessed object to embed doesn't implement to_api"; > > is( > $@->message, >@@ -362,7 +364,6 @@ subtest "to_api() tests" => sub { > "Exception message correct" > ); > >- > my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); > $builder->build_object( > { >@@ -401,16 +402,16 @@ subtest "to_api() tests" => sub { > > subtest 'public request tests' => sub { > >- my @all_attrs = Koha::Libraries->columns(); >+ my @all_attrs = Koha::Libraries->columns(); > my $public_attrs = { map { $_ => 1 } @{ Koha::Library->public_read_list() } }; >- my $mapping = Koha::Library->to_api_mapping; >+ my $mapping = Koha::Library->to_api_mapping; > > plan tests => scalar @all_attrs * 2; > > # Create a sample library > my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > >- my $unprivileged_representation = $library->to_api({ public => 1 }); >+ my $unprivileged_representation = $library->to_api( { public => 1 } ); > my $privileged_representation = $library->to_api; > > foreach my $attr (@all_attrs) { >@@ -425,15 +426,13 @@ subtest "to_api() tests" => sub { > exists $unprivileged_representation->{$mapped}, > "Attribute '$attr' is present when public" > ); >- } >- else { >+ } else { > ok( > !exists $unprivileged_representation->{$mapped}, > "Attribute '$attr' is not present when public" > ); > } >- } >- else { >+ } else { > ok( > !exists $privileged_representation->{$attr}, > "Unmapped attribute '$attr' is not present when privileged" >@@ -453,10 +452,12 @@ subtest "to_api() tests" => sub { > $schema->storage->txn_begin; > > # new category >- my $category = $builder->build_object({ class => 'Koha::AuthorisedValueCategories' }); >+ my $category = $builder->build_object( { class => 'Koha::AuthorisedValueCategories' } ); >+ > # add two countries > my $argentina = $builder->build_object( >- { class => 'Koha::AuthorisedValues', >+ { >+ class => 'Koha::AuthorisedValues', > value => { > category => $category->category_name, > lib => 'AR (Argentina)', >@@ -465,7 +466,8 @@ subtest "to_api() tests" => sub { > } > ); > my $france = $builder->build_object( >- { class => 'Koha::AuthorisedValues', >+ { >+ class => 'Koha::AuthorisedValues', > value => { > category => $category->category_name, > lib => 'FR (France)', >@@ -499,12 +501,14 @@ subtest "to_api() tests" => sub { > $city_mock->mock( 'public_read_list', sub { return [ 'city_id', 'city_country', 'city_name', 'city_state' ] } ); > > my $cordoba = $builder->build_object( >- { class => 'Koha::Cities', >+ { >+ class => 'Koha::Cities', > value => { city_country => $argentina->authorised_value, city_name => 'Cordoba' } > } > ); > my $marseille = $builder->build_object( >- { class => 'Koha::Cities', >+ { >+ class => 'Koha::Cities', > value => { city_country => $france->authorised_value, city_name => 'Marseille' } > } > ); >@@ -621,7 +625,7 @@ subtest "to_api_mapping() tests" => sub { > > $schema->storage->txn_begin; > >- my $action_log = $builder->build_object({ class => 'Koha::ActionLogs' }); >+ my $action_log = $builder->build_object( { class => 'Koha::ActionLogs' } ); > is_deeply( $action_log->to_api_mapping, {}, 'If no to_api_mapping present, return empty hashref' ); > > $schema->storage->txn_rollback; >@@ -633,11 +637,12 @@ subtest "from_api_mapping() tests" => sub { > > $schema->storage->txn_begin; > >- my $city = $builder->build_object({ class => 'Koha::Cities' }); >+ my $city = $builder->build_object( { class => 'Koha::Cities' } ); > > # Lets emulate an undef > my $city_class = Test::MockModule->new('Koha::City'); >- $city_class->mock( 'to_api_mapping', >+ $city_class->mock( >+ 'to_api_mapping', > sub { > return { > cityid => 'city_id', >@@ -656,8 +661,9 @@ subtest "from_api_mapping() tests" => sub { > 'Mapping returns correctly, undef ommited' > ); > >- $city_class->unmock( 'to_api_mapping'); >- $city_class->mock( 'to_api_mapping', >+ $city_class->unmock('to_api_mapping'); >+ $city_class->mock( >+ 'to_api_mapping', > sub { > return { > cityid => 'city_id', >@@ -677,7 +683,7 @@ subtest "from_api_mapping() tests" => sub { > ); > > # Get a fresh object >- $city = $builder->build_object({ class => 'Koha::Cities' }); >+ $city = $builder->build_object( { class => 'Koha::Cities' } ); > is_deeply( > $city->from_api_mapping, > { >@@ -688,22 +694,22 @@ subtest "from_api_mapping() tests" => sub { > 'Fresh mapping loaded' > ); > >- $city_class->unmock( 'to_api_mapping'); >+ $city_class->unmock('to_api_mapping'); > $city_class->mock( 'to_api_mapping', undef ); > > # Get a fresh object >- $city = $builder->build_object({ class => 'Koha::Cities' }); >+ $city = $builder->build_object( { class => 'Koha::Cities' } ); > is_deeply( > $city->from_api_mapping, > {}, > 'No to_api_mapping then empty hashref' > ); > >- $city_class->unmock( 'to_api_mapping'); >+ $city_class->unmock('to_api_mapping'); > $city_class->mock( 'to_api_mapping', sub { return; } ); > > # Get a fresh object >- $city = $builder->build_object({ class => 'Koha::Cities' }); >+ $city = $builder->build_object( { class => 'Koha::Cities' } ); > is_deeply( > $city->from_api_mapping, > {}, >@@ -719,19 +725,19 @@ subtest 'set_from_api() tests' => sub { > > $schema->storage->txn_begin; > >- my $city = $builder->build_object({ class => 'Koha::Cities' }); >+ my $city = $builder->build_object( { class => 'Koha::Cities' } ); > my $city_unblessed = $city->unblessed; >- my $attrs = { >+ my $attrs = { > name => 'Cordoba', > country => 'Argentina', > postal_code => '5000' > }; > $city->set_from_api($attrs); > >- is( $city->city_state, $city_unblessed->{city_state}, 'Untouched attributes are preserved' ); >- is( $city->city_name, $attrs->{name}, 'city_name updated correctly' ); >- is( $city->city_country, $attrs->{country}, 'city_country updated correctly' ); >- is( $city->city_zipcode, $attrs->{postal_code}, 'city_zipcode updated correctly' ); >+ is( $city->city_state, $city_unblessed->{city_state}, 'Untouched attributes are preserved' ); >+ is( $city->city_name, $attrs->{name}, 'city_name updated correctly' ); >+ is( $city->city_country, $attrs->{country}, 'city_country updated correctly' ); >+ is( $city->city_zipcode, $attrs->{postal_code}, 'city_zipcode updated correctly' ); > > $schema->storage->txn_rollback; > }; >@@ -749,9 +755,9 @@ subtest 'new_from_api() tests' => sub { > }; > my $city = Koha::City->new_from_api($attrs); > >- is( ref($city), 'Koha::City', 'Object type is correct' ); >- is( $city->city_name, $attrs->{name}, 'city_name updated correctly' ); >- is( $city->city_country, $attrs->{country}, 'city_country updated correctly' ); >+ is( ref($city), 'Koha::City', 'Object type is correct' ); >+ is( $city->city_name, $attrs->{name}, 'city_name updated correctly' ); >+ is( $city->city_country, $attrs->{country}, 'city_country updated correctly' ); > is( $city->city_zipcode, $attrs->{postal_code}, 'city_zipcode updated correctly' ); > > $schema->storage->txn_rollback; >@@ -773,30 +779,36 @@ subtest 'attributes_from_api() tests' => sub { > > my $attrs = $patron->attributes_from_api( > { >- updated_on => '2019-12-27T14:53:00Z', >- last_seen => '2019-12-27T14:53:00Z', >- date_of_birth => '2019-12-27', >+ updated_on => '2019-12-27T14:53:00Z', >+ last_seen => '2019-12-27T14:53:00Z', >+ date_of_birth => '2019-12-27', > } > ); > >- ok( exists $attrs->{updated_on}, >- 'No translation takes place if no mapping' ); >+ ok( >+ exists $attrs->{updated_on}, >+ 'No translation takes place if no mapping' >+ ); > is( > $attrs->{updated_on}, > '2019-12-27 14:53:00', > 'Given an rfc3339 formatted datetime string, a timestamp field is converted into an SQL formatted datetime string' > ); > >- ok( exists $attrs->{lastseen}, >- 'Translation takes place because of the defined mapping' ); >+ ok( >+ exists $attrs->{lastseen}, >+ 'Translation takes place because of the defined mapping' >+ ); > is( > $attrs->{lastseen}, > '2019-12-27 14:53:00', > 'Given an rfc3339 formatted datetime string, a datetime field is converted into an SQL formatted datetime string' > ); > >- ok( exists $attrs->{dateofbirth}, >- 'Translation takes place because of the defined mapping' ); >+ ok( >+ exists $attrs->{dateofbirth}, >+ 'Translation takes place because of the defined mapping' >+ ); > is( > $attrs->{dateofbirth}, > '2019-12-27', >@@ -820,43 +832,46 @@ subtest 'attributes_from_api() tests' => sub { > }, > { '=' => '2019-12-31 23:59:00' } > ], >- updated_on => { '>' => '2019-12-27 14:53:00' }, >- dateofbirth => [ { '>', '2019-12-27'}, { '=' => '2019-12-31' } ] >+ updated_on => { '>' => '2019-12-27 14:53:00' }, >+ dateofbirth => [ { '>', '2019-12-27' }, { '=' => '2019-12-31' } ] > } > ); > > $attrs = $patron->attributes_from_api( > { >- last_seen => undef, >- date_of_birth => undef, >+ last_seen => undef, >+ date_of_birth => undef, > } > ); > >- ok( exists $attrs->{lastseen}, >- 'undef parameter is not skipped (Bug 29157)' ); >+ ok( >+ exists $attrs->{lastseen}, >+ 'undef parameter is not skipped (Bug 29157)' >+ ); > is( > $attrs->{lastseen}, > undef, > 'Given undef, a datetime field is set to undef (Bug 29157)' > ); > >- ok( exists $attrs->{dateofbirth}, >- 'undef parameter is not skipped (Bug 29157)' ); >+ ok( >+ exists $attrs->{dateofbirth}, >+ 'undef parameter is not skipped (Bug 29157)' >+ ); > is( > $attrs->{dateofbirth}, > undef, > 'Given undef, a date field is set to undef (Bug 29157)' > ); > >- throws_ok >- { >- $attrs = $patron->attributes_from_api( >- { >- date_of_birth => '20141205', >- } >- ); >- } >- 'Koha::Exceptions::BadParameter', >+ throws_ok { >+ $attrs = $patron->attributes_from_api( >+ { >+ date_of_birth => '20141205', >+ } >+ ); >+ } >+ 'Koha::Exceptions::BadParameter', > 'Bad date throws an exception'; > > is( >@@ -894,23 +909,26 @@ subtest "Test update method" => sub { > > $schema->storage->txn_begin; > >- my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; >- my $library = Koha::Libraries->find( $branchcode ); >- $library->update({ branchname => 'New_Name', branchcity => 'AMS' }); >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; >+ my $library = Koha::Libraries->find($branchcode); >+ $library->update( { branchname => 'New_Name', branchcity => 'AMS' } ); > is( $library->branchname, 'New_Name', 'Changed name with update' ); >- is( $library->branchcity, 'AMS', 'Changed city too' ); >- is( $library->is_changed, 0, 'Change should be stored already' ); >+ is( $library->branchcity, 'AMS', 'Changed city too' ); >+ is( $library->is_changed, 0, 'Change should be stored already' ); > try { >- $library->update({ >- branchcity => 'NYC', not_a_column => 53, branchname => 'Name3', >- }); >- fail( 'It should not be possible to update an unexisting column without an error from Koha::Object/DBIx' ); >+ $library->update( >+ { >+ branchcity => 'NYC', not_a_column => 53, branchname => 'Name3', >+ } >+ ); >+ fail('It should not be possible to update an unexisting column without an error from Koha::Object/DBIx'); > } catch { > ok( $_->isa('Koha::Exceptions::Object'), 'Caught error when updating wrong column' ); >- $library->discard_changes; #requery after failing update >+ $library->discard_changes; #requery after failing update > }; >+ > # Check if the columns are not updated >- is( $library->branchcity, 'AMS', 'First column not updated' ); >+ is( $library->branchcity, 'AMS', 'First column not updated' ); > is( $library->branchname, 'New_Name', 'Third column not updated' ); > > $schema->storage->txn_rollback; >@@ -926,14 +944,14 @@ subtest 'store() tests' => sub { > $schema->storage->txn_begin; > > # Create a library to make sure its ID doesn't exist on the DB >- my $library = $builder->build_object({ class => 'Koha::Libraries' }); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > my $branchcode = $library->branchcode; > $library->delete; > > my $library_group = Koha::Library::Group->new( > { >- branchcode => $library->branchcode, >- title => 'a title', >+ branchcode => $library->branchcode, >+ title => 'a title', > } > ); > >@@ -941,9 +959,8 @@ subtest 'store() tests' => sub { > { > local *STDERR; > open STDERR, '>', '/dev/null'; >- throws_ok >- { $library_group->store } >- 'Koha::Exceptions::Object::FKConstraint', >+ throws_ok { $library_group->store } >+ 'Koha::Exceptions::Object::FKConstraint', > 'Exception is thrown correctly'; > is( > $@->message, >@@ -956,18 +973,17 @@ subtest 'store() tests' => sub { > 'Exception field is correct' > ); > >- $library_group = $builder->build_object({ class => 'Koha::Library::Groups' }); >+ $library_group = $builder->build_object( { class => 'Koha::Library::Groups' } ); > > my $new_library_group = Koha::Library::Group->new( > { >- branchcode => $library_group->branchcode, >- title => $library_group->title, >+ branchcode => $library_group->branchcode, >+ title => $library_group->title, > } > ); > >- throws_ok >- { $new_library_group->store } >- 'Koha::Exceptions::Object::DuplicateID', >+ throws_ok { $new_library_group->store } >+ 'Koha::Exceptions::Object::DuplicateID', > 'Exception is thrown correctly'; > > is( >@@ -977,15 +993,15 @@ subtest 'store() tests' => sub { > ); > > like( >- $@->duplicate_id, >- qr/(library_groups\.)?title/, >- 'Exception field is correct (note that MySQL 8 is displaying the tablename)' >+ $@->duplicate_id, >+ qr/(library_groups\.)?title/, >+ 'Exception field is correct (note that MySQL 8 is displaying the tablename)' > ); > close STDERR; > } > > # Successful test >- $library_group->set({ title => 'Manuel' }); >+ $library_group->set( { title => 'Manuel' } ); > my $ret = $library_group->store; > is( ref($ret), 'Koha::Library::Group', 'store() returns the object on success' ); > >@@ -1002,47 +1018,46 @@ subtest 'store() tests' => sub { > { > categorycode => $patron_category->categorycode, > branchcode => $library->branchcode, >- dateofbirth => "", # date will be set to NULL >- sms_provider_id => "", # Integer will be set to NULL >- privacy => "", # privacy cannot be NULL but has a default value >+ dateofbirth => "", # date will be set to NULL >+ sms_provider_id => "", # Integer will be set to NULL >+ privacy => "", # privacy cannot be NULL but has a default value > } > )->store; > }; >- is( $@, '', 'No error should be raised by ->store if empty strings are passed' ); >- is( $patron->privacy, 1, 'Default value for privacy should be set to 1' ); >- is( $patron->dateofbirth, undef, 'dateofbirth must have been set to undef'); >- is( $patron->sms_provider_id, undef, 'sms_provider_id must have been set to undef'); >+ is( $@, '', 'No error should be raised by ->store if empty strings are passed' ); >+ is( $patron->privacy, 1, 'Default value for privacy should be set to 1' ); >+ is( $patron->dateofbirth, undef, 'dateofbirth must have been set to undef' ); >+ is( $patron->sms_provider_id, undef, 'sms_provider_id must have been set to undef' ); > > my $itemtype = eval { > Koha::ItemType->new( > { >- itemtype => 'IT4test', >- rentalcharge => "", >- notforloan => "", >- hideinopac => "", >+ itemtype => 'IT4test', >+ rentalcharge => "", >+ notforloan => "", >+ hideinopac => "", > } > )->store; > }; >- is( $@, '', 'No error should be raised by ->store if empty strings are passed' ); >- is( $itemtype->rentalcharge, undef, 'decimal DEFAULT NULL should default to null'); >- is( $itemtype->hideinopac, 0, 'int NOT NULL DEFAULT 0 should default to 0'); >+ is( $@, '', 'No error should be raised by ->store if empty strings are passed' ); >+ is( $itemtype->rentalcharge, undef, 'decimal DEFAULT NULL should default to null' ); >+ is( $itemtype->hideinopac, 0, 'int NOT NULL DEFAULT 0 should default to 0' ); > > my $currency = eval { > Koha::Acquisition::Currency->new( > { >- currency => 'Cur_test', >- active => 'IT4test', >+ currency => 'Cur_test', >+ active => 'IT4test', > } > )->store; > }; >- is( $currency->active, undef, 'int DEFAULT NULL should default to null'); >+ is( $currency->active, undef, 'int DEFAULT NULL should default to null' ); > > subtest 'Bad value tests' => sub { > > plan tests => 3; > >- my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >- >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); > > try { > local *STDERR; >@@ -1051,7 +1066,8 @@ subtest 'store() tests' => sub { > close STDERR; > } catch { > ok( $_->isa('Koha::Exceptions::Object::BadValue'), 'Exception thrown correctly' ); >- like( $_->property, qr/(borrowers\.)?lastseen/, 'Column should be the expected one' ); # The table name is not always displayed, it depends on the DBMS version >+ like( $_->property, qr/(borrowers\.)?lastseen/, 'Column should be the expected one' ) >+ ; # The table name is not always displayed, it depends on the DBMS version > is( $_->value, 'wrong_value', 'Value should be the expected one' ); > }; > }; >@@ -1066,7 +1082,7 @@ subtest 'unblessed_all_relateds' => sub { > > # FIXME It's very painful to create an issue in tests! > my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >- t::lib::Mocks::mock_userenv({ branchcode => $library->branchcode }); >+ t::lib::Mocks::mock_userenv( { branchcode => $library->branchcode } ); > > my $patron_category = $builder->build( > { >@@ -1079,16 +1095,16 @@ subtest 'unblessed_all_relateds' => sub { > } > ); > my $patron_data = { >- firstname => 'firstname', >- surname => 'surname', >+ firstname => 'firstname', >+ surname => 'surname', > categorycode => $patron_category->{categorycode}, >- branchcode => $library->branchcode, >+ branchcode => $library->branchcode, > }; >- my $patron = Koha::Patron->new($patron_data)->store; >+ my $patron = Koha::Patron->new($patron_data)->store; > my ($biblionumber) = AddBiblio( MARC::Record->new, '' ); >- my $biblio = Koha::Biblios->find( $biblionumber ); >- my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; >- my $item = $builder->build_object( >+ my $biblio = Koha::Biblios->find($biblionumber); >+ my $itemtype = $builder->build( { source => 'Itemtype' } )->{itemtype}; >+ my $item = $builder->build_object( > { > class => 'Koha::Items', > value => { >@@ -1102,11 +1118,11 @@ subtest 'unblessed_all_relateds' => sub { > } > ); > >- my $issue = AddIssue( $patron, $item->barcode, dt_from_string()->subtract( days => 1 ) ); >- my $overdues = Koha::Patrons->find( $patron->id )->overdues; # Koha::Patron->overdues prefetches >- my $overdue = $overdues->next->unblessed_all_relateds; >- is( $overdue->{issue_id}, $issue->issue_id, 'unblessed_all_relateds has field from the original table (issues)' ); >- is( $overdue->{title}, $biblio->title, 'unblessed_all_relateds has field from other tables (biblio)' ); >+ my $issue = AddIssue( $patron, $item->barcode, dt_from_string()->subtract( days => 1 ) ); >+ my $overdues = Koha::Patrons->find( $patron->id )->overdues; # Koha::Patron->overdues prefetches >+ my $overdue = $overdues->next->unblessed_all_relateds; >+ is( $overdue->{issue_id}, $issue->issue_id, 'unblessed_all_relateds has field from the original table (issues)' ); >+ is( $overdue->{title}, $biblio->title, 'unblessed_all_relateds has field from other tables (biblio)' ); > is( $overdue->{homebranch}, $item->homebranch, 'unblessed_all_relateds has field from other tables (items)' ); > > $schema->storage->txn_rollback; >@@ -1124,13 +1140,17 @@ subtest 'get_from_storage' => sub { > Koha::Biblios->find( $biblio->biblionumber )->title($new_title)->store; > > is( $biblio->title, $old_title, 'current $biblio should not be modified' ); >- is( $biblio->get_from_storage->title, >- $new_title, 'get_from_storage should return an updated object' ); >+ is( >+ $biblio->get_from_storage->title, >+ $new_title, 'get_from_storage should return an updated object' >+ ); > > Koha::Biblios->find( $biblio->biblionumber )->delete; > is( ref($biblio), 'Koha::Biblio', 'current $biblio should not be deleted' ); >- is( $biblio->get_from_storage, undef, >- 'get_from_storage should return undef if the object has been deleted' ); >+ is( >+ $biblio->get_from_storage, undef, >+ 'get_from_storage should return undef if the object has been deleted' >+ ); > > $schema->storage->txn_rollback; > }; >@@ -1170,30 +1190,30 @@ subtest 'set_or_blank' => sub { > > $schema->storage->txn_begin; > >- my $item = $builder->build_sample_item; >+ my $item = $builder->build_sample_item; > my $item_info = $item->unblessed; > $item = $item->set_or_blank($item_info); >- is_deeply($item->unblessed, $item_info, 'set_or_blank assign the correct value if unchanged'); >+ is_deeply( $item->unblessed, $item_info, 'set_or_blank assign the correct value if unchanged' ); > > # int not null > delete $item_info->{itemlost}; > $item = $item->set_or_blank($item_info); >- is($item->itemlost, 0, 'set_or_blank should have set itemlost to 0, default value defined in DB'); >+ is( $item->itemlost, 0, 'set_or_blank should have set itemlost to 0, default value defined in DB' ); > > # int nullable > delete $item_info->{restricted}; > $item = $item->set_or_blank($item_info); >- is($item->restricted, undef, 'set_or_blank should have set restristed to null' ); >+ is( $item->restricted, undef, 'set_or_blank should have set restristed to null' ); > > # datetime nullable > delete $item_info->{dateaccessioned}; > $item = $item->set_or_blank($item_info); >- is($item->dateaccessioned, undef, 'set_or_blank should have set dateaccessioned to null'); >+ is( $item->dateaccessioned, undef, 'set_or_blank should have set dateaccessioned to null' ); > > # timestamp not null > delete $item_info->{timestamp}; > $item = $item->set_or_blank($item_info); >- isnt($item->timestamp, undef, 'set_or_blank should have set timestamp to a correct value'); >+ isnt( $item->timestamp, undef, 'set_or_blank should have set timestamp to a correct value' ); > > $schema->storage->txn_rollback; > }; >@@ -1209,19 +1229,20 @@ subtest 'messages() and add_message() tests' => sub { > my @messages = @{ $patron->object_messages }; > is( scalar @messages, 0, 'No messages' ); > >- $patron->add_message({ message => "message_1" }); >- $patron->add_message({ message => "message_2" }); >+ $patron->add_message( { message => "message_1" } ); >+ $patron->add_message( { message => "message_2" } ); > > @messages = @{ $patron->object_messages }; > >- is( scalar @messages, 2, 'Messages are returned' ); >- is( ref($messages[0]), 'Koha::Object::Message', 'Right type returned' ); >- is( ref($messages[1]), 'Koha::Object::Message', 'Right type returned' ); >- is( $messages[0]->message, 'message_1', 'Right message recorded' ); >+ is( scalar @messages, 2, 'Messages are returned' ); >+ is( ref( $messages[0] ), 'Koha::Object::Message', 'Right type returned' ); >+ is( ref( $messages[1] ), 'Koha::Object::Message', 'Right type returned' ); >+ is( $messages[0]->message, 'message_1', 'Right message recorded' ); >+ >+ my $patron_id = $builder->build_object( { class => 'Koha::Patrons' } )->id; > >- my $patron_id = $builder->build_object({ class => 'Koha::Patrons' })->id; > # get a patron from the DB, ->new is not called, ->object_messages should initialize _messages as an empty arrayref >- $patron = Koha::Patrons->find( $patron_id ); >+ $patron = Koha::Patrons->find($patron_id); > > isnt( $patron->object_messages, undef, '->messages initializes the array if required' ); > is( scalar @{ $patron->object_messages }, 0, '->messages returns an empty arrayref' ); >@@ -1248,12 +1269,8 @@ subtest 'is_accessible() tests' => sub { > } > ); > >- my $patron_1 = $builder->build_object( >- { class => 'Koha::Patrons', value => { branchcode => $library_1->id } } >- ); >- my $patron_2 = $builder->build_object( >- { class => 'Koha::Patrons', value => { branchcode => $library_2->id } } >- ); >+ my $patron_1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library_1->id } } ); >+ my $patron_2 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library_2->id } } ); > > t::lib::Mocks::mock_userenv( { patron => $patron } ); > >-- >2.39.5 (Apple Git-154)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=172112">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37902
:
171389
|
171390
|
171391
|
171392
|
171402
|
171403
|
171515
|
171551
|
171652
|
171762
|
171763
|
171764
|
171765
|
171766
|
171767
|
171768
|
172105
|
172106
|
172107
|
172108
|
172109
|
172110
|
172111
| 172112