[Rt-commit] rt branch, 4.2/shred-user-not-in-group, updated. rt-4.2.5-55-gc5f7226

[Alex Vandiver]

The branch, 4.2/shred-user-not-in-group has been updated
       via  c5f72267e3666e8b4bc2f62cb03bf4ad516653da (commit)
      from  865ce70987f5d13cfac970bd0aba77aafba68bfb (commit)

Summary of changes:
 lib/RT/Shredder/Plugin/Users.pm | 50 ++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

- Log -----------------------------------------------------------------
commit c5f72267e3666e8b4bc2f62cb03bf4ad516653da
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jun 19 18:37:40 2014 -0400

    Allow no_tickets and not_member_of to work together
    
    Both of the filters, as curreently written, return; thus, if
    not_member_of is used, no_tickets is ignored.
    
    Switch to a list of filters, which all must be passed and refactor out
    the loop over the objects (which was shared between
    FilterNotMemberOfGroup and FilterWithoutTickets).  This also allows the
    correct upper limit of results to be returned, as opposed to if the
    filters were run sequentially, with each imposing the upper limit -- if
    all of the first 50 non-group-members had tickets, the result set might
    be empty instead of finding the 50 non-group-members later in the set
    which had no tickets.

diff --git a/lib/RT/Shredder/Plugin/Users.pm b/lib/RT/Shredder/Plugin/Users.pm
index fb4ef4b..c7e8e75 100644
--- a/lib/RT/Shredder/Plugin/Users.pm
+++ b/lib/RT/Shredder/Plugin/Users.pm
@@ -197,22 +197,32 @@ sub Run
     if( $self->{'opt'}{'member_of'} ) {
         $objs->MemberOfGroup( $self->{'opt'}{'member_of'} );
     }
+    my @filter;
     if( $self->{'opt'}{'not_member_of'} ) {
-        return $self->FilterNotMemberOfGroup(
+        push @filter, $self->FilterNotMemberOfGroup(
             Shredder => $args{'Shredder'},
-            Objects  => $objs,
             GroupId  => $self->{'opt'}{'not_member_of'},
         );
     }
     if( $self->{'opt'}{'no_tickets'} ) {
-        return $self->FilterWithoutTickets(
+        push @filter, $self->FilterWithoutTickets(
             Shredder => $args{'Shredder'},
-            Objects  => $objs,
         );
-    } else {
-        if( $self->{'opt'}{'limit'} ) {
-            $objs->RowsPerPage( $self->{'opt'}{'limit'} );
+    }
+
+    if (@filter) {
+        $self->FetchNext( $objs, 'init' );
+        my @res;
+        USER: while ( my $user = $self->FetchNext( $objs ) ) {
+            for my $filter (@filter) {
+                next USER unless $filter->($user);
+            }
+            push @res, $user;
+            last if $self->{'opt'}{'limit'} && @res >= $self->{'opt'}{'limit'};
         }
+        $objs = \@res;
+    } elsif ( $self->{'opt'}{'limit'} ) {
+        $objs->RowsPerPage( $self->{'opt'}{'limit'} );
     }
     return (1, $objs);
 }
@@ -241,7 +251,6 @@ sub FilterNotMemberOfGroup {
     my $self = shift;
     my %args = (
         Shredder => undef,
-        Objects  => undef,
         GroupId  => undef,
         @_,
     );
@@ -249,15 +258,10 @@ sub FilterNotMemberOfGroup {
     my $group = RT::Group->new(RT->SystemUser);
     $group->Load($args{'GroupId'});
 
-    my $users = $args{Objects};
-    $self->FetchNext( $users, 'init' );
-
-    my @res;
-    while ( my $user = $self->FetchNext( $users ) ) {
-        push @res, $user unless $group->HasMemberRecursively($user->Id);
-        return (1, \@res) if $self->{'opt'}{'limit'} && @res >= $self->{'opt'}{'limit'};
-    }
-    return (1, \@res);
+    return sub {
+        my $user = shift;
+        not $group->HasMemberRecursively($user->id);
+    };
 }
 
 sub FilterWithoutTickets {
@@ -267,15 +271,11 @@ sub FilterWithoutTickets {
         Objects  => undef,
         @_,
     );
-    my $users = $args{Objects};
-    $self->FetchNext( $users, 'init' );
 
-    my @res;
-    while ( my $user = $self->FetchNext( $users ) ) {
-        push @res, $user if $self->_WithoutTickets( $user );
-        return (1, \@res) if $self->{'opt'}{'limit'} && @res >= $self->{'opt'}{'limit'};
-    }
-    return (1, \@res);
+    return sub {
+        my $user = shift;
+        $self->_WithoutTickets( $user )
+    };
 }
 
 sub _WithoutTickets {

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