[Rt-commit] r3944 - in rt/branches/3.4-RELEASE/lib: RT t/regression

ruz at bestpractical.com ruz at bestpractical.com
Mon Oct 10 15:27:37 EDT 2005


Author: ruz
Date: Mon Oct 10 15:27:36 2005
New Revision: 3944

Added:
   rt/branches/3.4-RELEASE/lib/t/regression/22search_tix_by_watcher.t
Modified:
   rt/branches/3.4-RELEASE/lib/RT/Ticket_Overlay.pm
   rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay.pm
   rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay_SQL.pm
Log:
backport of the 3.5-TESTING at 3943
Changes
* fix for search by owner's fields, now owner is WATCHERFIELD instead of ENUM
* added backward compatible variant for Owner, next searches should work
** Owner = '<id>'
** Owner != '<id>'
** Owner = '<name>'
** Owner != '<name>'
** for other operators or if subfield(subkey) is specified search works
   as for other watchers
* Fix for searches like "Cc.Name <> 'SomeBody'", was skipping tickets
  with empty Cc list.
* get rid of some unint warnings
* test suite for all corner cases


Modified: rt/branches/3.4-RELEASE/lib/RT/Ticket_Overlay.pm
==============================================================================
--- rt/branches/3.4-RELEASE/lib/RT/Ticket_Overlay.pm	(original)
+++ rt/branches/3.4-RELEASE/lib/RT/Ticket_Overlay.pm	Mon Oct 10 15:27:36 2005
@@ -3482,7 +3482,7 @@
     
         #If we can't actually set the field to the value, don't record
         # a transaction. instead, get out of here.
-        if ( $ret == 0 ) { return ( 0, $msg ); }
+        return ( 0, $msg ) unless $ret;
     }
 
     if ( $args{'RecordTransaction'} == 1 ) {

Modified: rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay.pm
==============================================================================
--- rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay.pm	(original)
+++ rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay.pm	Mon Oct 10 15:27:36 2005
@@ -103,7 +103,7 @@
     Type            => [ 'ENUM', ],
     Creator         => [ 'ENUM' => 'User', ],
     LastUpdatedBy   => [ 'ENUM' => 'User', ],
-    Owner           => [ 'ENUM' => 'User', ],
+    Owner           => [ 'WATCHERFIELD' => 'Owner', ],
     EffectiveId     => [ 'INT', ],
     id              => [ 'INT', ],
     InitialPriority => [ 'INT', ],
@@ -159,7 +159,7 @@
     LINKFIELD       => \&_LinkFieldLimit,
     CUSTOMFIELD     => \&_CustomFieldLimit,
 );
-my %can_bundle = ( WATCHERFIELD => "yeps", );
+my %can_bundle = ( WATCHERFIELD => "yes", );
 
 # Default EntryAggregator per type
 # if you specify OP, you must specify all valid OPs
@@ -791,7 +791,6 @@
     my $value = shift;
     my %rest  = (@_);
 
-    $self->_OpenParen;
 
     # Find out what sort of watcher we're looking for
     my $fieldname;
@@ -800,76 +799,113 @@
     }
     else {
         $fieldname = $field;
+        $field = [[$field, $op, $value, %rest]]; # gross hack
     }
     my $meta = $FIELDS{$fieldname};
     my $type = ( defined $meta->[1] ? $meta->[1] : undef );
 
