[Rt-commit] rt branch, 4.2/fixup-default-scrips, created. rt-4.1.8-559-g301e415
Thomas Sibley
trs at bestpractical.com
Mon Jul 1 16:35:00 EDT 2013
The branch, 4.2/fixup-default-scrips has been created
at 301e415fcc0ac5d19428bb600102a48baa446c73 (commit)
- Log -----------------------------------------------------------------
commit 264db7d8588af8c9774cc4ded53da8ab685bacf8
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Mar 3 13:02:38 2011 -0500
Make sure that On Create we notify Ccs and other recipients
Otherwise the ticket create form is a partial lie. I'm not sure how
this wasn't noticed for so long.
This probably also wants an upgrade step, but I'm not sure of the best
way to do that at the moment.
diff --git a/etc/initialdata b/etc/initialdata
index 29b9163..f30edf7 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -722,6 +722,14 @@ Hour: { $SubscriptionObj->SubValue('Hour') }
ScripCondition => 'On Create',
ScripAction => 'Notify AdminCcs',
Template => 'Transaction in HTML' },
+ { Description => 'On Create Notify Ccs',
+ ScripCondition => 'On Create',
+ ScripAction => 'Notify Ccs',
+ Template => 'Correspondence in HTML' },
+ { Description => 'On Create Notify Other Recipients',
+ ScripCondition => 'On Create',
+ ScripAction => 'Notify Other Recipients',
+ Template => 'Correspondence in HTML' },
{ Description => 'On Owner Change Notify Owner',
ScripCondition => 'On Owner Change',
ScripAction => 'Notify Owner',
commit e909ac3379d0349e063a7e0ccbff4733d4a58f94
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Sep 27 11:12:40 2012 -0700
Add a Notify Owner and AdminCcs action
Combining notification of Owner and AdminCcs into the same action
receives the benefit of duplicate recipient squashing if the Owner is
(as commonly is the case) a queue-level AdminCc.
diff --git a/etc/initialdata b/etc/initialdata
index f30edf7..e2ae031 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -58,7 +58,10 @@
Description => 'Sends mail to the administrative Ccs', # loc
ExecModule => 'Notify',
Argument => 'AdminCc' },
-
+ { Name => 'Notify Owner and AdminCcs', # loc
+ Description => 'Sends mail to the Owner and administrative Ccs', # loc
+ ExecModule => 'Notify',
+ Argument => 'Owner,AdminCc' },
{ Name => 'Notify Requestors and Ccs as Comment', # loc
Description => 'Send mail to requestors and Ccs as a comment', # loc
ExecModule => 'NotifyAsComment',
commit 9fcdd1bda3f376a54521b57384e14277a2a65542
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Sep 27 11:18:17 2012 -0700
Notify Owner too on Create and Correspond, not just AdminCcs
Otherwise Owners who aren't a queue AdminCc (or also a ticket level
AdminCc) don't receive mail!
This needs a smart upgrade path, similar to 63852e9, which adds the
action if it doesn't exist already and modifies the default scrips if
they still exist. It also needs a note in UPGRADING-4.2.
diff --git a/etc/initialdata b/etc/initialdata
index e2ae031..bbccab5 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -701,9 +701,9 @@ Hour: { $SubscriptionObj->SubValue('Hour') }
ScripCondition => 'On Comment',
ScripAction => 'Notify Other Recipients As Comment',
Template => 'Correspondence in HTML' },
- { Description => 'On Correspond Notify AdminCcs',
+ { Description => 'On Correspond Notify Owner and AdminCcs',
ScripCondition => 'On Correspond',
- ScripAction => 'Notify AdminCcs',
+ ScripAction => 'Notify Owner and AdminCcs',
Template => 'Admin Correspondence in HTML' },
{ Description => 'On Correspond Notify Other Recipients',
ScripCondition => 'On Correspond',
@@ -721,9 +721,9 @@ Hour: { $SubscriptionObj->SubValue('Hour') }
ScripCondition => 'On Create',
ScripAction => 'AutoReply To Requestors',
Template => 'AutoReply in HTML' },
- { Description => 'On Create Notify AdminCcs',
+ { Description => 'On Create Notify Owner and AdminCcs',
ScripCondition => 'On Create',
- ScripAction => 'Notify AdminCcs',
+ ScripAction => 'Notify Owner and AdminCcs',
Template => 'Transaction in HTML' },
{ Description => 'On Create Notify Ccs',
ScripCondition => 'On Create',
commit 8cf495e243fea410675ee8b0fe8bfe1213c8665b
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jun 6 13:39:20 2013 -0700
Add the new action "Notify Owner and AdminCcs" on upgrade
Rather than try to automatically upgrade scrips, make a note of the
changes and let admins manually update if they'd like.
diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 2a5bc12..f17463f 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -129,3 +129,7 @@ UPGRADING FROM RT 4.0.0 and greater
and granting Everyone the OwnTicket right is a common cause of
performance problems. Unprivileged Owners (if they exist) may still
be set using the Autocompleter.
+
+* New installs will notify Ccs and one-time Ccs/Bccs on create and Owners on
+ create and correspond. Upgraded installations will not. If you'd like to
+ adjust your scrips, the actions are available from the admin scrip pages.
diff --git a/etc/upgrade/4.1.15/content b/etc/upgrade/4.1.15/content
new file mode 100644
index 0000000..c1d8a61
--- /dev/null
+++ b/etc/upgrade/4.1.15/content
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+our @ScripActions = (
+ { Name => 'Notify Owner and AdminCcs', # loc
+ Description => 'Sends mail to the Owner and administrative Ccs', # loc
+ ExecModule => 'Notify',
+ Argument => 'Owner,AdminCc' },
+);
commit 2929fa6af8283104c50d3acaeec636593d2a5e99
Author: Thomas Sibley <trs at bestpractical.com>
Date: Wed Jun 12 11:52:49 2013 -0700
Fix broken assumptions about scrips in tests
Caused by the new scrip action and new recipients of the default scrips.
diff --git a/t/mail/sendmail-plaintext.t b/t/mail/sendmail-plaintext.t
index 6bb8bb7..39f26cb 100644
--- a/t/mail/sendmail-plaintext.t
+++ b/t/mail/sendmail-plaintext.t
@@ -2,7 +2,7 @@ use strict;
use warnings;
use File::Spec ();
-use RT::Test tests => 142, text_templates => 1;
+use RT::Test tests => undef, text_templates => 1;
use RT::EmailParser;
use RT::Tickets;
@@ -61,7 +61,7 @@ like (first_txn($tick)->Content , qr/The original message was received/, "It's t
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
undef @scrips_fired;
@@ -89,7 +89,7 @@ ok ($tick->Id, "found ticket ".$tick->Id);
is ($tick->Subject , 'I18NTest', "failed to create the new ticket from an unprivileged account");
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
# make sure it sends a notification to adminccs
@@ -124,7 +124,7 @@ like (first_txn($tick)->Content , qr/H\x{e5}vard/, "It's signed by havard. yay")
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
@@ -166,7 +166,7 @@ like (first_txn($tick)->Content , qr/H\x{e5}vard/, "It's signed by havard. yay")
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
@@ -544,3 +544,5 @@ diag q{regression test for #5248 from rt3.fsck.com};
# Don't taint the environment
$everyone->PrincipalObj->RevokeRight(Right =>'SuperUser');
+
+done_testing;
diff --git a/t/mail/sendmail.t b/t/mail/sendmail.t
index 327d2ad..4cce776 100644
--- a/t/mail/sendmail.t
+++ b/t/mail/sendmail.t
@@ -2,7 +2,7 @@ use strict;
use warnings;
use File::Spec ();
-use RT::Test tests => 174;
+use RT::Test tests => undef;
use RT::EmailParser;
use RT::Tickets;
@@ -61,7 +61,7 @@ like (first_txn($tick)->Content , qr/The original message was received/, "It's t
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
undef @scrips_fired;
@@ -89,7 +89,7 @@ ok ($tick->Id, "found ticket ".$tick->Id);
is ($tick->Subject , 'I18NTest', "failed to create the new ticket from an unprivileged account");
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
# make sure it sends a notification to adminccs
@@ -124,7 +124,7 @@ like (first_txn($tick)->Content , qr/H\x{e5}vard/, "It's signed by havard. yay")
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
@@ -166,7 +166,7 @@ like (first_txn($tick)->Content , qr/H\x{e5}vard/, "It's signed by havard. yay")
# make sure it fires scrips.
-is ($#scrips_fired, 1, "Fired 2 scrips on ticket creation");
+is (scalar @scrips_fired, 4, "Fired 4 scrips on ticket creation");
# make sure it sends an autoreply
@@ -556,3 +556,5 @@ diag q{regression test for #5248 from rt3.fsck.com};
# Don't taint the environment
$everyone->PrincipalObj->RevokeRight(Right =>'SuperUser');
+
+done_testing;
diff --git a/t/web/crypt-gnupg.t b/t/web/crypt-gnupg.t
index 220a202..1ad633a 100644
--- a/t/web/crypt-gnupg.t
+++ b/t/web/crypt-gnupg.t
@@ -2,7 +2,7 @@ use strict;
use warnings;
use RT::Test::GnuPG
- tests => 101,
+ tests => undef,
gnupg_options => {
passphrase => 'recipient',
'trust-model' => 'always',
@@ -351,8 +351,13 @@ $nokey->PrincipalObj->GrantRight(Right => 'CreateTicket');
$nokey->PrincipalObj->GrantRight(Right => 'OwnTicket');
my $tick = RT::Ticket->new( RT->SystemUser );
-$tick->Create(Subject => 'owner lacks pubkey', Queue => 'general',
- Owner => $nokey);
+warning_like {
+ $tick->Create(Subject => 'owner lacks pubkey', Queue => 'general',
+ Owner => $nokey);
+} [
+ qr/nokey\@example.com: skipped: public key not found/,
+ qr/Recipient 'nokey\@example.com' is unusable/,
+];
ok(my $id = $tick->id, 'created ticket for owner-without-pubkey');
$tick = RT::Ticket->new( RT->SystemUser );
@@ -456,3 +461,6 @@ like($content, qr/KR-nokey \(no pubkey!\)-K/,
$m->next_warning_like(qr/public key not found/);
$m->next_warning_like(qr/public key not found/);
$m->no_leftover_warnings_ok;
+
+undef $m;
+done_testing;
diff --git a/t/web/scrips.t b/t/web/scrips.t
index c1b9782..d669f4c 100644
--- a/t/web/scrips.t
+++ b/t/web/scrips.t
@@ -60,8 +60,8 @@ sub prepare_code_with_value {
$m->form_name('CreateScrip');
$m->set_fields(
'ScripCondition' => $condition,
- 'ScripAction' => 15, # User Defined
- 'Template' => 1, # Blank
+ 'ScripAction' => 'User Defined',
+ 'Template' => 'Blank',
'CustomPrepareCode' => $prepare_code,
);
$m->click('Create');
commit 301e415fcc0ac5d19428bb600102a48baa446c73
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Jun 18 17:51:28 2013 -0700
Suppress duplicate create notifications in the ___Approvals queue
Approval tests caught the duplicate notifications which occur via the
new owner on create scrip + existing NewPending rule. Until rules are
replaced by appropriate scrips, squash duplicates by suppressing the
scrip's notification via an empty template.
Unlike Owners which were notified by the rule, AdminCcs always received
notifications via the scrip (before it also included Owner). They now
receive notifications via the same rule as Owners, as originally
intended judging by the rule's description. This lets us suppress the
duplicate Owner notifications above while retaining the standard AdminCc
notifications. It also means Owners and AdminCcs are notified of new
approvals using a single template instead of two unrelated templates.
Tests also needed adjustment for the extra notifications sent to Owners
on correspond, but these are expected.
diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index f17463f..853f7da 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -133,3 +133,7 @@ UPGRADING FROM RT 4.0.0 and greater
* New installs will notify Ccs and one-time Ccs/Bccs on create and Owners on
create and correspond. Upgraded installations will not. If you'd like to
adjust your scrips, the actions are available from the admin scrip pages.
+
+* Notifications to AdminCcs on approvals are now handled via the New Pending
+ Approval template in the hidden ___Approvals queue. If you customized the
+ Transaction template, you should port your changes to New Pending Approval.
diff --git a/etc/initialdata b/etc/initialdata
index bbccab5..c5fd428 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -317,6 +317,16 @@ Content-Type: text/html
{$Transaction->Content( Type => "text/html")}
'
},
+ # Shadow the global templates of the same name to suppress duplicate
+ # notifications until rules is ripped out.
+ { Queue => "___Approvals",
+ Name => "Transaction in HTML",
+ Content => "",
+ },
+ { Queue => "___Approvals",
+ Name => "Transaction",
+ Content => "",
+ },
{
Queue => '0',
diff --git a/etc/upgrade/4.1.15/content b/etc/upgrade/4.1.15/content
index c1d8a61..3e1f1d5 100644
--- a/etc/upgrade/4.1.15/content
+++ b/etc/upgrade/4.1.15/content
@@ -7,3 +7,16 @@ our @ScripActions = (
ExecModule => 'Notify',
Argument => 'Owner,AdminCc' },
);
+
+our @Templates = (
+ # Shadow the global templates of the same name to suppress duplicate
+ # notifications until rules is ripped out.
+ { Queue => "___Approvals",
+ Name => "Transaction in HTML",
+ Content => "",
+ },
+ { Queue => "___Approvals",
+ Name => "Transaction",
+ Content => "",
+ },
+);
diff --git a/lib/RT/Approval/Rule/NewPending.pm b/lib/RT/Approval/Rule/NewPending.pm
index 97d3cfb..2054c53 100644
--- a/lib/RT/Approval/Rule/NewPending.pm
+++ b/lib/RT/Approval/Rule/NewPending.pm
@@ -75,7 +75,7 @@ sub Commit {
# first txn entry of the approval ticket
local $self->{TransactionObj} = $to;
- $self->RunScripAction('Notify Owner', 'New Pending Approval', @_);
+ $self->RunScripAction('Notify Owner and AdminCcs', 'New Pending Approval', @_);
return;
diff --git a/t/approval/admincc.t b/t/approval/admincc.t
index 22a360a..3be4a4b 100644
--- a/t/approval/admincc.t
+++ b/t/approval/admincc.t
@@ -9,7 +9,7 @@ BEGIN {
use RT;
-use RT::Test tests => 62;
+use RT::Test tests => "no_declare";
use RT::Test::Email;
RT->Config->Set( LogToSTDERR => 'debug' );
@@ -87,13 +87,12 @@ mail_ok {
to => 'minion at company.com',
subject => qr/PO for stationary/,
body => qr/automatically generated in response/
-},
-{ from => qr/RT System/,
- bcc => qr/ceo.*coo|coo.*ceo/i,
- subject => qr/PO for stationary/i,
-},
-{ from => qr/RT System/,
+},{ from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/PO for stationary/,
+},{ from => qr/RT System/,
to => 'cto at company.com',
+ bcc => qr/ceo.*coo|coo.*ceo/i,
subject => qr/New Pending Approval: CTO Approval/,
body => qr/pending your approval.*Your approval is requested.*Blah/s
}
@@ -134,6 +133,11 @@ mail_ok {
},
{
from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/Ticket Approved:/,
+},
+{
+ from => qr/RT System/,
to => 'minion at company.com',
subject => qr/Ticket Approved:/,
body => qr/approved by CTO.*notes: Resources exist to be consumed/s
@@ -180,6 +184,12 @@ for my $admin (qw/coo ceo/) {
body => qr/Resources exist to be consumed/,
},
{
+ from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/Ticket Approved:/,
+ body => qr/approved by \U$admin\E.*notes: Resources exist to be consumed/s
+ },
+ {
from => qr/RT System/,
to => 'minion at company.com',
subject => qr/Ticket Approved:/,
@@ -273,3 +283,6 @@ $m_coo->content_lacks( 'second approval', 'coo: second approval is gone too' );
$m_ceo->content_lacks( 'second approval', 'ceo: second approval is gone too' );
RT::Test->clean_caught_mails;
+
+undef $m;
+done_testing;
diff --git a/t/approval/basic.t b/t/approval/basic.t
index 45b0313..0e88b74 100644
--- a/t/approval/basic.t
+++ b/t/approval/basic.t
@@ -4,7 +4,6 @@ use RT::Test tests => undef;
BEGIN {
plan skip_all => 'Email::Abstract and Test::Email required.'
unless eval { require Email::Abstract; require Test::Email; 1 };
- plan tests => 38;
}
use RT::Test::Email;
@@ -88,6 +87,9 @@ mail_ok {
to => 'minion at company.com',
subject => qr/PO for stationary/,
body => qr/automatically generated in response/
+},{ from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/PO for stationary/,
}, { from => qr/RT System/,
to => 'cfo at company.com',
subject => qr/New Pending Approval: CFO Approval/,
@@ -135,6 +137,9 @@ mail_ok {
subject => qr/New Pending Approval: PO approval request for PO/,
body => qr/pending your approval.*CFO approved.*ok with that\?/s
},{ from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/Ticket Approved:/,
+},{ from => qr/RT System/,
to => 'minion at company.com',
subject => qr/Ticket Approved:/,
body => qr/approved by CFO.*notes: Resources exist to be consumed/s
@@ -163,10 +168,14 @@ mail_ok {
ok($ok, "ceo can approve - $msg");
} { from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/Ticket Approved:/,
+ body => qr/approved by CEO.*Its Owner may now start to act on it.*notes: And consumed they will be/s,
+},{ from => qr/RT System/,
to => 'minion at company.com',
subject => qr/Ticket Approved:/,
body => qr/approved by CEO.*Its Owner may now start to act on it.*notes: And consumed they will be/s,
-}, { from => qr/CEO via RT/,
+},{ from => qr/CEO via RT/,
to => 'root at localhost',
subject => qr/Ticket Approved/,
body => qr/The ticket has been approved, you may now start to act on it/,
@@ -201,6 +210,10 @@ mail_ok {
ok($ok, "cfo can approve - $msg");
} { from => qr/RT System/,
+ to => 'root at localhost',
+ subject => qr/Ticket Rejected: PO for stationary/,
+ body => qr/rejected by CFO.*out of resources/s,
+},{ from => qr/RT System/,
to => 'minion at company.com',
subject => qr/Ticket Rejected: PO for stationary/,
body => qr/rejected by CFO.*out of resources/s,
@@ -210,3 +223,4 @@ $t->Load($t->id);$dependson_ceo->Load($dependson_ceo->id);
is_deeply([ $t->Status, $dependson_cfo->Status, $dependson_ceo->Status ],
[ 'rejected', 'rejected', 'deleted'], 'ticket state after cfo rejection');
+done_testing;
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list