From ec43e695939c4e41a252d346b675e61831c967df Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 25 Aug 2023 07:21:50 +0000 Subject: [PATCH] Bug 33537: (QA follow-up) Tidy updated blocks in various files Resolving: WARN C4/Installer.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 170, now: 171) FAIL Koha/Exceptions/DomainLimit.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 4) WARN Koha/MailDomainLimit.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 1) WARN Koha/Notice/Util.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 26, now: 31) WARN admin/domain_limits.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 1) WARN installer/data/mysql/atomicupdate/bug33537.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 5) WARN t/db_dependent/Installer.t WARN tidiness The file is less tidy than before (bad/messy lines before: 18, now: 19) WARN t/db_dependent/Koha/MailDomainLimits.t WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 3) WARN t/db_dependent/Koha/Notice_Util.t WARN tidiness The file is less tidy than before (bad/messy lines before: 37, now: 38) WARN t/db_dependent/api/v1/domain_limits.t WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 7) WARN t/lib/TestBuilder.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 211, now: 212) FAIL Koha/REST/V1/Config/SMTP/DomainLimits.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 32) Test plan: Run qa tools. Note: Forbidden pattern warns in Koha/REST/V1/Config/SMTP/DomainLimits.pm are handled in next patch. Signed-off-by: Marcel de Rooy Signed-off-by: Pedro Amorim --- C4/Installer.pm | 17 +-- Koha/Exceptions/DomainLimit.pm | 8 +- Koha/MailDomainLimit.pm | 2 +- Koha/Notice/Util.pm | 41 +++--- Koha/REST/V1/Config/SMTP/DomainLimits.pm | 59 ++++----- admin/domain_limits.pl | 2 +- installer/data/mysql/atomicupdate/bug33537.pl | 12 +- t/db_dependent/Installer.t | 4 +- t/db_dependent/Koha/MailDomainLimits.t | 6 +- t/db_dependent/Koha/Notice_Util.t | 117 ++++++++++++------ t/db_dependent/api/v1/domain_limits.t | 18 +-- t/lib/TestBuilder.pm | 6 +- 12 files changed, 169 insertions(+), 123 deletions(-) diff --git a/C4/Installer.pm b/C4/Installer.pm index d41694d9d7..b3efdad4c8 100644 --- a/C4/Installer.pm +++ b/C4/Installer.pm @@ -34,7 +34,8 @@ use vars qw(@ISA @EXPORT); BEGIN { require Exporter; @ISA = qw( Exporter ); - push @EXPORT, qw( primary_key_exists unique_key_exists foreign_key_exists index_exists column_exists TableExists table_exists marc_framework_sql_list TransformToNum CheckVersion NewVersion SetVersion sanitize_zero_date update get_db_entries get_atomic_updates run_atomic_updates has_non_dynamic_row_format ); + push @EXPORT, + qw( primary_key_exists unique_key_exists foreign_key_exists index_exists column_exists TableExists table_exists marc_framework_sql_list TransformToNum CheckVersion NewVersion SetVersion sanitize_zero_date update get_db_entries get_atomic_updates run_atomic_updates has_non_dynamic_row_format ); }; =head1 NAME @@ -692,14 +693,14 @@ sub column_exists { =cut -sub TableExists { # Legacy name +sub TableExists { # Legacy name my $table = shift; eval { - my $dbh = C4::Context->dbh; - local $dbh->{PrintError} = 0; - local $dbh->{RaiseError} = 1; - $dbh->do(qq{SELECT * FROM $table WHERE 1 = 0 }); - }; + my $dbh = C4::Context->dbh; + local $dbh->{PrintError} = 0; + local $dbh->{RaiseError} = 1; + $dbh->do(qq{SELECT * FROM $table WHERE 1 = 0 }); + }; return 1 unless $@; return 0; } @@ -710,7 +711,7 @@ sub TableExists { # Legacy name =cut -sub table_exists { # More consistently named alias for TableExists +sub table_exists { # More consistently named alias for TableExists return TableExists(@_); } diff --git a/Koha/Exceptions/DomainLimit.pm b/Koha/Exceptions/DomainLimit.pm index 8ec30badf1..96369be22e 100644 --- a/Koha/Exceptions/DomainLimit.pm +++ b/Koha/Exceptions/DomainLimit.pm @@ -24,16 +24,16 @@ use Exception::Class ( isa => 'Koha::Exception', }, 'Koha::Exceptions::DomainLimit::EmptyLimitData' => { - isa => 'Koha::Exceptions::DomainLimit', + isa => 'Koha::Exceptions::DomainLimit', description => 'Unexpected empty limit data', - fields => [ 'parameter' ], + fields => ['parameter'], }, 'Koha::Exceptions::DomainLimit::MemberWithLimit' => { - isa => 'Koha::Exceptions::DomainLimit', + isa => 'Koha::Exceptions::DomainLimit', description => 'A group member cannot have individual limit', }, 'Koha::Exceptions::DomainLimit::NoSelfChaining' => { - isa => 'Koha::Exceptions::DomainLimit', + isa => 'Koha::Exceptions::DomainLimit', description => 'Cannot belong to yourself', }, ); diff --git a/Koha/MailDomainLimit.pm b/Koha/MailDomainLimit.pm index e3c23e438b..d39fcef482 100644 --- a/Koha/MailDomainLimit.pm +++ b/Koha/MailDomainLimit.pm @@ -78,7 +78,7 @@ This method returns the mapping for API object representation. sub to_api_mapping { return { - id => 'domain_limit_id', + id => 'domain_limit_id', }; } diff --git a/Koha/Notice/Util.pm b/Koha/Notice/Util.pm index 2099d3332e..180918fead 100644 --- a/Koha/Notice/Util.pm +++ b/Koha/Notice/Util.pm @@ -43,8 +43,8 @@ sub load_domain_limits { my ( $class ) = @_; my $rs = Koha::MailDomainLimits->search_with_group_domain; if ( $rs->count ) { - return { map { _init_domain_entry($_) } @{$rs->unblessed} }; - } else { # fall back to koha-conf + return { map { _init_domain_entry($_) } @{ $rs->unblessed } }; + } else { # fall back to koha-conf return _load_from_file(); } } @@ -52,14 +52,18 @@ sub load_domain_limits { sub _load_from_file { my $config = C4::Context->config('message_domain_limits'); my $domain_limits; - if( ref($config) eq 'HASH' ) { - if( exists $config->{domain} ) { + if ( ref($config) eq 'HASH' ) { + if ( exists $config->{domain} ) { + # Turn single hash entry into array - $domain_limits = ref($config->{domain}) eq 'HASH' + $domain_limits = + ref( $config->{domain} ) eq 'HASH' ? [ $config->{domain} ] : $config->{domain}; + # Convert to hash structure by domain name - $domain_limits = { map { _init_domain_entry({ %$_ }, 1); } @$domain_limits }; # copy hashes to prevent changes + $domain_limits = + { map { _init_domain_entry( {%$_}, 1 ); } @$domain_limits }; # copy hashes to prevent changes } } return $domain_limits; @@ -67,22 +71,23 @@ sub _load_from_file { sub _init_domain_entry { my ( $entry, $old_school ) = @_; + # entry comes from table or koha-conf # Return either a hash like ( name => { limit => , units =>, unit_type =>, count => } ) for regular entries # or return a hash like ( name => { belongs_to => } ) for a domain that is part of a group # We need old_school when passing data from koha-conf.xml - if( $old_school ) { + if ($old_school) { ( $entry->{units}, $entry->{unit_type} ) = _unit_info( delete $entry->{unit} ); } else { - $entry->{name} = delete $entry->{domain_name}; - $entry->{limit} = delete $entry->{domain_limit}; + $entry->{name} = delete $entry->{domain_name}; + $entry->{limit} = delete $entry->{domain_limit}; $entry->{belongs_to} = delete $entry->{group_domain}; } return if !exists $entry->{name}; my $elements; - if( $entry->{belongs_to} ) { + if ( $entry->{belongs_to} ) { $elements = { belongs_to => lc $entry->{belongs_to} }; } else { $elements = { @@ -95,9 +100,9 @@ sub _init_domain_entry { return ( lc $entry->{name}, $elements ); } -sub _unit_info { # Converting old-style unit like '3m' to ( 3, 'minutes' ) etc. - my ( $unit ) = @_; - if( $unit && $unit =~ /(\d+)([mhd])/ ) { +sub _unit_info { # Converting old-style unit like '3m' to ( 3, 'minutes' ) etc. + my ($unit) = @_; + if ( $unit && $unit =~ /(\d+)([mhd])/ ) { return ( $1, ABBREV->{$2} ); } return; @@ -111,9 +116,9 @@ sub _unit_info { # Converting old-style unit like '3m' to ( 3, 'minutes' ) etc. sub exceeds_limit { my ( $class, $params ) = @_; - my $domain_limits = $params->{limits} or return 0; # no limits at all - my $to_address = $params->{to} or return 0; # no address, no limit exceeded - my $incr = $params->{incr} // 1; # by default we increment + my $domain_limits = $params->{limits} or return 0; # no limits at all + my $to_address = $params->{to} or return 0; # no address, no limit exceeded + my $incr = $params->{incr} // 1; # by default we increment my $domain = q{}; $domain = lc $1 if $to_address && $to_address =~ /@(\H+)/; @@ -124,9 +129,9 @@ sub exceeds_limit { _get_domain_count( $domain, $group, $domain_limits ) if !defined $domain_limits->{$group}->{count}; return 1 if $domain_limits->{$group}->{count} >= $domain_limits->{$group}->{limit}; - if( $incr ) { + if ($incr) { $domain_limits->{$group}->{count}++; - warn "Sending messages: domain $group reached limit of ". $domain_limits->{$group}->{limit} + warn "Sending messages: domain $group reached limit of " . $domain_limits->{$group}->{limit} if $domain_limits->{$group}->{count} == $domain_limits->{$group}->{limit}; } return 0; diff --git a/Koha/REST/V1/Config/SMTP/DomainLimits.pm b/Koha/REST/V1/Config/SMTP/DomainLimits.pm index 8e3dfe801a..9a4c3b47c1 100644 --- a/Koha/REST/V1/Config/SMTP/DomainLimits.pm +++ b/Koha/REST/V1/Config/SMTP/DomainLimits.pm @@ -20,7 +20,7 @@ package Koha::REST::V1::Config::SMTP::DomainLimits; use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; use Scalar::Util qw( blessed ); -use Try::Tiny qw( catch try ); +use Try::Tiny qw( catch try ); use Koha::MailDomainLimits; @@ -39,8 +39,7 @@ sub list { return try { my $limits = $c->objects->search( Koha::MailDomainLimits->search_with_group_domain ); return $c->render( _render_helper( 200, undef, result => $limits ) ); - } - catch { + } catch { $c->unhandled_exception($_); }; } @@ -55,10 +54,9 @@ sub get { my $c = shift->openapi->valid_input or return; return try { my $rec = Koha::MailDomainLimits->search_with_group_domain->find( $c->validation->param('domain_limit_id') ); - return $c->render( _render_helper( 404, undef, error => "Domain limit not found" ) ) if !$rec; + return $c->render( _render_helper( 404, undef, error => "Domain limit not found" ) ) if !$rec; return $c->render( _render_helper( 200, undef, result => $rec->to_api ) ); - } - catch { + } catch { $c->unhandled_exception($_); } } @@ -76,17 +74,16 @@ sub add { $limit->store->discard_changes; $c->res->headers->location( $c->req->url->to_string . '/' . $limit->id ); return $c->render( _render_helper( 201, undef, result => $limit->to_api ) ); - } - catch { - if( blessed($_) ) { + } catch { + if ( blessed($_) ) { return $c->render( _render_helper( 409, $_ ) ) if $_->isa('Koha::Exceptions::DomainLimit::NoSelfChaining') - || $_->isa('Koha::Exceptions::DomainLimit::MemberWithLimit') - || $_->isa('Koha::Exceptions::Object::DuplicateID'); + || $_->isa('Koha::Exceptions::DomainLimit::MemberWithLimit') + || $_->isa('Koha::Exceptions::Object::DuplicateID'); return $c->render( _render_helper( 400, $_, missing => $_->parameter ) ) if $_->isa('Koha::Exceptions::DomainLimit::EmptyLimitData'); return $c->render( _render_helper( 404, $_ ) ) - if $_->isa('Koha::Exceptions::Object::FKConstraint'); # on belongs_to + if $_->isa('Koha::Exceptions::Object::FKConstraint'); # on belongs_to } $c->unhandled_exception($_); }; @@ -99,24 +96,23 @@ Controller method that handles updating a domain limit =cut sub update { - my $c = shift->openapi->valid_input or return; + my $c = shift->openapi->valid_input or return; my $limit = Koha::MailDomainLimits->find( $c->validation->param('domain_limit_id') ); - return $c->render( _render_helper( 404, undef, error => "Object not found" ) ) if !$limit; + return $c->render( _render_helper( 404, undef, error => "Object not found" ) ) if !$limit; return try { $limit->set_from_api( $c->validation->param('body') ); $limit->store->discard_changes; return $c->render( _render_helper( 200, undef, result => $limit->to_api ) ); - } - catch { - if( blessed($_) ) { - return $c->render(_render_helper( 409, $_ ) ) + } catch { + if ( blessed($_) ) { + return $c->render( _render_helper( 409, $_ ) ) if $_->isa('Koha::Exceptions::DomainLimit::NoSelfChaining') - || $_->isa('Koha::Exceptions::DomainLimit::MemberWithLimit') - || $_->isa('Koha::Exceptions::Object::DuplicateID'); - return $c->render(_render_helper( 400, $_, missing => $_->parameter ) ) + || $_->isa('Koha::Exceptions::DomainLimit::MemberWithLimit') + || $_->isa('Koha::Exceptions::Object::DuplicateID'); + return $c->render( _render_helper( 400, $_, missing => $_->parameter ) ) if $_->isa('Koha::Exceptions::DomainLimit::EmptyLimitData'); - return $c->render(_render_helper( 404, $_ ) ) - if $_->isa('Koha::Exceptions::Object::FKConstraint'); # on belongs_to + return $c->render( _render_helper( 404, $_ ) ) + if $_->isa('Koha::Exceptions::Object::FKConstraint'); # on belongs_to } $c->unhandled_exception($_); }; @@ -129,14 +125,13 @@ Controller method that handles deleting a domain limit =cut sub delete { - my $c = shift->openapi->valid_input or return; + my $c = shift->openapi->valid_input or return; my $limit = Koha::MailDomainLimits->find( $c->validation->param('domain_limit_id') ); - return $c->render( _render_helper( 404, undef, error => "Domain limit not found" ) ) if !$limit; + return $c->render( _render_helper( 404, undef, error => "Domain limit not found" ) ) if !$limit; return try { $limit->delete; return $c->render( _render_helper( 204, undef, result => q{} ) ); - } - catch { + } catch { $c->unhandled_exception($_); }; } @@ -146,13 +141,13 @@ sub delete { =cut sub _render_helper { - my ( $status, $exception, %result ) = @_; # %result can be: result => $result to pass only one result - my $resp = { status => $status }; + my ( $status, $exception, %result ) = @_; # %result can be: result => $result to pass only one result + my $resp = { status => $status }; my @error_message = $exception ? ( error => ( $exception->message || $exception->description ) ) : (); - if( @error_message ) { + if (@error_message) { $resp->{openapi} = { @error_message, %result }; - } elsif( keys %result ) { - $resp->{openapi} = $result{result} // { %result }; + } elsif ( keys %result ) { + $resp->{openapi} = $result{result} // {%result}; } return %$resp; } diff --git a/admin/domain_limits.pl b/admin/domain_limits.pl index b6679a19ee..9f5cf789d8 100755 --- a/admin/domain_limits.pl +++ b/admin/domain_limits.pl @@ -20,7 +20,7 @@ use Modern::Perl; use CGI qw ( -utf8 ); -use C4::Auth qw( get_template_and_user ); +use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_html_with_http_headers ); my $input = CGI->new; diff --git a/installer/data/mysql/atomicupdate/bug33537.pl b/installer/data/mysql/atomicupdate/bug33537.pl index 8a621d9f81..47eec5ca8b 100755 --- a/installer/data/mysql/atomicupdate/bug33537.pl +++ b/installer/data/mysql/atomicupdate/bug33537.pl @@ -1,14 +1,15 @@ use Modern::Perl; return { - bug_number => 33537, + bug_number => 33537, description => "Add table mail_domain_limits", - up => sub { + up => sub { my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; + my ( $dbh, $out ) = @$args{qw(dbh out)}; if ( !table_exists('mail_domain_limits') ) { - $dbh->do(q{ + $dbh->do( + q{ CREATE TABLE `mail_domain_limits` ( `id` int(11) NOT NULL AUTO_INCREMENT, `domain_name` tinytext COLLATE utf8mb4_unicode_ci NOT NULL, @@ -21,7 +22,8 @@ CREATE TABLE `mail_domain_limits` ( KEY `mail_domain_limits_fk1` (`belongs_to`), CONSTRAINT `mail_domain_limits_fk1` FOREIGN KEY (`belongs_to`) REFERENCES `mail_domain_limits` (`id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; - }); + } + ); say $out "Added new table 'mail_domain_limits'"; } }, diff --git a/t/db_dependent/Installer.t b/t/db_dependent/Installer.t index 5f21b10822..9bc94ff268 100755 --- a/t/db_dependent/Installer.t +++ b/t/db_dependent/Installer.t @@ -79,8 +79,8 @@ ok( ! primary_key_exists( 'borrowers', 'email'), 'Borrowers does not have a prim subtest 'table_exists' => sub { plan tests => 2; - is( table_exists( 'borrowers' ), 1, 'Table borrowers still exists ;)' ); - is( table_exists( 'this_table_will_never_exist' ), 0, 'This table does not exist, not likely to be created too' ); + is( table_exists('borrowers'), 1, 'Table borrowers still exists ;)' ); + is( table_exists('this_table_will_never_exist'), 0, 'This table does not exist, not likely to be created too' ); }; subtest 'marc_framework_sql_list' => sub { diff --git a/t/db_dependent/Koha/MailDomainLimits.t b/t/db_dependent/Koha/MailDomainLimits.t index 73eda368cb..8804b029a5 100755 --- a/t/db_dependent/Koha/MailDomainLimits.t +++ b/t/db_dependent/Koha/MailDomainLimits.t @@ -47,7 +47,7 @@ subtest 'store' => sub { plan tests => 5; my $limit01 = $builder->build_object( { class => 'Koha::MailDomainLimits' } ); my $limit02 = $builder->build_object( { class => 'Koha::MailDomainLimits' } ); - my $value = { domain_limit => 1, belongs_to => $limit01->id }; + my $value = { domain_limit => 1, belongs_to => $limit01->id }; throws_ok { $limit02->set($value)->store } 'Koha::Exceptions::DomainLimit::MemberWithLimit', 'Group member with limit raised exception'; $value = { units => 0, belongs_to => undef }; @@ -71,11 +71,11 @@ subtest 'search_with_group_domain, group_domain' => sub { # Check group_domain with and without search_with is( $limit02->_result->has_column_loaded('group_domain'), 0, 'No additional column' ); - is( $limit02->group_domain, $limit01->domain_name, 'Check group domain name via relation' ); + is( $limit02->group_domain, $limit01->domain_name, 'Check group domain name via relation' ); my $rs_plus = Koha::MailDomainLimits->search_with_group_domain; my $limit02_plus = $rs_plus->find( $limit02->id ); is( $limit02_plus->_result->has_column_loaded('group_domain'), 1, 'Includes additional column' ); - is( $limit02_plus->group_domain, $limit01->domain_name, 'Check group domain name from search_with' ); + is( $limit02_plus->group_domain, $limit01->domain_name, 'Check group domain name from search_with' ); $limit02_plus->belongs_to( $limit03->id )->store->discard_changes; is( $limit02_plus->_result->has_column_loaded('group_domain'), 0, 'Column lost by discarding changes' ); is( diff --git a/t/db_dependent/Koha/Notice_Util.t b/t/db_dependent/Koha/Notice_Util.t index 7f216e44ea..fb8032bc09 100755 --- a/t/db_dependent/Koha/Notice_Util.t +++ b/t/db_dependent/Koha/Notice_Util.t @@ -47,9 +47,9 @@ subtest 'load_domain_limits' => sub { is( Koha::Notice::Util->load_domain_limits, undef, 'koha-conf contains blank entry' ); t::lib::Mocks::mock_config( 'message_domain_limits', { domain => { name => 'A', limit => 2, unit => '1d' } } ); $domain_limits = Koha::Notice::Util->load_domain_limits; - is( keys %$domain_limits, 1, 'koha-conf contains one domain' ); - is( $domain_limits->{a}->{limit}, 2, 'check limit of first entry' ); - is( $domain_limits->{a}->{units}, 1, 'check unit of first entry' ); + is( keys %$domain_limits, 1, 'koha-conf contains one domain' ); + is( $domain_limits->{a}->{limit}, 2, 'check limit of first entry' ); + is( $domain_limits->{a}->{units}, 1, 'check unit of first entry' ); is( $domain_limits->{a}->{unit_type}, 'days', 'check unit of first entry' ); t::lib::Mocks::mock_config( 'message_domain_limits', @@ -62,15 +62,19 @@ subtest 'load_domain_limits' => sub { } ); $domain_limits = Koha::Notice::Util->load_domain_limits; - is( keys %$domain_limits, 3, 'koha-conf contains two domains' ); - is( $domain_limits->{b}->{limit}, 3, 'check limit of second entry' ); - is( $domain_limits->{b}->{count}, undef, 'check if count still undefined' ); - is( $domain_limits->{c}->{belongs_to}, 'b', 'check group domain of C' ); + is( keys %$domain_limits, 3, 'koha-conf contains two domains' ); + is( $domain_limits->{b}->{limit}, 3, 'check limit of second entry' ); + is( $domain_limits->{b}->{count}, undef, 'check if count still undefined' ); + is( $domain_limits->{c}->{belongs_to}, 'b', 'check group domain of C' ); # Test with database - my $limit01 = Koha::MailDomainLimit->new({ domain_name => 'A', domain_limit => 2, units => '2', unit_type => 'days' })->store; - my $limit02 = Koha::MailDomainLimit->new({ domain_name => 'B', domain_limit => 3, units => '3', unit_type => 'hours' })->store; - my $limit03 = Koha::MailDomainLimit->new({ domain_name => 'C', belongs_to => $limit02->id })->store; + my $limit01 = + Koha::MailDomainLimit->new( { domain_name => 'A', domain_limit => 2, units => '2', unit_type => 'days' } ) + ->store; + my $limit02 = + Koha::MailDomainLimit->new( { domain_name => 'B', domain_limit => 3, units => '3', unit_type => 'hours' } ) + ->store; + my $limit03 = Koha::MailDomainLimit->new( { domain_name => 'C', belongs_to => $limit02->id } )->store; my $domain_limits_db = Koha::Notice::Util->load_domain_limits; is_deeply( $domain_limits_db, $domain_limits, 'Same results for limits in database' ); }; @@ -79,24 +83,40 @@ subtest 'counting in exceeds_limit' => sub { plan tests => 3; my $domain_limits; + # Check counting my @values = ( message_transport_type => 'email', status => 'sent' ); - my $today = dt_from_string(); + my $today = dt_from_string(); + #FIXME Why are the three following build calls so slow? - $builder->build_object({ class => 'Koha::Notice::Messages', - value => { @values, to_address => 'a@A', updated_on => $today->clone->subtract( hours => 36 ) }}); - $builder->build_object({ class => 'Koha::Notice::Messages', - value => { @values, to_address => 'b@A', updated_on => $today->clone->subtract( hours => 49 ) }}); - $builder->build_object({ class => 'Koha::Notice::Messages', - value => { @values, to_address => 'c@A', updated_on => $today->clone->subtract( days => 3 ) }}); - - $domain_limits = Koha::Notice::Util->load_domain_limits; # still using last config for A: 2/2d - Koha::Notice::Util->exceeds_limit({ to => '@A', limits => $domain_limits, incr => 0 }); # force counting + $builder->build_object( + { + class => 'Koha::Notice::Messages', + value => { @values, to_address => 'a@A', updated_on => $today->clone->subtract( hours => 36 ) } + } + ); + $builder->build_object( + { + class => 'Koha::Notice::Messages', + value => { @values, to_address => 'b@A', updated_on => $today->clone->subtract( hours => 49 ) } + } + ); + $builder->build_object( + { + class => 'Koha::Notice::Messages', + value => { @values, to_address => 'c@A', updated_on => $today->clone->subtract( days => 3 ) } + } + ); + + $domain_limits = Koha::Notice::Util->load_domain_limits; # still using last config for A: 2/2d + Koha::Notice::Util->exceeds_limit( { to => '@A', limits => $domain_limits, incr => 0 } ); # force counting is( $domain_limits->{a}->{count}, 1, '1 message to A within unit of 2d' ); - Koha::MailDomainLimits->search({ domain_name => 'A' })->next->set({ domain_limit => 2, units => '50', unit_type => 'hours' })->store; + Koha::MailDomainLimits->search( { domain_name => 'A' } ) + ->next->set( { domain_limit => 2, units => '50', unit_type => 'hours' } )->store; $domain_limits = Koha::Notice::Util->load_domain_limits; - Koha::Notice::Util->exceeds_limit({ to => 'x@A ', limits => $domain_limits, incr => 0 }); # force counting + Koha::Notice::Util->exceeds_limit( { to => 'x@A ', limits => $domain_limits, incr => 0 } ); # force counting is( $domain_limits->{a}->{count}, 2, '2 messages to A within unit of 50h' ); + # Check count for B; if counted, there would be 0 or higher, otherwise undef ok( !defined $domain_limits->{b}->{count}, 'Prove that we did not count b if not asked for' ); }; @@ -106,10 +126,13 @@ subtest '_calc_startdate' => sub { # Date subtraction - edge case (start of summer time) my $mock_context = Test::MockModule->new('C4::Context'); - $mock_context->mock( 'tz', sub { return DateTime::TimeZone->new( name => 'Europe/Amsterdam' )} ); + $mock_context->mock( 'tz', sub { return DateTime::TimeZone->new( name => 'Europe/Amsterdam' ) } ); my $dt = dt_from_string( '2023-03-31 02:30:00', 'iso', '+02:00' ); - is( Koha::Notice::Util::_calc_startdate( $dt, 4, 'days')->stringify, '2023-03-27T02:30:00', '02:30 is fine' ); - is( Koha::Notice::Util::_calc_startdate( $dt, 1, 'days')->stringify, '2023-03-26T01:30:00', 'Moved 02:30 to 01:30' ); + is( Koha::Notice::Util::_calc_startdate( $dt, 4, 'days' )->stringify, '2023-03-27T02:30:00', '02:30 is fine' ); + is( + Koha::Notice::Util::_calc_startdate( $dt, 1, 'days' )->stringify, '2023-03-26T01:30:00', + 'Moved 02:30 to 01:30' + ); $mock_context->unmock('tz'); }; @@ -120,25 +143,41 @@ subtest 'exceeds_limit with group domains' => sub { Koha::MailDomainLimits->delete; t::lib::Mocks::mock_config( 'message_domain_limits', undef ); $domain_limits = Koha::Notice::Util->load_domain_limits; - is( Koha::Notice::Util->exceeds_limit({ to => 'marcel@koha.nl', limits => $domain_limits }), 0, 'False when having no limits' ); + is( + Koha::Notice::Util->exceeds_limit( { to => 'marcel@koha.nl', limits => $domain_limits } ), 0, + 'False when having no limits' + ); - my $limit01 = Koha::MailDomainLimit->new({ domain_name => 'A', domain_limit => 3, units => 5, unit_type => 'minutes' })->store; - my $limit02 = Koha::MailDomainLimit->new({ domain_name => 'B', domain_limit => 2, units => 5, unit_type => 'minutes' })->store; - my $limit03 = Koha::MailDomainLimit->new({ domain_name => 'C', belongs_to => $limit01->id })->store; + my $limit01 = + Koha::MailDomainLimit->new( { domain_name => 'A', domain_limit => 3, units => 5, unit_type => 'minutes' } ) + ->store; + my $limit02 = + Koha::MailDomainLimit->new( { domain_name => 'B', domain_limit => 2, units => 5, unit_type => 'minutes' } ) + ->store; + my $limit03 = Koha::MailDomainLimit->new( { domain_name => 'C', belongs_to => $limit01->id } )->store; $domain_limits = Koha::Notice::Util->load_domain_limits; my $result; - is( Koha::Notice::Util->exceeds_limit({ to => '1@A', limits => $domain_limits }), 0, 'First message to A' ); - is( Koha::Notice::Util->exceeds_limit({ to => '2@C', limits => $domain_limits }), 0, 'Second message to A (via C)' ); + is( Koha::Notice::Util->exceeds_limit( { to => '1@A', limits => $domain_limits } ), 0, 'First message to A' ); + is( + Koha::Notice::Util->exceeds_limit( { to => '2@C', limits => $domain_limits } ), 0, + 'Second message to A (via C)' + ); ok( !exists $domain_limits->{c}->{count}, 'No count exists for grouped domain' ); - warning_like { $result = Koha::Notice::Util->exceeds_limit({ to => '3@A', limits => $domain_limits }) } - qr/Sending messages: domain a reached limit/, 'Check warn for reaching limit A'; + warning_like { $result = Koha::Notice::Util->exceeds_limit( { to => '3@A', limits => $domain_limits } ) } + qr/Sending messages: domain a reached limit/, 'Check warn for reaching limit A'; is( $result, 0, 'Limit for A reached, not exceeded' ); - is( Koha::Notice::Util->exceeds_limit({ to => '4@C', limits => $domain_limits }), 1, 'Limit for A exceeded (via C)' ); - is( Koha::Notice::Util->exceeds_limit({ to => '5@B', limits => $domain_limits }), 0, 'First message to B' ); - is( $domain_limits->{b}->{count}, 1, 'Count B updated' ); - is( Koha::Notice::Util->exceeds_limit({ to => '5@B', limits => $domain_limits, incr => 0 }), 0, 'Test incr flag' ); - is( $domain_limits->{b}->{count}, 1, 'Count B still 1' ); - is( Koha::Notice::Util->exceeds_limit({ to => '6@D', limits => $domain_limits }), 0, 'No limits for D' ); + is( + Koha::Notice::Util->exceeds_limit( { to => '4@C', limits => $domain_limits } ), 1, + 'Limit for A exceeded (via C)' + ); + is( Koha::Notice::Util->exceeds_limit( { to => '5@B', limits => $domain_limits } ), 0, 'First message to B' ); + is( $domain_limits->{b}->{count}, 1, 'Count B updated' ); + is( + Koha::Notice::Util->exceeds_limit( { to => '5@B', limits => $domain_limits, incr => 0 } ), 0, + 'Test incr flag' + ); + is( $domain_limits->{b}->{count}, 1, 'Count B still 1' ); + is( Koha::Notice::Util->exceeds_limit( { to => '6@D', limits => $domain_limits } ), 0, 'No limits for D' ); }; $schema->storage->txn_rollback; diff --git a/t/db_dependent/api/v1/domain_limits.t b/t/db_dependent/api/v1/domain_limits.t index 8b60ebeb69..09f62ec3d0 100755 --- a/t/db_dependent/api/v1/domain_limits.t +++ b/t/db_dependent/api/v1/domain_limits.t @@ -38,13 +38,14 @@ subtest 'Few CRUD tests' => sub { $schema->storage->txn_begin; Koha::MailDomainLimits->delete; - my $user = $builder->build_object({ class => 'Koha::Patrons' }); + my $user = $builder->build_object( { class => 'Koha::Patrons' } ); my $userid = $user->userid; my $password = '2345'; $user->set_password( { password => $password, skip_validation => 1 } ); # Test GET ALL, 403 Unauthorized $t->get_ok("//$userid:$password@/api/v1/smtp/domain_limits")->status_is(403); + # Change flags $user->flags(8)->store; $t->get_ok("//$userid:$password@/api/v1/smtp/domain_limits")->status_is(200)->json_is( [] ); @@ -53,23 +54,26 @@ subtest 'Few CRUD tests' => sub { my $data = { domain_name => 'test.nl', domain_limit => 1, units => 2, unit_type => 'minutes' }; $t->post_ok( "//$userid:$password@/api/v1/smtp/domain_limits" => json => $data )->status_is(201); is( Koha::MailDomainLimits->count, 1, 'Added record' ); - my $limit = Koha::MailDomainLimits->search->next; + my $limit = Koha::MailDomainLimits->search->next; my $limit_id = $limit->id; is( $limit->domain_name, 'test.nl', 'Check a field' ); + # Add member - $t->post_ok( "//$userid:$password@/api/v1/smtp/domain_limits" => json => { domain_name => 'test.be', belongs_to => $limit_id } ) - ->status_is(201); + $t->post_ok( "//$userid:$password@/api/v1/smtp/domain_limits" => json => + { domain_name => 'test.be', belongs_to => $limit_id } )->status_is(201); # Single GET, check joined column for member just added $t->get_ok("//$userid:$password@/api/v1/smtp/domain_limits/0")->status_is(404); my $member_id = $limit_id + 1; - $t->get_ok("//$userid:$password@/api/v1/smtp/domain_limits/$member_id")->status_is(200)->json_has('/group_domain', 'test.nl'); + $t->get_ok("//$userid:$password@/api/v1/smtp/domain_limits/$member_id")->status_is(200) + ->json_has( '/group_domain', 'test.nl' ); # Modify with PUT $data->{units} = 3; $t->put_ok( "//$userid:$password@/api/v1/smtp/domain_limits/$limit_id" => json => $data )->status_is(200); $limit->discard_changes; is( $limit->units, 3, 'PUT changed a field' ); + # Try to set belongs_to, triggers exception $data->{belongs_to} = $limit->id; $t->put_ok( "//$userid:$password@/api/v1/smtp/domain_limits/$limit_id" => json => $data )->status_is(409); @@ -78,9 +82,9 @@ subtest 'Few CRUD tests' => sub { $t->post_ok( "//$userid:$password@/api/v1/smtp/domain_limits" => json => $data )->status_is(409); # Try DELETE - $t->delete_ok( "//$userid:$password@/api/v1/smtp/domain_limits/$limit_id" )->status_is(204); + $t->delete_ok("//$userid:$password@/api/v1/smtp/domain_limits/$limit_id")->status_is(204); is( Koha::MailDomainLimits->count, 0, 'Removed record' ); - $t->delete_ok( "//$userid:$password@/api/v1/smtp/domain_limits/$limit_id" )->status_is(404); + $t->delete_ok("//$userid:$password@/api/v1/smtp/domain_limits/$limit_id")->status_is(404); $schema->storage->txn_rollback; }; diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index f1da2d5b64..ac0b9ae025 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -661,10 +661,10 @@ sub _gen_default_values { closure_reason => undef, renewal_priority => undef, vendor_id => undef, - }, + }, MailDomainLimit => { - belongs_to => undef, - }, + belongs_to => undef, + }, }; } -- 2.30.2