[Rt-commit] rt branch, 4.0/forbid-linking-deleted-tickets, updated. rt-4.0.0-193-g5e3240c

[? sunnavy]

The branch, 4.0/forbid-linking-deleted-tickets has been updated
       via  5e3240c8ef340fbe4d593bca01c88766afa8cc96 (commit)
       via  ee40b505f9b44b160504a2f44fc187ec8c461d7d (commit)
       via  445e338d16e7a561b13af02eea70a7291915e0f3 (commit)
      from  81fdc254820c40bf41aa61b73a8189b6cd2ae787 (commit)

Summary of changes:
 lib/RT/Links.pm               |   53 +++++++++++++++++++++++++++++-----------
 share/html/Elements/ShowLinks |    7 +++--
 t/api/link.t                  |   33 ++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 19 deletions(-)

- Log -----------------------------------------------------------------
commit 445e338d16e7a561b13af02eea70a7291915e0f3
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 6f330d3..6a06d6a 100755
--- a/share/html/Elements/ShowLinks
+++ b/share/html/Elements/ShowLinks
@@ -54,11 +54,12 @@
     </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') ) {
-        next if $target->Status eq 'deleted';
         if ( $target->QueueObj->IsInactiveStatus( $target->Status ) ) {
             push( @inactive, $link->TargetURI );
         }

commit ee40b505f9b44b160504a2f44fc187ec8c461d7d
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue May 10 14:02:06 2011 +0800

    ItemsArrayRef should return non-deleted objects too

diff --git a/lib/RT/Links.pm b/lib/RT/Links.pm
index df510c6..ef81d48 100644
--- a/lib/RT/Links.pm
+++ b/lib/RT/Links.pm
@@ -140,24 +140,12 @@ sub LimitReferredToBy {
 
 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;
-    }
+    return $Link if $self->IsValidLink( $Link );
+    return $self->Next;
 }
 
 # }}}
@@ -172,6 +160,41 @@ sub NewItem {
     my $self = shift;
     return(RT::Link->new($self->CurrentUser));
 }
+
+=head2 ItemsArrayRef
+
+Returns a reference to the set of all valid links found in this search.
+
+=cut
+
+sub ItemsArrayRef {
+    my $self = shift;
+    return [ grep { $self->IsValidLink($_) } @{ $self->SUPER::ItemsArrayRef } ];
+}
+
+=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 5e3240c8ef340fbe4d593bca01c88766afa8cc96
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")

-----------------------------------------------------------------------