[Rt-commit] rt branch, 4.4/parse-notify-argument, created. rt-4.4.3-60-gaada2ab9f
? sunnavy
sunnavy at bestpractical.com
Thu Oct 11 17:09:19 EDT 2018
The branch, 4.4/parse-notify-argument has been created
at aada2ab9ff762ef7b41d5089abfd6c8418efd927 (commit)
- Log -----------------------------------------------------------------
commit 70cb7e22c490213684b9101ac0a7d0d67d7a05df
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Oct 12 03:57:53 2018 +0800
Parse Notify action argument assuming it's comma separated
We use comma separated string in core, but the code isn't tweaked for
it, which makes it hard to support not-just-a-word custom role name like
"Foo Bar" or "'Foo, Bar'". This commits tweaks the parse process to
support much more characters(including comma) in custom role names.
diff --git a/lib/RT/Action/Notify.pm b/lib/RT/Action/Notify.pm
index 5a44a6278..c42631393 100644
--- a/lib/RT/Action/Notify.pm
+++ b/lib/RT/Action/Notify.pm
@@ -55,6 +55,7 @@ use warnings;
use base qw(RT::Action::SendEmail);
use Email::Address;
+use Regexp::Common;
=head2 Prepare
@@ -82,66 +83,44 @@ sub SetRecipients {
my $ticket = $self->TicketObj;
my $arg = $self->Argument;
- $arg =~ s/\bAll\b/Owner,Requestor,AdminCc,Cc/;
my ( @To, @PseudoTo, @Cc, @Bcc );
+ my %args = map { $_ => 1 } $self->SplitArgument;
- if ( $arg =~ /\bRequestor\b/ ) {
+ if ( $args{All} ) {
+ $args{$_} ||= 1 for qw/Owner Requestor AdminCc Cc/;
+ }
+
+ if ( $args{Requestor} ) {
push @To, $ticket->Requestors->MemberEmailAddresses;
}
# custom role syntax: gives:
- # name (undef, role name, Cc)
- # RT::CustomRole-# (role id, undef, Cc)
- # name/To (undef, role name, To)
- # RT::CustomRole-#/To (role id, undef, To)
- # name/Cc (undef, role name, Cc)
- # RT::CustomRole-#/Cc (role id, undef, Cc)
- # name/Bcc (undef, role name, Bcc)
- # RT::CustomRole-#/Bcc (role id, undef, Bcc)
+ # name (role name, Cc)
+ # RT::CustomRole-# (role with id, Cc)
+ # name/To (role name, To)
+ # RT::CustomRole-#/To (role with id, To)
+ # name/Cc (role name, Cc)
+ # RT::CustomRole-#/Cc (role with id, Cc)
+ # name/Bcc (role name, Bcc)
+ # RT::CustomRole-#/Bcc (role with id, Bcc)
# this has to happen early because adding To addresses affects how Cc
# is handled
- my $custom_role_re = qr!
- ( # $1 match everything for error reporting
-
- # word boundary
- \b
-
- # then RT::CustomRole-# or a role name
- (?:
- RT::CustomRole-(\d+) # $2 role id
- | ( \w+ ) # $3 role name
- )
-
- # then, optionally, a type after a slash
- (?:
- /
- (To | Cc | Bcc) # $4 type
- )?
-
- # finally another word boundary, either from
- # the end of role identifier or from the end of type
- \b
- )
- !x;
- while ($arg =~ m/$custom_role_re/g) {
- my ($argument, $role_id, $name, $type) = ($1, $2, $3, $4);
- my $role;
-
- if ($name) {
- # skip anything that is a core Notify argument
- next if $name eq 'All'
- || $name eq 'Owner'
- || $name eq 'Requestor'
- || $name eq 'AdminCc'
- || $name eq 'Cc'
- || $name eq 'OtherRecipients'
- || $name eq 'AlwaysNotifyActor'
- || $name eq 'NeverNotifyActor';
+ for my $item ( sort keys %args ) {
+ next if $item =~ /^(?:All|Owner|Requestor|AdminCc|Cc|OtherRecipients|AlwaysNotifyActor|NeverNotifyActor)$/;
+ my ( $name, $type ) = ( $item =~ m{^(.+?)(?:/(To|Cc|Bcc))?$} );
+ next unless $name;
+ my $role;
+ if ( $name =~ /^RT::CustomRole-(\d+)$/ ) {
+ my $id = $1;
+ $role = RT::CustomRole->new( $self->CurrentUser );
+ $role->Load( $id );
+ }
+ else {
my $roles = RT::CustomRoles->new( $self->CurrentUser );
$roles->Limit( FIELD => 'Name', VALUE => $name, CASESENSITIVE => 0 );
@@ -152,13 +131,9 @@ sub SetRecipients {
$role = undef if $roles->Next;
}
}
- else {
- $role = RT::CustomRole->new( $self->CurrentUser );
- $role->Load( $role_id );
- }
unless ($role && $role->id) {
- $RT::Logger->debug("Unable to load custom role from scrip action argument '$argument'");
+ $RT::Logger->debug("Unable to load custom role from scrip action argument '$item'");
next;
}
@@ -178,7 +153,7 @@ sub SetRecipients {
}
}
- if ( $arg =~ /\bCc\b/ ) {
+ if ( $args{Cc} ) {
#If we have a To, make the Ccs, Ccs, otherwise, promote them to To
if (@To) {
@@ -191,7 +166,7 @@ sub SetRecipients {
}
}
- if ( $arg =~ /\bOwner\b/
+ if ( $args{Owner}
&& $ticket->OwnerObj->id != RT->Nobody->id
&& $ticket->OwnerObj->EmailAddress
&& not $ticket->OwnerObj->Disabled
@@ -207,7 +182,7 @@ sub SetRecipients {
}
- if ( $arg =~ /\bAdminCc\b/ ) {
+ if ( $args{AdminCc} ) {
push ( @Bcc, $ticket->AdminCc->MemberEmailAddresses );
push ( @Bcc, $ticket->QueueObj->AdminCc->MemberEmailAddresses );
}
@@ -224,7 +199,7 @@ sub SetRecipients {
@{ $self->{'Bcc'} } = @Bcc;
@{ $self->{'PseudoTo'} } = @PseudoTo;
- if ( $arg =~ /\bOtherRecipients\b/ ) {
+ if ( $args{OtherRecipients} ) {
if ( my $attachment = $self->TransactionObj->Attachments->First ) {
push @{ $self->{'NoSquelch'}{'Cc'} ||= [] }, map $_->address,
Email::Address->parse( $attachment->GetHeader('RT-Send-Cc') );
@@ -253,18 +228,48 @@ sub RemoveInappropriateRecipients {
my $TransactionCurrentUser = RT::CurrentUser->new;
$TransactionCurrentUser->LoadByName($creatorObj->Name);
+ my %args = map { $_ => 1 } $self->SplitArgument;
+
$self->RecipientFilter(
Callback => sub {
return unless lc $_[0] eq lc $creator;
return "not sending to $creator, creator of the transaction, due to NotifyActor setting";
},
- ) if $self->Argument =~ /\bNeverNotifyActor\b/ ||
+ ) if $args{NeverNotifyActor} ||
(!RT->Config->Get('NotifyActor',$TransactionCurrentUser)
- && $self->Argument !~ /\bAlwaysNotifyActor\b/);
+ && !$args{AlwaysNotifyActor});
$self->SUPER::RemoveInappropriateRecipients();
}
+
+=head2 SplitArgument
+
+Split comma separated argument. Like CSV, it also supports quoted
+values, so values like "'foo, bar'" is treated like a single value.
+
+Return the list of the split values.
+
+=cut
+
+sub SplitArgument {
+ my $self = shift;
+ my $arg = shift // $self->Argument;
+
+ my @args;
+ while ( $arg =~ s/^($RE{quoted}|[^,]+)(?:,\s*|$)//g ) {
+ my $item = $1;
+ if ( $item =~ /^(['"])(.*)\1/ ) {
+ next unless length $2;
+ push @args, $2;
+ }
+ else {
+ push @args, $item;
+ }
+ }
+ return @args;
+}
+
RT::Base->_ImportOverlays();
1;
commit 4e4f004ce3f6a75d997aecfa97644e5efc1b9542
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Oct 12 04:41:54 2018 +0800
Fix typo
diff --git a/t/customroles/notify.t b/t/customroles/notify.t
index 98bf1c7f1..770b5b2ed 100644
--- a/t/customroles/notify.t
+++ b/t/customroles/notify.t
@@ -160,7 +160,7 @@ diag 'create scrips' if $ENV{'TEST_VERBOSE'};
ok($val, $msg);
my $s3 = RT::Scrip->new(RT->SystemUser);
- ($val, $msg) = $s2->Create(
+ ($val, $msg) = $s3->Create(
Queue => 'Specs',
ScripCondition => 'On Create',
ScripAction => 'Notify Unapplied as Bcc',
commit aada2ab9ff762ef7b41d5089abfd6c8418efd927
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Oct 12 04:24:28 2018 +0800
Test Notify action for custom roles with spaces/commas in name
diff --git a/t/customroles/notify.t b/t/customroles/notify.t
index 770b5b2ed..19d93c3ec 100644
--- a/t/customroles/notify.t
+++ b/t/customroles/notify.t
@@ -8,6 +8,8 @@ my $specs = RT::Test->load_or_create_queue( Name => 'Specs' );
my $engineer = RT::CustomRole->new(RT->SystemUser);
my $sales = RT::CustomRole->new(RT->SystemUser);
+my $designer = RT::CustomRole->new(RT->SystemUser);
+my $pre_sales = RT::CustomRole->new(RT->SystemUser);
my $unapplied = RT::CustomRole->new(RT->SystemUser);
my $linus = RT::Test->load_or_create_user( EmailAddress => 'linus at example.com' );
@@ -32,6 +34,18 @@ diag 'setup' if $ENV{'TEST_VERBOSE'};
);
ok($ok, "created Sales role: $msg");
+ ($ok, $msg) = $designer->Create(
+ Name => 'UX designer',
+ MaxValues => 1,
+ );
+ ok($ok, "created UX designer role: $msg");
+
+ ($ok, $msg) = $pre_sales->Create(
+ Name => 'Sales, Pre',
+ MaxValues => 0,
+ );
+ ok($ok, "created Sales, Pre role: $msg");
+
($ok, $msg) = $unapplied->Create(
Name => 'Unapplied',
MaxValues => 0,
@@ -44,6 +58,12 @@ diag 'setup' if $ENV{'TEST_VERBOSE'};
($ok, $msg) = $engineer->AddToObject($specs->id);
ok($ok, "added Engineer to Specs: $msg");
+ ($ok, $msg) = $designer->AddToObject($specs->id);
+ ok($ok, "added UX designer to Specs: $msg");
+
+ ($ok, $msg) = $pre_sales->AddToObject($specs->id);
+ ok($ok, "added Sales, Pre to Specs: $msg");
+
}
diag 'create tickets in Specs without scrips' if $ENV{'TEST_VERBOSE'};
@@ -138,7 +158,7 @@ diag 'create scrips' if $ENV{'TEST_VERBOSE'};
($val, $msg) = $a2->Create(
Name => 'Notify Sales as To',
ExecModule => 'Notify',
- Argument => 'RT::CustomRole-2/To',
+ Argument => 'RT::CustomRole-2/To, "Sales, Pre/Bcc"',
);
ok($val, $msg);
@@ -167,6 +187,23 @@ diag 'create scrips' if $ENV{'TEST_VERBOSE'};
Template => 'Admin Correspondence',
);
ok($val, $msg);
+
+ my $a4 = RT::ScripAction->new(RT->SystemUser);
+ ($val, $msg) = $a4->Create(
+ Name => 'Notify UX Designer as Cc',
+ ExecModule => 'Notify',
+ Argument => 'UX Designer',
+ );
+ ok($val, $msg);
+
+ my $s4 = RT::Scrip->new(RT->SystemUser);
+ ($val, $msg) = $s4->Create(
+ Queue => 'Specs',
+ ScripCondition => 'On Create',
+ ScripAction => 'Notify UX Designer as Cc',
+ Template => 'Correspondence',
+ );
+ ok($val, $msg);
}
diag 'create tickets in Specs with scrips' if $ENV{'TEST_VERBOSE'};
@@ -200,9 +237,11 @@ diag 'create tickets in Specs with scrips' if $ENV{'TEST_VERBOSE'};
Subject => 'oops',
Owner => $ricky,
$engineer->GroupType => $linus,
+ $designer->GroupType => $moss,
);
} { To => $ricky->EmailAddress, Cc => '', Bcc => '' },
- { To => '', Cc => $linus->EmailAddress, Bcc => '' };
+ { To => '', Cc => $linus->EmailAddress, Bcc => '' },
+ { To => '', Cc => $moss->EmailAddress, Bcc => '' };
mail_ok {
RT::Test->create_ticket(
@@ -211,10 +250,11 @@ diag 'create tickets in Specs with scrips' if $ENV{'TEST_VERBOSE'};
Owner => $ricky,
$engineer->GroupType => $linus,
$sales->GroupType => [$blake->EmailAddress],
+ $pre_sales->GroupType => [$williamson->EmailAddress],
);
} { To => $ricky->EmailAddress, Cc => '', Bcc => '' },
{ To => '', Cc => $linus->EmailAddress, Bcc => '' },
- { To => $blake->EmailAddress, Cc => '', Bcc => '' };
+ { To => $blake->EmailAddress, Cc => '', Bcc => $williamson->EmailAddress };
mail_ok {
RT::Test->create_ticket(
-----------------------------------------------------------------------
More information about the rt-commit
mailing list