-# We only want _one_ clause for all of requestors, cc, admincc
-# It's less flexible than what we used to do, but now it sort of actually works. (no huge cartesian products that hose the db)
-    my $groups = $self->{ 'watcherlimit_' . ('global') . "_groups" } ||=
-      $self->NewAlias('Groups');
-    my $groupmembers =
-      $self->{ 'watcherlimit_' . ('global') . "_groupmembers" } ||=
-      $self->NewAlias('CachedGroupMembers');
-    my $users = $self->{ 'watcherlimit_' . ('global') . "_users" } ||=
-      $self->NewAlias('Users');
+    # Owner was ENUM field, so "Owner = 'xxx'" allowed user to
+    # search by id and Name at the same time, this is workaround
+    # to preserve backward compatibility
+    if( $fieldname eq 'Owner' ) {
+        my $flag = 0;
+        for my $chunk (splice @$field) {
+            my ( $f, $op, $value, %rest ) = @$chunk;
+            if( !$rest{SUBKEY} && $op =~ /^!?=$/ ) {
+                $self->_OpenParen unless $flag++;
+                my $o = RT::User->new( $self->CurrentUser );
+                $o->Load($value);
+                $value = $o->Id;
+                $self->_SQLLimit(
+                    FIELD    => 'Owner',
+                    OPERATOR => $op,
+                    VALUE    => $value,
+                    %rest,
+                );
+            } else {
+                push @$field, $chunk;
+            }
+        }
+        $self->_CloseParen if $flag;
+        return unless @$field;
+    }
 
-# Use regular joins instead of SQL joins since we don't want the joins inside ticketsql or we get a huge cartesian product
-    $self->SUPER::Limit(
-        ALIAS           => $groups,
-        FIELD           => 'Domain',
-        VALUE           => 'RT::Ticket-Role',
-        ENTRYAGGREGATOR => 'AND'
-    );
-    $self->Join(
-        ALIAS1 => $groups,
-        FIELD1 => 'Instance',
-        ALIAS2 => 'main',
-        FIELD2 => 'id'
-    );
-    $self->Join(
-        ALIAS1 => $groups,
-        FIELD1 => 'id',
-        ALIAS2 => $groupmembers,
-        FIELD2 => 'GroupId'
-    );
-    $self->Join(
-        ALIAS1 => $groupmembers,
-        FIELD1 => 'MemberId',
-        ALIAS2 => $users,
-        FIELD2 => 'id'
-    );
+    my $users = $self->_WatcherJoin($type);
 
     # If we're looking for multiple watchers of a given type,
     # TicketSQL will be handing it to us as an array of clauses in
     # $field
-    if ( ref $field ) {    # gross hack
-        $self->_OpenParen;
-        for my $chunk (@$field) {
-            ( $field, $op, $value, %rest ) = @$chunk;
-            $self->_SQLLimit(
-                ALIAS         => $users,
-                FIELD         => $rest{SUBKEY} || 'EmailAddress',
-                VALUE         => $value,
-                OPERATOR      => $op,
-                CASESENSITIVE => 0,
-                %rest
-            );
-        }
-        $self->_CloseParen;
-    }
-    else {
+    $self->_OpenParen;
+    for my $chunk (@$field) {
+        ( $field, $op, $value, %rest ) = @$chunk;
+        $rest{SUBKEY} ||= 'EmailAddress';
+        
+        my $re_negative_op = qr[!=|NOT LIKE];
+        $self->_OpenParen if $op =~ /$re_negative_op/;
+
         $self->_SQLLimit(
             ALIAS         => $users,
-            FIELD         => $rest{SUBKEY} || 'EmailAddress',
+            FIELD         => $rest{SUBKEY},
             VALUE         => $value,
             OPERATOR      => $op,
             CASESENSITIVE => 0,
             %rest
         );
+
+        if( $op =~ /$re_negative_op/ ) {
+            $self->_SQLLimit(
+                ALIAS           => $users,
+                FIELD           => $rest{SUBKEY},
+                OPERATOR        => 'IS',
+                VALUE           => 'NULL',
+                ENTRYAGGREGATOR => 'OR',
+            );
+            $self->_CloseParen;
+        }
     }
+    $self->_CloseParen;
+}
 
