[Rt-commit] rt branch, 4.0-trunk, updated. rt-4.0.2-171-g593059f
Thomas Sibley
trs at bestpractical.com
Mon Oct 3 15:34:59 EDT 2011
The branch, 4.0-trunk has been updated
via 593059f31c3e850a48e32bc1f6235e046d3b6ca6 (commit)
via 2723ed9a14826aed97a051b2e50e4c3bbad63fdc (commit)
via 1a12222db7587c2b3b28b9d0cb9a7a73fb261f3b (commit)
via 02416a1f6df35a2c9cfba298fa7f4dd831b5517d (commit)
via 4fa0cab2f46d8f86b9b210c9e2c389681aef3620 (commit)
via 0dc38eef82130590134210365e2826166b6d5130 (commit)
via ea8e2b033e49dbc11a86219768f502c51830af66 (commit)
from 858d144d1a82e492fc571ab822cd2575cbc37f6d (commit)
Summary of changes:
lib/RT/Links.pm | 57 ++++++++++++---------
lib/RT/Ticket.pm | 22 ++++++--
share/html/Elements/ShowLinks | 6 ++-
t/api/link.t | 33 ++++++++++++-
t/web/ticket_links.t | 110 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 195 insertions(+), 33 deletions(-)
create mode 100644 t/web/ticket_links.t
- Log -----------------------------------------------------------------
commit ea8e2b033e49dbc11a86219768f502c51830af66
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon May 9 14:08:01 2011 +0800
ticket links test
diff --git a/t/web/ticket_links.t b/t/web/ticket_links.t
new file mode 100644
index 0000000..4071dde
--- /dev/null
+++ b/t/web/ticket_links.t
@@ -0,0 +1,85 @@
+use strict;
+use warnings;
+use RT::Test tests => 73;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok( $m->login, "Logged in" );
+
+my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+ok( $queue->Id, "loaded the General queue" );
+
+my ( $deleted, $active, $inactive ) = RT::Test->create_tickets(
+ { Queue => 'General' },
+ { Subject => 'deleted ticket', },
+ { Subject => 'active ticket', },
+ { Subject => 'inactive ticket', }
+);
+
+my ( $deleted_id, $active_id, $inactive_id ) =
+ ( $deleted->id, $active->id, $inactive->id );
+
+$deleted->SetStatus('deleted');
+is( $deleted->Status, 'deleted', "deleted $deleted_id" );
+
+$inactive->SetStatus('resolved');
+is( $inactive->Status, 'resolved', 'resolved $inactive_id' );
+
+for my $type ( "DependsOn", "MemberOf", "RefersTo" ) {
+ for my $c (qw/base target/) {
+ my $ticket = RT::Test->create_ticket(
+ Queue => 'General',
+ Subject => "test $type $c",
+ );
+ my $id = $ticket->id;
+
+ $m->goto_ticket($id);
+ $m->follow_link_ok( { text => 'Links' }, "Followed link to Links" );
+
+ ok( $m->form_with_fields("$id-DependsOn"), "found the form" );
+ if ( $c eq 'base' ) {
+ $m->field( "$id-$type", "$deleted_id $active_id $inactive_id" );
+ }
+ else {
+ $m->field( "$type-$id", "$deleted_id $active_id $inactive_id" );
+ }
+ $m->submit;
+
+ if ( $c eq 'base' ) {
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$active_id"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$inactive_id"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink--$type-.*?ticket/$deleted_id"},
+ "base for $type: no deleted ticket",
+ );
+ }
+ else {
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$active_id-$type-"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$inactive_id-$type-"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink-.*?ticket/$deleted_id-$type-"},
+ "base for $type: no deleted ticket",
+ );
+ }
+
+ $m->goto_ticket($id);
+ $m->content_like( qr{$active_id:.*?\[new\]}, "has active ticket", );
+ $m->content_like(
+ qr{$inactive_id:.*?\[resolved\]},
+ "has inactive ticket",
+ );
+ $m->content_unlike( qr{$deleted_id.*?\[deleted\]}, "no deleted ticket", );
+ }
+}
+
commit 0dc38eef82130590134210365e2826166b6d5130
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue May 10 12:37:03 2011 +0800
others use Links->Next, let's do that too. besides, Links->ItemsArrayRef includes deleted tickets, which need fix too
diff --git a/share/html/Elements/ShowLinks b/share/html/Elements/ShowLinks
index c35cabc..6a06d6a 100755
--- a/share/html/Elements/ShowLinks
+++ b/share/html/Elements/ShowLinks
@@ -54,8 +54,10 @@
</td>
<td class="value">
<%PERL>
-my ( @active, @inactive, @not_tickets );
-for my $link ( @{ $Ticket->DependsOn->ItemsArrayRef } ) {
+my ( $depends_on, @active, @inactive, @not_tickets );
+$depends_on = $Ticket->DependsOn;
+
+while ( my $link = $depends_on->Next ) {
my $target = $link->TargetObj;
if ( $target && $target->isa('RT::Ticket') ) {
if ( $target->QueueObj->IsInactiveStatus( $target->Status ) ) {
commit 4fa0cab2f46d8f86b9b210c9e2c389681aef3620
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon May 9 15:29:40 2011 +0800
test linknig deleted tickets
diff --git a/t/web/ticket_links.t b/t/web/ticket_links.t
index 4071dde..ffa83ee 100644
--- a/t/web/ticket_links.t
+++ b/t/web/ticket_links.t
@@ -1,12 +1,12 @@
use strict;
use warnings;
-use RT::Test tests => 73;
+use RT::Test tests => 97;
my ( $baseurl, $m ) = RT::Test->started_ok;
ok( $m->login, "Logged in" );
my $queue = RT::Test->load_or_create_queue( Name => 'General' );
-ok( $queue->Id, "loaded the General queue" );
+ok( $queue->id, "loaded the General queue" );
my ( $deleted, $active, $inactive ) = RT::Test->create_tickets(
{ Queue => 'General' },
@@ -26,51 +26,75 @@ is( $inactive->Status, 'resolved', 'resolved $inactive_id' );
for my $type ( "DependsOn", "MemberOf", "RefersTo" ) {
for my $c (qw/base target/) {
- my $ticket = RT::Test->create_ticket(
- Queue => 'General',
- Subject => "test $type $c",
- );
- my $id = $ticket->id;
+ my $id;
- $m->goto_ticket($id);
- $m->follow_link_ok( { text => 'Links' }, "Followed link to Links" );
+ diag "create ticket with links of type $type $c";
+ {
+ ok( $m->goto_create_ticket($queue), "go to create ticket" );
+ $m->form_name('TicketCreate');
+ $m->field( Subject => "test ticket creation with $type $c" );
+ if ( $c eq 'base' ) {
+ $m->field( "new-$type", "$deleted_id $active_id $inactive_id" );
+ }
+ else {
+ $m->field( "$type-new", "$deleted_id $active_id $inactive_id" );
+ }
- ok( $m->form_with_fields("$id-DependsOn"), "found the form" );
- if ( $c eq 'base' ) {
- $m->field( "$id-$type", "$deleted_id $active_id $inactive_id" );
+ $m->submit;
+ $m->content_like(qr/Ticket \d+ created/, 'created ticket');
+ $m->content_contains("Can't link to a deleted ticket");
+ $id = RT::Test->last_ticket->id;
}
- else {
- $m->field( "$type-$id", "$deleted_id $active_id $inactive_id" );
- }
- $m->submit;
- if ( $c eq 'base' ) {
- $m->content_like(
- qr{"DeleteLink--$type-.*?ticket/$active_id"},
- "$c for $type: has active ticket",
- );
- $m->content_like(
- qr{"DeleteLink--$type-.*?ticket/$inactive_id"},
- "base for $type: has inactive ticket",
- );
- $m->content_unlike(
- qr{"DeleteLink--$type-.*?ticket/$deleted_id"},
- "base for $type: no deleted ticket",
- );
- }
- else {
- $m->content_like(
- qr{"DeleteLink-.*?ticket/$active_id-$type-"},
- "$c for $type: has active ticket",
- );
- $m->content_like(
- qr{"DeleteLink-.*?ticket/$inactive_id-$type-"},
- "base for $type: has inactive ticket",
- );
- $m->content_unlike(
- qr{"DeleteLink-.*?ticket/$deleted_id-$type-"},
- "base for $type: no deleted ticket",
+ diag "add ticket links of type $type $c";
+ {
+ my $ticket = RT::Test->create_ticket(
+ Queue => 'General',
+ Subject => "test $type $c",
);
+ $id = $ticket->id;
+
+ $m->goto_ticket($id);
+ $m->follow_link_ok( { text => 'Links' }, "Followed link to Links" );
+
+ ok( $m->form_with_fields("$id-DependsOn"), "found the form" );
+ if ( $c eq 'base' ) {
+ $m->field( "$id-$type", "$deleted_id $active_id $inactive_id" );
+ }
+ else {
+ $m->field( "$type-$id", "$deleted_id $active_id $inactive_id" );
+ }
+ $m->submit;
+ $m->content_contains("Can't link to a deleted ticket");
+
+ if ( $c eq 'base' ) {
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$active_id"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$inactive_id"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink--$type-.*?ticket/$deleted_id"},
+ "base for $type: no deleted ticket",
+ );
+ }
+ else {
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$active_id-$type-"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$inactive_id-$type-"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink-.*?ticket/$deleted_id-$type-"},
+ "base for $type: no deleted ticket",
+ );
+ }
}
$m->goto_ticket($id);
@@ -79,7 +103,8 @@ for my $type ( "DependsOn", "MemberOf", "RefersTo" ) {
qr{$inactive_id:.*?\[resolved\]},
"has inactive ticket",
);
- $m->content_unlike( qr{$deleted_id.*?\[deleted\]}, "no deleted ticket", );
+ $m->content_unlike( qr{$deleted_id.*?\[deleted\]}, "no deleted ticket",
+ );
}
}
commit 02416a1f6df35a2c9cfba298fa7f4dd831b5517d
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon May 9 15:28:59 2011 +0800
forbid to link to deleted tickets, as we don't show them in page either. see also #17013
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 3ad34cc..16225ec 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -613,20 +613,27 @@ sub Create {
foreach my $link (
ref( $args{$type} ) ? @{ $args{$type} } : ( $args{$type} ) )
{
+ my ( $val, $msg, $obj ) = $self->__GetTicketFromURI( URI => $link );
+ unless ($val) {
+ push @non_fatal_errors, $msg;
+ next;
+ }
+
# Check rights on the other end of the link if we must
# then run _AddLink that doesn't check for ACLs
if ( RT->Config->Get( 'StrictLinkACL' ) ) {
- my ($val, $msg, $obj) = $self->__GetTicketFromURI( URI => $link );
- unless ( $val ) {
- push @non_fatal_errors, $msg;
- next;
- }
if ( $obj && !$obj->CurrentUserHasRight('ModifyTicket') ) {
push @non_fatal_errors, $self->loc('Linking. Permission denied');
next;
}
}
-
+
+ if ( $obj && $obj->Status eq 'deleted' ) {
+ push @non_fatal_errors,
+ $self->loc("Linking. Can't link to a deleted ticket");
+ next;
+ }
+
my ( $wval, $wmsg ) = $self->_AddLink(
Type => $LINKTYPEMAP{$type}->{'Type'},
$LINKTYPEMAP{$type}->{'Mode'} => $link,
@@ -2498,6 +2505,9 @@ sub AddLink {
return ( 0, $self->loc("Permission Denied") );
}
+ return ( 0, "Can't link to a deleted ticket" )
+ if $other_ticket && $other_ticket->Status eq 'deleted';
+
return $self->_AddLink(%args);
}
commit 1a12222db7587c2b3b28b9d0cb9a7a73fb261f3b
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue May 10 14:09:15 2011 +0800
deleted ticket links test on api level
diff --git a/t/api/link.t b/t/api/link.t
index b110fc7..903b587 100644
--- a/t/api/link.t
+++ b/t/api/link.t
@@ -2,7 +2,7 @@
use strict;
use warnings;
-use RT::Test nodata => 1, tests => 83;
+use RT::Test nodata => 1, tests => 87;
use RT::Test::Web;
use RT::Link;
@@ -209,8 +209,39 @@ ok $cid, 'created a ticket #'. $cid or diag "error: $msg";
;
}
+{
+ clean_links();
+ $child->SetStatus('deleted');
+
+ my ($status, $msg) = $parent->AddLink(
+ Type => 'MemberOf', Base => $child->id,
+ );
+ ok(!$status, "can't link to deleted ticket: $msg");
+
+ $child->SetStatus('new');
+ ($status, $msg) = $parent->AddLink(
+ Type => 'MemberOf', Base => $child->id,
+ );
+ ok($status, "created a link: $msg");
+
+ $child->SetStatus('deleted');
+ my $children = $parent->Members;
+ $children->RedoSearch;
+
+ my $total = 0;
+ $total++ while $children->Next;
+ is( $total, 0, 'Next skips deleted tickets' );
+
+ is( @{ $children->ItemsArrayRef },
+ 0, 'ItemsArrayRef skips deleted tickets' );
+
+ # back to active status
+ $child->SetStatus('new');
+}
+
sub clean_links {
my $links = RT::Links->new( RT->SystemUser );
+ $links->UnLimit;
while ( my $link = $links->Next ) {
my ($status, $msg) = $link->Delete;
$RT::Logger->error("Couldn't delete a link: $msg")
commit 2723ed9a14826aed97a051b2e50e4c3bbad63fdc
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri May 13 11:56:22 2011 +0800
hack AddRecord to make both ->Next and ->ItemsArrayRef work; Count needs Search still though
diff --git a/lib/RT/Links.pm b/lib/RT/Links.pm
index df510c6..07f8774 100644
--- a/lib/RT/Links.pm
+++ b/lib/RT/Links.pm
@@ -136,30 +136,6 @@ sub LimitReferredToBy {
$self->Limit(FIELD => 'Base', VALUE => $URI);
}
-
-
-sub Next {
- my $self = shift;
-
- my $Link = $self->SUPER::Next();
- return $Link unless $Link && ref $Link;
-
- # skip very invalid Link records
- unless ($Link->Target && $Link->Base) {
- return $self->Next;
- }
- # Skip links to local objects thast are deleted
- if ( $Link->TargetURI->IsLocal and UNIVERSAL::isa($Link->TargetObj,"RT::Ticket")
- and $Link->TargetObj->__Value('status') eq "deleted") {
- return $self->Next;
- } elsif ($Link->BaseURI->IsLocal and UNIVERSAL::isa($Link->BaseObj,"RT::Ticket")
- and $Link->BaseObj->__Value('status') eq "deleted") {
- return $self->Next;
- } else {
- return $Link;
- }
-}
-
# }}}
=head2 NewItem
@@ -172,6 +148,39 @@ sub NewItem {
my $self = shift;
return(RT::Link->new($self->CurrentUser));
}
+
+sub AddRecord {
+ my $self = shift;
+ my $record = shift;
+ return unless $self->IsValidLink($record);
+
+ push @{$self->{'items'}}, $record;
+ $self->{'rows'}++;
+}
+
+=head2 IsValidLink
+
+if linked to a local ticket and is deleted, then the link is invalid.
+
+=cut
+
+sub IsValidLink {
+ my $self = shift;
+ my $link = shift;
+
+ return unless $link && ref $link && $link->Target && $link->Base;
+
+ # Skip links to local objects thast are deleted
+ return
+ if $link->TargetURI->IsLocal
+ && ( UNIVERSAL::isa( $link->TargetObj, "RT::Ticket" )
+ && $link->TargetObj->__Value('status') eq "deleted"
+ || UNIVERSAL::isa( $link->BaseObj, "RT::Ticket" )
+ && $link->BaseObj->__Value('status') eq "deleted" );
+
+ return 1;
+}
+
RT::Base->_ImportOverlays();
1;
commit 593059f31c3e850a48e32bc1f6235e046d3b6ca6
Merge: 858d144 2723ed9
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Oct 3 15:32:28 2011 -0400
Merge branch '4.0/forbid-linking-deleted-tickets' into 4.0-trunk
Conflicts:
t/api/link.t
test counts
t/web/ticket_links.t
fix test count for new create_tickets tests on trunk
diff --cc t/api/link.t
index 228ff91,903b587..a9e54a7
--- a/t/api/link.t
+++ b/t/api/link.t
@@@ -1,9 -1,9 +1,9 @@@
use strict;
use warnings;
- use RT::Test nodata => 1, tests => 80;
-use RT::Test nodata => 1, tests => 87;
++use RT::Test nodata => 1, tests => 84;
use RT::Test::Web;
+use Test::Warn;
use RT::Link;
my $link = RT::Link->new(RT->SystemUser);
diff --cc t/web/ticket_links.t
index 0000000,ffa83ee..cb30e92
mode 000000,100644..100644
--- a/t/web/ticket_links.t
+++ b/t/web/ticket_links.t
@@@ -1,0 -1,110 +1,110 @@@
+ use strict;
+ use warnings;
-use RT::Test tests => 97;
++use RT::Test tests => 106;
+
+ my ( $baseurl, $m ) = RT::Test->started_ok;
+ ok( $m->login, "Logged in" );
+
+ my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+ ok( $queue->id, "loaded the General queue" );
+
+ my ( $deleted, $active, $inactive ) = RT::Test->create_tickets(
+ { Queue => 'General' },
+ { Subject => 'deleted ticket', },
+ { Subject => 'active ticket', },
+ { Subject => 'inactive ticket', }
+ );
+
+ my ( $deleted_id, $active_id, $inactive_id ) =
+ ( $deleted->id, $active->id, $inactive->id );
+
+ $deleted->SetStatus('deleted');
+ is( $deleted->Status, 'deleted', "deleted $deleted_id" );
+
+ $inactive->SetStatus('resolved');
+ is( $inactive->Status, 'resolved', 'resolved $inactive_id' );
+
+ for my $type ( "DependsOn", "MemberOf", "RefersTo" ) {
+ for my $c (qw/base target/) {
+ my $id;
+
+ diag "create ticket with links of type $type $c";
+ {
+ ok( $m->goto_create_ticket($queue), "go to create ticket" );
+ $m->form_name('TicketCreate');
+ $m->field( Subject => "test ticket creation with $type $c" );
+ if ( $c eq 'base' ) {
+ $m->field( "new-$type", "$deleted_id $active_id $inactive_id" );
+ }
+ else {
+ $m->field( "$type-new", "$deleted_id $active_id $inactive_id" );
+ }
+
+ $m->submit;
+ $m->content_like(qr/Ticket \d+ created/, 'created ticket');
+ $m->content_contains("Can't link to a deleted ticket");
+ $id = RT::Test->last_ticket->id;
+ }
+
+ diag "add ticket links of type $type $c";
+ {
+ my $ticket = RT::Test->create_ticket(
+ Queue => 'General',
+ Subject => "test $type $c",
+ );
+ $id = $ticket->id;
+
+ $m->goto_ticket($id);
+ $m->follow_link_ok( { text => 'Links' }, "Followed link to Links" );
+
+ ok( $m->form_with_fields("$id-DependsOn"), "found the form" );
+ if ( $c eq 'base' ) {
+ $m->field( "$id-$type", "$deleted_id $active_id $inactive_id" );
+ }
+ else {
+ $m->field( "$type-$id", "$deleted_id $active_id $inactive_id" );
+ }
+ $m->submit;
+ $m->content_contains("Can't link to a deleted ticket");
+
+ if ( $c eq 'base' ) {
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$active_id"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink--$type-.*?ticket/$inactive_id"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink--$type-.*?ticket/$deleted_id"},
+ "base for $type: no deleted ticket",
+ );
+ }
+ else {
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$active_id-$type-"},
+ "$c for $type: has active ticket",
+ );
+ $m->content_like(
+ qr{"DeleteLink-.*?ticket/$inactive_id-$type-"},
+ "base for $type: has inactive ticket",
+ );
+ $m->content_unlike(
+ qr{"DeleteLink-.*?ticket/$deleted_id-$type-"},
+ "base for $type: no deleted ticket",
+ );
+ }
+ }
+
+ $m->goto_ticket($id);
+ $m->content_like( qr{$active_id:.*?\[new\]}, "has active ticket", );
+ $m->content_like(
+ qr{$inactive_id:.*?\[resolved\]},
+ "has inactive ticket",
+ );
+ $m->content_unlike( qr{$deleted_id.*?\[deleted\]}, "no deleted ticket",
+ );
+ }
+ }
+
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list