[Rt-commit] rt branch, 4.4/businesshours-7-warning, created. rt-4.4.1-216-gc00e56a
Shawn Moore
shawn at bestpractical.com
Wed Jan 18 12:09:20 EST 2017
The branch, 4.4/businesshours-7-warning has been created
at c00e56a8dba7fc54f0db6986a1e74368cb7ac19b (commit)
- Log -----------------------------------------------------------------
commit 0c8b78212d917f7430786d129566078484e312aa
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Dec 29 02:33:10 2016 +0000
Switch SLA config get/set to use core RT patterns
This was a vestigate of SLA first being an extension, then being pulled
into core.
This enables the following commit to implement a PostLoadCheck for
ServiceBusinessHours.
diff --git a/lib/RT/Action/SLA_SetDue.pm b/lib/RT/Action/SLA_SetDue.pm
index 0215e71..e1d112b 100644
--- a/lib/RT/Action/SLA_SetDue.pm
+++ b/lib/RT/Action/SLA_SetDue.pm
@@ -121,7 +121,7 @@ sub IsOutsideActor {
# owner is always treated as inside actor
return 0 if $actor->id == $self->TicketObj->Owner;
- if ( $RT::ServiceAgreements{'AssumeOutsideActor'} ) {
+ if ( RT->Config->Get('ServiceAgreements')->{'AssumeOutsideActor'} ) {
# All non-admincc users are outside actors
return 0 if $self->TicketObj ->AdminCc->HasMemberRecursively( $actor )
or $self->TicketObj->QueueObj->AdminCc->HasMemberRecursively( $actor );
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 3407796..09fd0c7 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -1173,6 +1173,14 @@ our %META;
$self->Set( 'ExternalInfoPriority', \@values );
},
},
+
+ ServiceBusinessHours => {
+ Type => 'HASH',
+ },
+
+ ServiceAgreements => {
+ Type => 'HASH',
+ },
);
my %OPTIONS = ();
my @LOADED_CONFIGS = ();
diff --git a/lib/RT/SLA.pm b/lib/RT/SLA.pm
index 6db67a5..d681a32 100644
--- a/lib/RT/SLA.pm
+++ b/lib/RT/SLA.pm
@@ -67,8 +67,9 @@ sub BusinessHours {
require Business::Hours;
my $res = new Business::Hours;
- $res->business_hours( %{ $RT::ServiceBusinessHours{ $name } } )
- if $RT::ServiceBusinessHours{ $name };
+ my %config = RT->Config->Get('ServiceBusinessHours');
+ $res->business_hours(%{ $config{$name} })
+ if $config{$name};
return $res;
}
@@ -83,7 +84,8 @@ sub Agreement {
@_
);
- my $meta = $RT::ServiceAgreements{'Levels'}{ $args{'Level'} };
+ my %config = RT->Config->Get('ServiceAgreements');
+ my $meta = $config{'Levels'}{ $args{'Level'} };
return undef unless $meta;
if ( exists $meta->{'StartImmediately'} || !defined $meta->{'Starts'} ) {
@@ -115,8 +117,8 @@ sub Agreement {
$res{'OutOfHours'} = $meta->{'OutOfHours'}{ $args{'Type'} };
$args{'Queue'} ||= $args{'Ticket'}->QueueObj if $args{'Ticket'};
- if ( $args{'Queue'} && ref $RT::ServiceAgreements{'QueueDefault'}{ $args{'Queue'}->Name } ) {
- $res{'Timezone'} = $RT::ServiceAgreements{'QueueDefault'}{ $args{'Queue'}->Name }{'Timezone'};
+ if ( $args{'Queue'} && ref $config{'QueueDefault'}{ $args{'Queue'}->Name } ) {
+ $res{'Timezone'} = $config{'QueueDefault'}{ $args{'Queue'}->Name }{'Timezone'};
}
$res{'Timezone'} ||= $meta->{'Timezone'} || $RT::Timezone;
@@ -236,16 +238,18 @@ sub GetDefaultServiceLevel {
if ( !$args{'Queue'} && $args{'Ticket'} ) {
$args{'Queue'} = $args{'Ticket'}->QueueObj;
}
+
+ my %config = RT->Config->Get('ServiceAgreements');
if ( $args{'Queue'} ) {
return undef if $args{Queue}->SLADisabled;
return $args{'Queue'}->SLA if $args{'Queue'}->SLA;
- if ( $RT::ServiceAgreements{'QueueDefault'} &&
- ( my $info = $RT::ServiceAgreements{'QueueDefault'}{ $args{'Queue'}->Name } )) {
+ if ( $config{'QueueDefault'} &&
+ ( my $info = $config{'QueueDefault'}{ $args{'Queue'}->Name } )) {
return $info unless ref $info;
- return $info->{'Level'} || $RT::ServiceAgreements{'Default'};
+ return $info->{'Level'} || $config{'Default'};
}
}
- return $RT::ServiceAgreements{'Default'};
+ return $config{'Default'};
}
RT::Base->_ImportOverlays();
diff --git a/share/html/Elements/SelectSLA b/share/html/Elements/SelectSLA
index d91251d..b066275 100644
--- a/share/html/Elements/SelectSLA
+++ b/share/html/Elements/SelectSLA
@@ -49,7 +49,7 @@
% if ($DefaultValue) {
<option value=""<% !$Default ? qq[ selected="selected"] : '' |n %>><%$DefaultLabel |n%></option>
% }
-% for my $sla ( sort keys %{$RT::ServiceAgreements{'Levels'}} ) {
+% for my $sla ( sort keys %{RT->Config->Get('ServiceAgreements')->{'Levels'}} ) {
<option <% $sla eq ($Default//'') ? qq[ selected="selected"] : '' |n %> value="<% $sla %>" ><% $sla %></option>
% }
</select>
diff --git a/t/sla/business_hours.t b/t/sla/business_hours.t
index 939220b..e81b0b8 100644
--- a/t/sla/business_hours.t
+++ b/t/sla/business_hours.t
@@ -13,8 +13,7 @@ RT::Test->load_or_create_queue( Name => 'General', SLADisabled => 0 );
diag 'check business hours' if $ENV{'TEST_VERBOSE'};
{
- no warnings 'once';
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => 'Sunday',
Levels => {
Sunday => {
@@ -25,9 +24,9 @@ diag 'check business hours' if $ENV{'TEST_VERBOSE'};
Resolve => { BusinessMinutes => 60 },
},
},
- );
+ ));
- %RT::ServiceBusinessHours = (
+ RT->Config->Set(ServiceBusinessHours => (
Sunday => {
0 => {
Name => 'Sunday',
@@ -42,7 +41,7 @@ diag 'check business hours' if $ENV{'TEST_VERBOSE'};
End => '17:00'
},
},
- );
+ ));
set_fixed_time('2007-01-01T00:00:00Z');
diff --git a/t/sla/due.t b/t/sla/due.t
index 120cc82..ec18c57 100644
--- a/t/sla/due.t
+++ b/t/sla/due.t
@@ -7,13 +7,13 @@ RT::Test->load_or_create_queue( Name => 'General', SLADisabled => 0 );
diag 'check change of Due date when SLA for a ticket is changed' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Resolve => { RealMinutes => 60*2 } },
'4' => { Resolve => { RealMinutes => 60*4 } },
},
- );
+ ));
my $time = time;
@@ -39,12 +39,12 @@ diag 'check change of Due date when SLA for a ticket is changed' if $ENV{'TEST_V
diag 'when not requestor creates a ticket, we dont set due date' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Response => { RealMinutes => 60*2 } },
},
- );
+ ));
my $ticket = RT::Ticket->new( $RT::SystemUser );
my ($id) = $ticket->Create(
@@ -62,12 +62,12 @@ diag 'when not requestor creates a ticket, we dont set due date' if $ENV{'TEST_V
diag 'check that reply to requestors unset due date' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Response => { RealMinutes => 60*2 } },
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -160,7 +160,7 @@ diag 'check that reply to requestors unset due date' if $ENV{'TEST_VERBOSE'};
diag 'check that reply to requestors dont unset due date with KeepInLoop' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => {
@@ -168,7 +168,7 @@ diag 'check that reply to requestors dont unset due date with KeepInLoop' if $EN
KeepInLoop => { RealMinutes => 60*4 },
},
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -270,12 +270,12 @@ diag 'check that reply to requestors dont unset due date with KeepInLoop' if $EN
diag 'check that replies dont affect resolve deadlines' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Resolve => { RealMinutes => 60*2 } },
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -334,12 +334,12 @@ diag 'check that replies dont affect resolve deadlines' if $ENV{'TEST_VERBOSE'};
diag 'check that owner is not treated as requestor' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Response => { RealMinutes => 60*2 } },
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -367,12 +367,12 @@ diag 'check that owner is not treated as requestor' if $ENV{'TEST_VERBOSE'};
diag 'check that response deadline is left alone when there is no requestor' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Response => { RealMinutes => 60*2 } },
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
diff --git a/t/sla/ignore-on-statuses.t b/t/sla/ignore-on-statuses.t
index 3d39da8..50ff743 100644
--- a/t/sla/ignore-on-statuses.t
+++ b/t/sla/ignore-on-statuses.t
@@ -7,14 +7,14 @@ RT::Test->load_or_create_queue( Name => 'General', SLADisabled => 0 );
diag 'check that reply to requestors dont unset due date with KeepInLoop' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => {
KeepInLoop => { RealMinutes => 60*4, IgnoreOnStatuses => ['stalled'] },
},
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -89,7 +89,7 @@ diag 'check that reply to requestors dont unset due date with KeepInLoop' if $EN
diag 'Check that failing to reply to the requestors is not ignored' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => {
@@ -97,7 +97,7 @@ diag 'Check that failing to reply to the requestors is not ignored' if $ENV{'TES
KeepInLoop => { RealMinutes => 60*4, IgnoreOnStatuses => ['stalled'] },
},
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -169,14 +169,14 @@ diag 'Check that failing to reply to the requestors is not ignored' if $ENV{'TES
diag 'check the ExcludeTimeOnIgnoredStatuses option' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => {
Response => { RealMinutes => 60*2, IgnoreOnStatuses => ['stalled'] },
},
},
- );
+ ));
my $root = RT::User->new( $RT::SystemUser );
$root->LoadByEmail('root at localhost');
@@ -236,14 +236,14 @@ diag 'check the ExcludeTimeOnIgnoredStatuses option' if $ENV{'TEST_VERBOSE'};
ok $tmp == $due, "deadline not changed";
}
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => {
Response => { RealMinutes => 60*2, IgnoreOnStatuses => ['stalled'], ExcludeTimeOnIgnoredStatuses => 1 },
},
},
- );
+ ));
{
my $ticket = RT::Ticket->new( $RT::SystemUser );
$ticket->Load( $id );
diff --git a/t/sla/queue.t b/t/sla/queue.t
index 891976c..01b9b19 100644
--- a/t/sla/queue.t
+++ b/t/sla/queue.t
@@ -20,14 +20,13 @@ diag 'check set of Due date with Queue default SLA' if $ENV{'TEST_VERBOSE'};
ok( $id, 'Created SLA Attribute for General' );
- no warnings 'once';
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => '2',
Levels => {
'2' => { Resolve => { RealMinutes => 60 * 2 } },
'4' => { StartImmediately => 1, Resolve => { RealMinutes => 60 * 4 } },
},
- );
+ ));
set_fixed_time('2007-01-01T00:00:00Z');
diff --git a/t/sla/starts.t b/t/sla/starts.t
index 4e9951c..b180ff0 100644
--- a/t/sla/starts.t
+++ b/t/sla/starts.t
@@ -14,7 +14,7 @@ my $bhours = RT::SLA->BusinessHours;
diag 'check Starts date' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => 'standard',
Levels => {
'standard' => {
@@ -22,8 +22,8 @@ diag 'check Starts date' if $ENV{'TEST_VERBOSE'};
Resolve => 7 * 60 * 24,
},
},
- );
- %RT::ServiceBusinessHours = (
+ ));
+ RT->Config->Set(ServiceBusinessHours => (
Default => {
1 => {
Name => 'Monday',
@@ -36,7 +36,7 @@ diag 'check Starts date' if $ENV{'TEST_VERBOSE'};
End => '17:00'
},
}
- );
+ ));
my %time = (
'2007-01-01T13:15:00Z' => 1167657300, # 2007-01-01T13:15:00Z
@@ -57,7 +57,7 @@ diag 'check Starts date' if $ENV{'TEST_VERBOSE'};
diag 'check Starts date with StartImmediately enabled' if $ENV{'TEST_VERBOSE'};
{
- %RT::ServiceAgreements = (
+ RT->Config->Set(ServiceAgreements => (
Default => 'start immediately',
Levels => {
'start immediately' => {
@@ -66,7 +66,7 @@ diag 'check Starts date with StartImmediately enabled' if $ENV{'TEST_VERBOSE'};
Resolve => 7 * 60 * 24,
},
},
- );
+ ));
my $time = time;
my $ticket = RT::Ticket->new($RT::SystemUser);
diff --git a/t/sla/timezone.t b/t/sla/timezone.t
index 29a73b2..4f23c98 100644
--- a/t/sla/timezone.t
+++ b/t/sla/timezone.t
@@ -10,8 +10,7 @@ ok $ru_queue && $ru_queue->id, 'created RU queue';
my $us_queue = RT::Test->load_or_create_queue( Name => 'US', SLADisabled => 0 );
ok $us_queue && $ru_queue->id, 'created US queue';
-no warnings 'once';
-%RT::ServiceAgreements = (
+RT->Config->Set(ServiceAgreements => (
Default => 2,
QueueDefault => {
RU => { Timezone => 'Europe/Moscow' },
@@ -20,7 +19,7 @@ no warnings 'once';
Levels => {
'2' => { Resolve => { BusinessMinutes => 60 * 2 } },
},
-);
+));
set_fixed_time('2007-01-01T22:00:00Z');
commit c00e56a8dba7fc54f0db6986a1e74368cb7ac19b
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Thu Dec 29 02:42:27 2016 +0000
Log an error if config incorrectly specifies Sunday as 7
Sunday should be specified as day 0. Specifying it as 7 is completely
and silently ignored.
This is an easy misconfiguration to make, especially because in some
cultures Sunday is shown on calendars as the last day of the week,
rather than the first as is customary in USA.
Fixes: I#32487
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 09fd0c7..6adde8a 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -1176,6 +1176,15 @@ our %META;
ServiceBusinessHours => {
Type => 'HASH',
+ PostLoadCheck => sub {
+ my $self = shift;
+ my $config = $self->Get('ServiceBusinessHours');
+ for my $name (keys %$config) {
+ if ($config->{$name}->{7}) {
+ RT->Logger->error("Config option \%ServiceBusinessHours '$name' erroneously specifies '$config->{$name}->{7}->{Name}' as day 7; Sunday should be specified as day 0.");
+ }
+ }
+ },
},
ServiceAgreements => {
diff --git a/t/sla/business_hours.t b/t/sla/business_hours.t
index e81b0b8..c0b5475 100644
--- a/t/sla/business_hours.t
+++ b/t/sla/business_hours.t
@@ -62,4 +62,27 @@ diag 'check business hours' if $ENV{'TEST_VERBOSE'};
is( $due, 1167645600, 'Due date is 2007-01-01T10:00:00Z' );
}
+diag 'check that RT warns about specifying Sunday as 7 rather than 0' if $ENV{'TEST_VERBOSE'};
+{
+ my @warnings;
+ local $SIG{__WARN__} = sub {
+ push @warnings, $_[0];
+ };
+
+ RT->Config->Set(ServiceBusinessHours => (
+ Invalid => {
+ 7 => {
+ Name => 'Domingo',
+ Start => '9:00',
+ End => '17:00'
+ }
+ },
+ ));
+
+ RT->Config->PostLoadCheck;
+
+ is(@warnings, 1);
+ like($warnings[0], qr/Config option %ServiceBusinessHours 'Invalid' erroneously specifies 'Domingo' as day 7; Sunday should be specified as day 0\./);
+}
+
done_testing();
-----------------------------------------------------------------------
More information about the rt-commit
mailing list