-    $self->_SQLLimit(
+=head2 _WatcherJoin
+
+Helper function which provides joins to a watchers table both for limits
+and for ordering.
+
+=cut
+
+sub _WatcherJoin {
+    my $self  = shift;
+    my $type  = shift;
+
+    # we cache joins chain per watcher type
+    # if we limit by requestor then we shouldn't join requestors again
+    # for sort or limit on other requestors
+    if( $self->{'_watcher_join_users_alias'}{$type||'any'} ) {
+        return $self->{'_watcher_join_users_alias'}{$type||'any'};
+    }
+
+    # we always have watcher groups for ticket
+    # this join should be NORMAL
+    # XXX: if we change this from Join to NewAlias+Limit
+    # then Pg will complain because SB build wrong query.
+    # Query looks like "FROM (Tickets LEFT JOIN CGM ON(Groups.id = CGM.GroupId)), Groups"
+    # Pg doesn't like that fact that it doesn't know about Groups table yet when
+    # join CGM table into Tickets. Problem is in Join method which doesn't use
+    # ALIAS1 argument when build braces.
+    my $groups = $self->Join(
+        ALIAS1 => 'main',
+        FIELD1 => 'id',
+        TABLE2 => 'Groups',
+        FIELD2 => 'Instance',
+        ENTRYAGGREGATOR => 'AND'
+    );
+    $self->SUPER::Limit(
+        ALIAS           => $groups,
+        FIELD           => 'Domain',
+        VALUE           => 'RT::Ticket-Role',
+        ENTRYAGGREGATOR => 'AND'
+    );
+    $self->SUPER::Limit(
         ALIAS           => $groups,
         FIELD           => 'Type',
         VALUE           => $type,
@@ -877,7 +913,35 @@
       )
       if ($type);
 
-    $self->_CloseParen;
+    my $groupmembers = $self->Join(
+        TYPE   => 'LEFT',
+        ALIAS1 => $groups,
+        FIELD1 => 'id',
+        TABLE2 => 'CachedGroupMembers',
+        FIELD2 => 'GroupId'
+    );
+    # XXX: work around, we must hide groups that
+    # are members of the role group we search in,
+    # otherwise them result in wrong NULLs in Users
+    # table and break ordering. Now, we know that
+    # RT doesn't allow to add groups as members of the
+    # ticket roles, so we just hide entries in CGM table
+    # with MemberId == GroupId from results
+    my $groupmembers = $self->SUPER::Limit(
+        LEFTJOIN => $groupmembers,
+        FIELD => 'GroupId',
+        OPERATOR => '!=',
+        VALUE => "$groupmembers.MemberId",
+        QUOTEVALUE => 0,
+    );
+    my $users = $self->Join(
+        TYPE   => 'LEFT',
+        ALIAS1 => $groupmembers,
+        FIELD1 => 'MemberId',
+        TABLE2 => 'Users',
+        FIELD2 => 'id'
+    );
+    return $self->{'_watcher_join_users_alias'}{$type||'any'} = $users;
 }
 
 =head2 _WatcherMembershipLimit
@@ -1265,11 +1329,13 @@
 
 # If we're looking at the effective id, we don't want to append the other clause
 # which limits us to tickets where id = effective id
-    if ( $args{'FIELD'} eq 'EffectiveId' ) {
+    if ( $args{'FIELD'} eq 'EffectiveId' &&
+         (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
         $self->{'looking_at_effective_id'} = 1;
     }
 
-    if ( $args{'FIELD'} eq 'Type' ) {
+    if ( $args{'FIELD'} eq 'Type' &&
+         (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
         $self->{'looking_at_type'} = 1;
     }
 

Modified: rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay_SQL.pm
==============================================================================
--- rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay_SQL.pm	(original)
+++ rt/branches/3.4-RELEASE/lib/RT/Tickets_Overlay_SQL.pm	Mon Oct 10 15:27:36 2005
@@ -85,11 +85,13 @@
 sub _SQLLimit {
   my $self = shift;
     my %args = (@_);
-    if ($args{'FIELD'} eq 'EffectiveId') {
+    if ($args{'FIELD'} eq 'EffectiveId' &&
+         (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
         $self->{'looking_at_effective_id'} = 1;
     }      
     
-    if ($args{'FIELD'} eq 'Type') {
+    if ($args{'FIELD'} eq 'Type' &&
+         (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
         $self->{'looking_at_type'} = 1;
     }
 
@@ -288,7 +290,7 @@
       #    print "$ea Key=[$key] op=[$op]  val=[$val]\n";
 
 
-   my $subkey;
+   my $subkey = '';
    if ($key =~ /^(.+?)\.(.+)$/) {
      $key = $1;
      $subkey = $2;

Added: rt/branches/3.4-RELEASE/lib/t/regression/22search_tix_by_watcher.t
==============================================================================
--- (empty file)
+++ rt/branches/3.4-RELEASE/lib/t/regression/22search_tix_by_watcher.t	Mon Oct 10 15:27:36 2005
@@ -0,0 +1,215 @@
+#!/usr/bin/perl -w
+use strict;
+use warnings;
+
+use Test::More qw/no_plan/;
+use_ok('RT');
+RT::LoadConfig();
+RT::Init();
+use RT::Ticket;
+
+my $q = RT::Queue->new($RT::SystemUser);
+my $queue = 'SearchTests-'.rand(200);
+$q->Create(Name => $queue);
+
+my @data = (
+    { Subject => '1', Requestor => 'bravo at example.com' },
+    { Subject => '2', Cc => 'alpha at example.com' },
+);
+
+my $total = 0;
+
+sub add_tix_from_data {
+    my @res = ();
+    while (@data) {
+        my $t = RT::Ticket->new($RT::SystemUser);
+        my ( $id, undef $msg ) = $t->Create(
+            Queue => $q->id,
+            %{ shift(@data) },
+        );
+        ok( $id, "ticket created" ) or diag("error: $msg");
+        push @res, $t;
+        $total++;
+    }
+    return @res;
+}
+add_tix_from_data();
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue'");
+    is($tix->Count, $total, "found $total tickets");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Requestor = 'bravo\@example.com'");
+    is($tix->Count, 1, "found ticket(s)");
+    is($tix->First->RequestorAddresses, 'bravo at example.com',"correct requestor");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Cc = 'alpha\@example.com'");
+    is($tix->Count, 1, "found ticket(s)");
+    is($tix->First->CcAddresses, 'alpha at example.com', "correct Cc");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND (Cc = 'alpha\@example.com' OR Requestor = 'bravo\@example.com')");
+    is($tix->Count, 2, "found ticket(s)");
+    my @mails;
+    while (my $t = $tix->Next) {
+        push @mails, $t->RequestorAddresses;
+        push @mails, $t->CcAddresses;
+    }
+    @mails = sort grep $_, @mails;
+    is_deeply(\@mails, ['alpha at example.com', 'bravo at example.com'], "correct addresses");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND (Cc = 'alpha\@example.com' AND Requestor = 'bravo\@example.com')");
+    is($tix->Count, 0, "found ticket(s)");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Cc != 'alpha\@example.com'");
+    is($tix->Count, 1, "found ticket(s)");
+    is($tix->First->RequestorAddresses, 'bravo at example.com',"correct requestor");
+}
+
+ at data = ( { Subject => '3' } );
+add_tix_from_data();
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Cc != 'alpha\@example.com'");
+    is($tix->Count, 2, "found ticket(s)");
+    my @mails;
+    while (my $t = $tix->Next) { push @mails, ($t->CcAddresses||'') }
+    is( scalar(grep 'alpha at example.com' eq $_, @mails), 0, "no tickets with non required data");
+}
+
+{
+    # has no requestor search
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Requestor IS NULL");
+    is($tix->Count, 2, "found ticket(s)");
+    my @mails;
+    while (my $t = $tix->Next) { push @mails, ($t->RequestorAddresses||'') }
+    is( scalar(grep $_, @mails), 0, "no tickets with non required data");
+}
+
+{
+    # has at least one requestor search
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Requestor IS NOT NULL");
+    is($tix->Count, 1, "found ticket(s)");
+    my @mails;
+    while (my $t = $tix->Next) { push @mails, ($t->RequestorAddresses||'') }
+    is( scalar(grep !$_, @mails), 0, "no tickets with non required data");
+}
+
+ at data = ( { Subject => '3', Requestor => 'charly at example.com' } );
+add_tix_from_data();
+
+{
+    # has no requestor search
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND
+                   (Requestor = 'bravo\@example.com' OR Requestor = 'charly\@example.com')");
+    is($tix->Count, 2, "found ticket(s)");
+    my @mails;
+    while (my $t = $tix->Next) { push @mails, ($t->RequestorAddresses||'') }
+    is_deeply( [sort @mails],
+               ['bravo at example.com', 'charly at example.com'],
+               "requestor addresses are correct"
+             );
+}
+
+# owner is special watcher because reference is duplicated in two places,
+# owner was an ENUM field now it's WATCHERFIELD, but should support old
+# style ENUM searches for backward compatibility
+my $nobody = RT::Nobody();
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner = '". $nobody->id ."'");
+    is($tix->Count, 4, "found ticket(s)");
+}
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner = '". $nobody->Name ."'");
+    is($tix->Count, 4, "found ticket(s)");
+}
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner != '". $nobody->id ."'");
+    is($tix->Count, 0, "found ticket(s)");
+}
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner != '". $nobody->Name ."'");
+    is($tix->Count, 0, "found ticket(s)");
+}
+
+{
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner.Name LIKE 'nob'");
+    is($tix->Count, 4, "found ticket(s)");
+}
+
+{
+    # create ticket and force type to not a 'ticket' value
+    # bug #6898 at rt3.fsck.com
+    # and http://marc.theaimsgroup.com/?l=rt-devel&m=112662934627236&w=2
+    @data = ( { Subject => 'not a ticket' } );
+    my($t) = add_tix_from_data();
+    $t->_Set( Field             => 'Type',
+              Value             => 'not a ticket',
+              CheckACL          => 0,
+              RecordTransaction => 0,
+            );
+    $total--;
+
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND Owner = 'Nobody'");
+    is($tix->Count, 4, "found ticket(s)");
+}
+
+{
+    my $everyone = RT::Group->new( $RT::SystemUser );
+    $everyone->LoadSystemInternalGroup('Everyone');
+    ok($everyone->id, "loaded 'everyone' group");
+    my($id, $msg) = $everyone->PrincipalObj->GrantRight( Right => 'OwnTicket',
+                                                         Object => $q
+                                                       );
+    ok($id, "granted OwnTicket right to Everyone on '$queue'") or diag("error: $msg");
+
+    my $u = RT::User->new( $RT::SystemUser );
+    $u->LoadByCols( EmailAddress => 'alpha at example.com' );
+    ok($u->id, "loaded user");
+    @data = ( { Subject => '4', Owner => $u->id } );
+    my($t) = add_tix_from_data();
+    is( $t->Owner, $u->id, "created ticket with custom owner" );
+    my $u_alpha_id = $u->id;
+
+    $u = RT::User->new( $RT::SystemUser );
+    $u->LoadByCols( EmailAddress => 'bravo at example.com' );
+    ok($u->id, "loaded user");
+    @data = ( { Subject => '5', Owner => $u->id } );
+    ($t) = add_tix_from_data();
+    is( $t->Owner, $u->id, "created ticket with custom owner" );
+    my $u_bravo_id = $u->id;
+
+    my $tix = RT::Tickets->new($RT::SystemUser);
+    $tix->FromSQL("Queue = '$queue' AND
+                   ( Owner = '$u_alpha_id' OR
+                     Owner = '$u_bravo_id' )"
+                 );
+    is($tix->Count, 2, "found ticket(s)");
+}
+
+exit(0)


More information about the Rt-commit mailing list