[Rt-commit] rt branch, 4.2/referenced-queues, created. rt-4.2.11-15-g39df7b8

? sunnavy sunnavy at bestpractical.com
Wed May 20 08:15:13 EDT 2015


The branch, 4.2/referenced-queues has been created
        at  39df7b8c1001705698a405f06e148d91b276b389 (commit)

- Log -----------------------------------------------------------------
commit ed9a8e59e1454a7f3bccd1347ac6fd2dcc161058
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 17:38:26 2015 -0500

    Properly walk the Queue parts of the TicketSQL in GetReferencedQueues
    
    Previously, GetReferencedQueues simply OR'd the positive Queue
    references of the query together.  This did not take into account ACLs,
    negative restrictions, or AND'd limits.
    
    Walk the parsed query tree, removing nodes that do not reference Queue
    limits, and removing empty AND/OR nodes; this produces a tree which
    details which queues can possibly match.  Applying this tree to a
    RT::Queues object produces ground-truth on what queues the matching
    tickets can possibly be from.
    
    This information can be used to trim the Owners and Status drop-downs
    even when no queue have been selected.  For backwards compatibility, the
    set of possibly-matching CFs is only shown if a Queue limit is found.

diff --git a/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
index d7de61c..3189d85 100644
--- a/lib/RT/Interface/Web/QueryBuilder/Tree.pm
+++ b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
@@ -90,34 +90,90 @@ sub TraversePrePost {
    $postfunc->($self) if $postfunc;
 }
 
-=head2 GetReferencedQueues
+=head2 GetReferencedQueues [C<CurrentUser> => I<USER>]
 
-Returns a hash reference; each queue referenced with an '=' operation
-will appear as a key whose value is 1.
+Returns a hash reference; the keys are the ids of each queue which
+results may appear in, and the values are the respective queue objects.
+
+In array context, returns a true/false value as a second return value,
+which is if any explicit queue limits were found.
 
 =cut
 
 sub GetReferencedQueues {
     my $self = shift;
+    my %args = (
+        CurrentUser => undef,
+        @_,
+    );
 
-    my $queues = {};
-
-    $self->traverse(
+    my $q_refs = $self->clone;
+    $q_refs->TraversePrePost(
         sub {
             my $node = shift;
-
-            return if $node->isRoot;
-            return unless $node->isLeaf;
-
             my $clause = $node->getNodeValue();
-            return unless $clause->{Key} eq 'Queue';
-            return unless $clause->{Op} eq '=';
-
-            $queues->{ $clause->{RawValue} } = 1;
+            return unless $node->isLeaf and ref $clause;
+            return if $clause->{Key} eq 'Queue';
+
+            # This is a leaf node not dealing with queues; remove it
+            if ($node->isRoot) {
+                $node->setNodeValue(0);
+            } else {
+                $node->getParent->removeChild($node);
+                $node->DESTROY;
+            }
+        },
+        sub {
+            my $node = shift;
+            my $clause = $node->getNodeValue();
+            return unless $node->isLeaf and not ref $clause;
+
+            # This is a AND/OR node with no children
+            if ($node->isRoot) {
+                $node->setNodeValue(0);
+            } else {
+                $node->getParent->removeChild($node);
+                $node->DESTROY;
+            }
+        }
+    );
+    my $limits = 0;
+    my $queues = RT::Queues->new( $args{CurrentUser} );
+    $q_refs->TraversePrePost(
+        sub {
+            my $node = shift;
+            if ($node->isLeaf) {
+                my $clause = $node->getNodeValue();
+                return unless $clause;
+                $queues->Limit(
+                    FIELD    => ($clause->{RawValue} =~ /\D/ ? "Name" : "id"),
+                    CASESENSITIVE => ($clause->{RawValue} =~ /\D/ ? 0 : 1),
+                    OPERATOR => $clause->{Op},
+                    VALUE    => $clause->{RawValue},
+                    ENTRYAGGREGATOR => (
+                        $node->isRoot ? "OR" :
+                        $node->getParent->getNodeValue),
+                    SUBCLAUSE => "referenced",
+                );
+                $limits++;
+            } else {
+                $queues->_OpenParen("referenced");
+            }
+        },
+        sub {
+            my $node = shift;
+            return if $node->isLeaf;
+            $queues->_CloseParen("referenced");
         }
     );
+    $queues->UnLimit unless $limits;
+
+    my %queues;
+    while (my $q = $queues->Next) {
+        $queues{$q->id} = $q;
+    }
 
-    return $queues;
+    return wantarray ? (\%queues, $limits) : \%queues;
 }
 
 =head2 GetQueryAndOptionList SELECTED_NODES
diff --git a/lib/RT/Report/Tickets.pm b/lib/RT/Report/Tickets.pm
index 19bca18..2120017 100644
--- a/lib/RT/Report/Tickets.pm
+++ b/lib/RT/Report/Tickets.pm
@@ -153,24 +153,20 @@ our %GROUPINGS_META = (
             my $args = shift;
 
 
-            my $queues = $args->{'Queues'};
-            if ( !$queues && $args->{'Query'} ) {
+            unless ( $args->{Queues} ) {
                 require RT::Interface::Web::QueryBuilder::Tree;
                 my $tree = RT::Interface::Web::QueryBuilder::Tree->new('AND');
                 $tree->ParseSQL( Query => $args->{'Query'}, CurrentUser => $self->CurrentUser );
-                $queues = $args->{'Queues'} = $tree->GetReferencedQueues;
+                @$args{qw/Queues LimitedQueues/} = $tree->GetReferencedQueues( CurrentUser => $self->CurrentUser );
             }
-            return () unless $queues;
 
             my @res;
 
             my $CustomFields = RT::CustomFields->new( $self->CurrentUser );
-            foreach my $id (keys %$queues) {
-                my $queue = RT::Queue->new( $self->CurrentUser );
-                $queue->Load($id);
-                next unless $queue->id;
-
-                $CustomFields->LimitToQueue($queue->id);
+            if ($args->{LimitedQueues}) {
+                foreach my $queue (values %{ $args->{Queues} }) {
+                    $CustomFields->LimitToQueue($queue->id);
+                }
             }
             $CustomFields->LimitToGlobal;
             while ( my $CustomField = $CustomFields->Next ) {
diff --git a/share/html/Elements/SelectOwner b/share/html/Elements/SelectOwner
index 731de45..6331d76 100644
--- a/share/html/Elements/SelectOwner
+++ b/share/html/Elements/SelectOwner
@@ -60,11 +60,7 @@ if ($TicketObj) {
 } elsif ($QueueObj) {
     @objects = ($QueueObj);
 } elsif (%Queues) {
-    for my $name (keys %Queues) {
-        my $q = RT::Queue->new($session{'CurrentUser'});
-        $q->Load($name);
-        push @objects, $q;
-    }
+    push @objects, values %Queues;
 } else {
     # Let's check rights on an empty queue object. that will do a search
     # for any queue.
diff --git a/share/html/Search/Build.html b/share/html/Search/Build.html
index e5ba586..df4c476 100644
--- a/share/html/Search/Build.html
+++ b/share/html/Search/Build.html
@@ -78,7 +78,7 @@
 
 
 <div id="pick-criteria">
-    <& Elements/PickCriteria, query => $query{'Query'}, queues => $queues &>
+    <& Elements/PickCriteria, query => $query{'Query'}, queues => $queues, LimitedQueues => $limited_queues &>
 </div>
 <& /Elements/Submit,  Label => loc('Add these terms'), SubmitId => 'AddClause', Name => 'AddClause'&>
 <& /Elements/Submit, Label => loc('Add these terms and Search'), SubmitId => 'DoSearch', Name => 'DoSearch'&>
@@ -277,8 +277,7 @@ my $optionlist = join "\n", map { qq(<option value="$_->{INDEX}" $_->{SELECTED}>
                                   . (" " x (5 * $_->{DEPTH}))
                                   . $m->interp->apply_escapes($_->{TEXT}, 'h') . qq(</option>) } @$optionlist_arrayref;
 
-
-my $queues = $tree->GetReferencedQueues;
+my ($queues, $limited_queues) = $tree->GetReferencedQueues( CurrentUser => $session{CurrentUser} );
 
 # Deal with format changes
 my ( $AvailableColumns, $CurrentFormat );
@@ -286,6 +285,7 @@ my ( $AvailableColumns, $CurrentFormat );
     'Elements/BuildFormatString',
     %ARGS,
     queues => $queues,
+    LimitedQueues => $limited_queues,
     Format => $query{'Format'},
 );
 
diff --git a/share/html/Search/Elements/BuildFormatString b/share/html/Search/Elements/BuildFormatString
index 47e8094..4a634c3 100644
--- a/share/html/Search/Elements/BuildFormatString
+++ b/share/html/Search/Elements/BuildFormatString
@@ -104,11 +104,7 @@ my @fields = qw(
 $m->callback( CallbackOnce => 1, CallbackName => 'SetFieldsOnce', Fields => \@fields );
 
 my $CustomFields = RT::CustomFields->new( $session{'CurrentUser'});
-foreach my $id (keys %queues) {
-    # Gotta load up the $queue object, since queues get stored by name now.
-    my $queue = RT::Queue->new($session{'CurrentUser'});
-    $queue->Load($id);
-    next unless $queue->Id;
+foreach my $queue (values %queues) {
     $CustomFields->LimitToQueue($queue->Id);
     $CustomFields->SetContextObject( $queue ) if keys %queues == 1;
 }
diff --git a/share/html/Search/Elements/PickCriteria b/share/html/Search/Elements/PickCriteria
index e248422..b8f1d40 100644
--- a/share/html/Search/Elements/PickCriteria
+++ b/share/html/Search/Elements/PickCriteria
@@ -51,10 +51,10 @@
 
 
 % $m->callback( %ARGS, CallbackName => "BeforeBasics" );
-<& PickBasics, queues => \%queues &>
-<& PickTicketCFs, queues => \%queues &>
-<& PickObjectCFs, Class => 'Transaction', queues => \%queues &>
-<& PickObjectCFs, Class => 'Queue', queues => \%queues &>
+<& PickBasics, queues => \%queues, LimitedQueues => $LimitedQueues &>
+<& PickTicketCFs, queues => \%queues, LimitedQueues => $LimitedQueues &>
+<& PickObjectCFs, Class => 'Transaction', queues => \%queues, LimitedQueues => $LimitedQueues &>
+<& PickObjectCFs, Class => 'Queue', queues => \%queues, LimitedQueues => $LimitedQueues &>
 % $m->callback( %ARGS, CallbackName => "AfterCFs" );
 
 <tr class="separator"><td colspan="3"><hr /></td></tr>
@@ -72,4 +72,5 @@
 $addquery => 0
 $query => undef
 %queues => ()
+$LimitedQueues => 0
 </%ARGS>
diff --git a/share/html/Search/Elements/PickObjectCFs b/share/html/Search/Elements/PickObjectCFs
index 1a67338..80e8232 100644
--- a/share/html/Search/Elements/PickObjectCFs
+++ b/share/html/Search/Elements/PickObjectCFs
@@ -48,6 +48,7 @@
 <%ARGS>
 $Class
 %queues => ()
+$LimitedQueues => 0
 </%ARGS>
 <%init>
 my $CustomFields = RT::CustomFields->new( $session{'CurrentUser'} );
@@ -55,12 +56,11 @@ $CustomFields->ApplySortOrder;
 $CustomFields->LimitToLookupType( "RT::$Class"->CustomFieldLookupType );
 $CustomFields->LimitToObjectId(0);
 
-foreach my $name (keys %queues) {
-    my $queue = RT::Queue->new($session{'CurrentUser'});
-    $queue->Load($name);
-    next unless $queue->Id;
-    $CustomFields->LimitToObjectId($queue->Id);
-    $CustomFields->SetContextObject( $queue ) if keys %queues == 1;
+if ($LimitedQueues) {
+    foreach my $queue (values %queues) {
+        $CustomFields->LimitToObjectId($queue->Id);
+        $CustomFields->SetContextObject( $queue ) if keys %queues == 1;
+    }
 }
 
 my $has_cf = $CustomFields->First ? 1 : 0;
diff --git a/share/html/Search/Elements/PickTicketCFs b/share/html/Search/Elements/PickTicketCFs
index ae3a4a2..23e9dff 100644
--- a/share/html/Search/Elements/PickTicketCFs
+++ b/share/html/Search/Elements/PickTicketCFs
@@ -47,16 +47,15 @@
 %# END BPS TAGGED BLOCK }}}
 <%ARGS>
 %queues => ()
+$LimitedQueues => 0
 </%ARGS>
 <%init>
 my $CustomFields = RT::CustomFields->new( $session{'CurrentUser'});
-foreach my $id (keys %queues) {
-    # Gotta load up the $queue object, since queues get stored by name now.
-    my $queue = RT::Queue->new($session{'CurrentUser'});
-    $queue->Load($id);
-    next unless $queue->Id;
-    $CustomFields->LimitToQueue($queue->Id);
-    $CustomFields->SetContextObject( $queue ) if keys %queues == 1;
+if ($LimitedQueues) {
+    foreach my $queue (values %queues) {
+        $CustomFields->LimitToQueue($queue->Id);
+        $CustomFields->SetContextObject( $queue ) if keys %queues == 1;
+    }
 }
 $CustomFields->LimitToGlobal;
 $CustomFields->OrderBy( FIELD => 'Name', ORDER => 'ASC' );
diff --git a/share/html/Ticket/Elements/SelectStatus b/share/html/Ticket/Elements/SelectStatus
index 2258caf..8a35a52 100644
--- a/share/html/Ticket/Elements/SelectStatus
+++ b/share/html/Ticket/Elements/SelectStatus
@@ -53,12 +53,7 @@
    Type => 'ticket',
 &>
 <%INIT>
-my @Lifecycles;
-for my $id (keys %Queues) {
-    my $queue = RT::Queue->new($session{'CurrentUser'});
-    $queue->Load($id);
-    push @Lifecycles, $queue->LifecycleObj if $queue->id;
-}
+my @Lifecycles = map {$_->LifecycleObj} values %Queues;
 
 if ($TicketObj) {
     $ARGS{DefaultLabel} = loc("[_1] (Unchanged)", loc($TicketObj->Status));
diff --git a/t/web/query_builder_queue_limits.t b/t/web/query_builder_queue_limits.t
index 6bbf333..4a9ce56 100644
--- a/t/web/query_builder_queue_limits.t
+++ b/t/web/query_builder_queue_limits.t
@@ -111,7 +111,7 @@ is_deeply(
     \@owners, [ '', qw/Nobody root user_b/], 'no user_a'
 );
 
-diag "limit queue to general too";
+diag "limit queue to general AND foo (impossible)";
 
 $m->submit_form(
     fields => { ValueOfQueue => 'General' },
@@ -119,8 +119,8 @@ $m->submit_form(
 );
 
 $form = $m->form_name('BuildQuery');
-ok( $form->find_input("ValueOfCF.{general_cf}"), 'found general_cf' );
-ok( $form->find_input("ValueOfCF.{foo_cf}"), 'found foo_cf' );
+ok( !$form->find_input("ValueOfCF.{general_cf}"), 'no general_cf' );
+ok( !$form->find_input("ValueOfCF.{foo_cf}"), 'no foo_cf' );
 ok( $form->find_input("ValueOfCF.{global_cf}"), 'found global_cf' );
 $status_input = $form->find_input('ValueOfStatus');
 @statuses     = sort $status_input->possible_values;
@@ -135,7 +135,7 @@ is_deeply(
     \@owners, [ '', qw/Nobody root user_a user_b/], 'found all users again'
 );
 
-diag "limit queue to != foo";
+diag "limit queue to != foo (same as = general)";
 $m->get_ok( $url . '/Search/Build.html?NewQuery=1' );
 $m->submit_form(
     form_name => 'BuildQuery',
@@ -146,17 +146,17 @@ $m->submit_form(
 $form = $m->form_name('BuildQuery');
 ok( $form->find_input("ValueOfCF.{global_cf}"), 'found global_cf' );
 ok( !$form->find_input("ValueOfCF.{foo_cf}"), 'no foo_cf' );
-ok( !$form->find_input("ValueOfCF.{general_cf}"), 'no general_cf' );
+ok( $form->find_input("ValueOfCF.{general_cf}"), 'found general_cf' );
 $status_input = $form->find_input('ValueOfStatus');
 @statuses     = sort $status_input->possible_values;
 is_deeply(
-    \@statuses, [ '', qw/initial new open open rejected resolved resolved stalled/],
-    'found all statuses'
+    \@statuses, [ '', qw/new open rejected resolved stalled/],
+    'found only general statuses'
 ) or diag "Statuses are: ", explain \@statuses;
 $owner_input = $form->find_input('ValueOfActor');
 @owners     = sort $owner_input->possible_values;
 is_deeply(
-    \@owners, [ '', qw/Nobody root user_a user_b/], 'found all users'
+    \@owners, [ '', qw/Nobody root user_a/], 'users from general'
 );
 
 diag "limit queue to General OR foo";

commit 59b693f58cf1c8b1dc351173e174a9784c99db83
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 18:17:46 2015 -0500

    Also use QueueCF limits when examining GetReferencedQueues

diff --git a/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
index 3189d85..f38e2f5 100644
--- a/lib/RT/Interface/Web/QueryBuilder/Tree.pm
+++ b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
@@ -113,9 +113,32 @@ sub GetReferencedQueues {
             my $node = shift;
             my $clause = $node->getNodeValue();
             return unless $node->isLeaf and ref $clause;
-            return if $clause->{Key} eq 'Queue';
+            if ($clause->{Key} eq "Queue") {
+                return;
+            } elsif ($clause->{Key} =~ /^QueueCF\.(.*)$/) {
+                # Check we can find the CF; if we can't, we fall through
+                # to the below and trim out the node.
+                my $subkey = $1;
+                $subkey =~ s/^\{(.*?)\}$/$1/;
+
+                my $cf;
+                if ( $subkey =~ /\D/ ) {
+                    my $cfs = RT::CustomFields->new( $args{CurrentUser} );
+                    $cfs->Limit( FIELD => 'Name', VALUE => $subkey, CASESENSITIVE => 0 );
+                    $cfs->LimitToLookupType(RT::Queue->CustomFieldLookupType);
+                    $cf = $cfs->First;
+                } else {
+                    $cf = RT::CustomField->new( $args{CurrentUser} );
+                    $cf->Load($subkey);
+                }
+                if ($cf and $cf->id) {
+                    $clause->{CF} = $cf;
+                    return;
+                }
+            }
 
-            # This is a leaf node not dealing with queues; remove it
+            # This is a leaf node not dealing with queues, or a Queue CF
+            # we couldn't find; remove it
             if ($node->isRoot) {
                 $node->setNodeValue(0);
             } else {
@@ -145,16 +168,28 @@ sub GetReferencedQueues {
             if ($node->isLeaf) {
                 my $clause = $node->getNodeValue();
                 return unless $clause;
-                $queues->Limit(
-                    FIELD    => ($clause->{RawValue} =~ /\D/ ? "Name" : "id"),
-                    CASESENSITIVE => ($clause->{RawValue} =~ /\D/ ? 0 : 1),
-                    OPERATOR => $clause->{Op},
-                    VALUE    => $clause->{RawValue},
-                    ENTRYAGGREGATOR => (
-                        $node->isRoot ? "OR" :
-                        $node->getParent->getNodeValue),
-                    SUBCLAUSE => "referenced",
-                );
+                if ($clause->{Key} eq "Queue") {
+                    $queues->Limit(
+                        FIELD    => ($clause->{RawValue} =~ /\D/ ? "Name" : "id"),
+                        CASESENSITIVE => ($clause->{RawValue} =~ /\D/ ? 0 : 1),
+                        OPERATOR => $clause->{Op},
+                        VALUE    => $clause->{RawValue},
+                        ENTRYAGGREGATOR => (
+                            $node->isRoot ? "OR" :
+                            $node->getParent->getNodeValue),
+                        SUBCLAUSE => "referenced",
+                    );
+                } else {
+                    $queues->LimitCustomField(
+                        CUSTOMFIELD => $clause->{CF},
+                        OPERATOR    => $clause->{Op},
+                        VALUE       => $clause->{RawValue},
+                        ENTRYAGGREGATOR => (
+                            $node->isRoot ? "OR" :
+                            $node->getParent->getNodeValue),
+                        SUBCLAUSE => "referenced",
+                    );
+                }
                 $limits++;
             } else {
                 $queues->_OpenParen("referenced");

commit 7fae5c745e7d8c7ea7c90c35bb108c4a754ef9f4
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 19:36:22 2015 -0500

    Calling DESTROY is unnecessary due to use_weak_refs

diff --git a/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
index f38e2f5..3e71c86 100644
--- a/lib/RT/Interface/Web/QueryBuilder/Tree.pm
+++ b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
@@ -143,7 +143,6 @@ sub GetReferencedQueues {
                 $node->setNodeValue(0);
             } else {
                 $node->getParent->removeChild($node);
-                $node->DESTROY;
             }
         },
         sub {
@@ -156,7 +155,6 @@ sub GetReferencedQueues {
                 $node->setNodeValue(0);
             } else {
                 $node->getParent->removeChild($node);
-                $node->DESTROY;
             }
         }
     );
@@ -261,7 +259,6 @@ sub PruneChildlessAggregators {
 
             # OK, this is a childless aggregator.  Remove self.
             $node->getParent->removeChild($node);
-            $node->DESTROY;
         }
     );
 }

commit e3c303db8a3fdedd3c698a69ff3755a7facd2085
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 19:48:50 2015 -0500

    Simply override ->traverse, rather than write a separate method

diff --git a/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
index 3e71c86..a65670b 100644
--- a/lib/RT/Interface/Web/QueryBuilder/Tree.pm
+++ b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
@@ -65,28 +65,18 @@ It is a subclass of L<Tree::Simple>.
 
 =head1 METHODS
 
-=head2 TraversePrePost PREFUNC POSTFUNC
+=head2 traverse PREFUNC POSTFUNC
 
-Traverses the tree depth-first.  Before processing the node's children,
-calls PREFUNC with the node as its argument; after processing all of the
-children, calls POSTFUNC with the node as its argument.
-
-(Note that unlike Tree::Simple's C<traverse>, it actually calls its functions
-on the root node passed to it.)
+Override's L<Tree::Simple/traverse>, to call its functions on the root
+node passed to it.
 
 =cut
 
-sub TraversePrePost {
+sub traverse {
    my ($self, $prefunc, $postfunc) = @_;
 
-   # XXX: if pre or post action changes siblings (delete or adds)
-   # we could have problems
    $prefunc->($self) if $prefunc;
-
-   foreach my $child ($self->getAllChildren()) { 
-           $child->TraversePrePost($prefunc, $postfunc);
-   }
-   
+   $_->traverse( $prefunc, $postfunc ) for $self->getAllChildren();
    $postfunc->($self) if $postfunc;
 }
 
@@ -108,7 +98,7 @@ sub GetReferencedQueues {
     );
 
     my $q_refs = $self->clone;
-    $q_refs->TraversePrePost(
+    $q_refs->traverse(
         sub {
             my $node = shift;
             my $clause = $node->getNodeValue();
@@ -160,7 +150,7 @@ sub GetReferencedQueues {
     );
     my $limits = 0;
     my $queues = RT::Queues->new( $args{CurrentUser} );
-    $q_refs->TraversePrePost(
+    $q_refs->traverse(
         sub {
             my $node = shift;
             if ($node->isLeaf) {
@@ -246,7 +236,7 @@ or parenthesizations with no children, get rid of them.
 sub PruneChildlessAggregators {
     my $self = shift;
 
-    $self->TraversePrePost(
+    $self->traverse(
         undef,
         sub {
             my $node = shift;
@@ -281,7 +271,7 @@ sub __LinearizeTree {
 
     my ($list, $i) = ([], 0);
 
-    $self->TraversePrePost( sub {
+    $self->traverse( sub {
         my $node = shift;
         return if $node->isRoot;
 

commit 2b5cc0b7207ed89d9e584c985378966629f696f8
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 20:04:52 2015 -0500

    Move RT::Interface::QueryBuilder::Tree to under RT::Tickets
    
    This removes an empty package in the heirarchy and puts it under where
    it is used.

diff --git a/lib/RT/Interface/Web/QueryBuilder.pm b/lib/RT/Interface/Web/QueryBuilder.pm
deleted file mode 100644
index b551424..0000000
--- a/lib/RT/Interface/Web/QueryBuilder.pm
+++ /dev/null
@@ -1,56 +0,0 @@
-# BEGIN BPS TAGGED BLOCK {{{
-#
-# COPYRIGHT:
-#
-# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
-#                                          <sales at bestpractical.com>
-#
-# (Except where explicitly superseded by other copyright notices)
-#
-#
-# LICENSE:
-#
-# This work is made available to you under the terms of Version 2 of
-# the GNU General Public License. A copy of that license should have
-# been provided with this software, but in any event can be snarfed
-# from www.gnu.org.
-#
-# This work is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
-# 02110-1301 or visit their web page on the internet at
-# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
-#
-#
-# CONTRIBUTION SUBMISSION POLICY:
-#
-# (The following paragraph is not intended to limit the rights granted
-# to you to modify and distribute this software under the terms of
-# the GNU General Public License and is only of importance to you if
-# you choose to contribute your changes and enhancements to the
-# community by submitting them to Best Practical Solutions, LLC.)
-#
-# By intentionally submitting any modifications, corrections or
-# derivatives to this work, or any other work intended for use with
-# Request Tracker, to Best Practical Solutions, LLC, you confirm that
-# you are the copyright holder for those contributions and you grant
-# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
-# royalty-free, perpetual, license to use, copy, create derivative
-# works based on those contributions, and sublicense and distribute
-# those contributions and any derivatives thereof.
-#
-# END BPS TAGGED BLOCK }}}
-
-package RT::Interface::Web::QueryBuilder;
-
-use strict;
-use warnings;
-
-RT::Base->_ImportOverlays();
-
-1;
diff --git a/lib/RT/Report/Tickets.pm b/lib/RT/Report/Tickets.pm
index 2120017..9ca5ef1 100644
--- a/lib/RT/Report/Tickets.pm
+++ b/lib/RT/Report/Tickets.pm
@@ -154,8 +154,7 @@ our %GROUPINGS_META = (
 
 
             unless ( $args->{Queues} ) {
-                require RT::Interface::Web::QueryBuilder::Tree;
-                my $tree = RT::Interface::Web::QueryBuilder::Tree->new('AND');
+                my $tree = RT::Tickets::Tree->new('AND');
                 $tree->ParseSQL( Query => $args->{'Query'}, CurrentUser => $self->CurrentUser );
                 @$args{qw/Queues LimitedQueues/} = $tree->GetReferencedQueues( CurrentUser => $self->CurrentUser );
             }
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index c641cd2..14bce0e 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -79,6 +79,7 @@ use Scalar::Util qw/blessed/;
 
 use RT::Ticket;
 use RT::SQL;
+use RT::Tickets::Tree;
 
 sub Table { 'Tickets'}
 
diff --git a/lib/RT/Interface/Web/QueryBuilder/Tree.pm b/lib/RT/Tickets/Tree.pm
similarity index 98%
rename from lib/RT/Interface/Web/QueryBuilder/Tree.pm
rename to lib/RT/Tickets/Tree.pm
index a65670b..1d5453f 100644
--- a/lib/RT/Interface/Web/QueryBuilder/Tree.pm
+++ b/lib/RT/Tickets/Tree.pm
@@ -46,7 +46,7 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-package RT::Interface::Web::QueryBuilder::Tree;
+package RT::Tickets::Tree;
 
 use strict;
 use warnings;
@@ -56,7 +56,7 @@ use base qw/Tree::Simple/;
 
 =head1 NAME
 
-  RT::Interface::Web::QueryBuilder::Tree - subclass of Tree::Simple used in Query Builder
+RT::Tickets::Tree - subclass of Tree::Simple used in Query Builder
 
 =head1 DESCRIPTION
 
diff --git a/share/html/Search/Build.html b/share/html/Search/Build.html
index df4c476..587052c 100644
--- a/share/html/Search/Build.html
+++ b/share/html/Search/Build.html
@@ -54,7 +54,7 @@
 %#
 %#   After doing some stuff with default arguments and saved searches, the ParseQuery
 %#   function (which is similar to, but not the same as, _parser in lib/RT/Tickets.pm)
-%#   converts the Query into a RT::Interface::Web::QueryBuilder::Tree.  This mason file
+%#   converts the Query into a RT::Tickets::Tree.  This mason file
 %#   then adds stuff to or modifies the tree based on the actions that had been requested
 %#   by clicking buttons.  It then calls GetQueryAndOptionList on the tree to generate
 %#   the SQL query (which is saved as a hidden input) and the option list for the Clauses
@@ -107,9 +107,6 @@
 </form>
 
 <%INIT>
-use RT::Interface::Web::QueryBuilder;
-use RT::Interface::Web::QueryBuilder::Tree;
-
 $ARGS{SavedChartSearchId} ||= 'new';
 
 my $title = loc("Query Builder");
@@ -166,7 +163,7 @@ if ( $NewQuery ) {
 my $ParseQuery = sub {
     my ($string, $results) = @_;
 
-    my $tree = RT::Interface::Web::QueryBuilder::Tree->new('AND');
+    my $tree = RT::Tickets::Tree->new('AND');
     @$results = $tree->ParseSQL( Query => $string, CurrentUser => $session{'CurrentUser'} );
 
     return $tree;
@@ -256,7 +253,7 @@ foreach my $arg ( keys %ARGS ) {
             RawValue => $rawvalue,
         };
 
-        push @new_values, RT::Interface::Web::QueryBuilder::Tree->new($clause);
+        push @new_values, RT::Tickets::Tree->new($clause);
     }
 }
 
diff --git a/share/html/Search/Elements/EditQuery b/share/html/Search/Elements/EditQuery
index 7bdded1..ef41355 100644
--- a/share/html/Search/Elements/EditQuery
+++ b/share/html/Search/Elements/EditQuery
@@ -140,7 +140,7 @@ elsif ( $ARGS{"Right"} ) {
                 my $sibling = $parent->getChild( $index - 1 );
                 $newparent = $sibling unless $sibling->isLeaf;
             }
-            $newparent ||= RT::Interface::Web::QueryBuilder::Tree->new( $ARGS{'AndOr'} || 'AND', $parent );
+            $newparent ||= RT::Tickets::Tree->new( $ARGS{'AndOr'} || 'AND', $parent );
 
             $parent->removeChild($value);
             $newparent->addChild($value);

commit 1036d48d005f137a00c134e82c10dd0cb02e3ae4
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 20:10:05 2015 -0500

    Remove the unused ParseToArray
    
    Its only callsite was removed in 8e1f6b3d

diff --git a/lib/RT/SQL.pm b/lib/RT/SQL.pm
index 9f8dad8..88aff93 100644
--- a/lib/RT/SQL.pm
+++ b/lib/RT/SQL.pm
@@ -70,22 +70,6 @@ my $re_op          = qr[=|!=|>=|<=|>|<|(?i:IS NOT)|(?i:IS)|(?i:NOT LIKE)|(?i:LIK
 my $re_open_paren  = qr[\(];
 my $re_close_paren = qr[\)];
 
-sub ParseToArray {
-    my ($string) = shift;
-
-    my ($tree, $node, @pnodes);
-    $node = $tree = [];
-
-    my %callback;
-    $callback{'OpenParen'} = sub { push @pnodes, $node; $node = []; push @{ $pnodes[-1] }, $node };
-    $callback{'CloseParen'} = sub { $node = pop @pnodes };
-    $callback{'EntryAggregator'} = sub { push @$node, $_[0] };
-    $callback{'Condition'} = sub { push @$node, { key => $_[0], op => $_[1], value => $_[2] } };
-
-    Parse($string, \%callback);
-    return $tree;
-}
-
 sub Parse {
     my ($string, $cb) = @_;
     my $loc = sub {HTML::Mason::Commands::loc(@_)};

commit b4cf8a0af4cc40d579282f0f75a5765d28c5bec7
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 20:44:50 2015 -0500

    Temporarily remove bundling code; it will be replaced by an optimizer

diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index 914c74b..b8c868d 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -262,11 +262,7 @@ sub RoleLimit {
     $args{FIELD} ||= 'EmailAddress';
 
     my ($groups, $group_members, $users);
-    if ( $args{'BUNDLE'} ) {
-        ($groups, $group_members, $users) = @{ $args{'BUNDLE'} };
-    } else {
-        $groups = $self->_RoleGroupsJoin( Name => $type, Class => $class, New => !$type );
-    }
+    $groups = $self->_RoleGroupsJoin( Name => $type, Class => $class, New => !$type );
 
     $self->_OpenParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
     if ( $args{OPERATOR} =~ /^IS(?: NOT)?$/i ) {
@@ -393,7 +389,6 @@ sub RoleLimit {
         }
     }
     $self->_CloseParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
-    return ($groups, $group_members, $users);
 }
 
 1;
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 14bce0e..4f997c3 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2931,64 +2931,19 @@ sub _parser {
     my ($self,$string) = @_;
     my $ea = '';
 
-    # Bundling of joins is implemented by dynamically tracking a parallel query
-    # tree in %sub_tree as the TicketSQL is parsed.
-    #
-    # Only positive, OR'd watcher conditions are bundled currently.  Each key
-    # in %sub_tree is a watcher type (Requestor, Cc, AdminCc) or the generic
-    # "Watcher" for any watcher type.  Owner is not bundled because it is
-    # denormalized into a Tickets column and doesn't need a join.  AND'd
-    # conditions are not bundled since a record may have multiple watchers
-    # which independently match the conditions, thus necessitating two joins.
-    #
-    # The values of %sub_tree are arrayrefs made up of:
-    #
-    #   * Open parentheses "(" pushed on by the OpenParen callback
-    #   * Arrayrefs of bundled join aliases pushed on by the Condition callback
-    #   * Entry aggregators (AND/OR) pushed on by the EntryAggregator callback
-    #
-    # The CloseParen callback takes care of backing off the query trees until
-    # outside of the just-closed parenthetical, thus restoring the tree state
-    # an equivalent of before the parenthetical was entered.
-    #
-    # The Condition callback handles starting a new subtree or extending an
-    # existing one, determining if bundling the current condition with any
-    # subtree is possible, and pruning any dangling entry aggregators from
-    # trees.
-    #
-
-    my %sub_tree;
-    my $depth = 0;
-
     my %callback;
     $callback{'OpenParen'} = sub {
       $self->_OpenParen;
-      $depth++;
-      push @$_, '(' foreach values %sub_tree;
     };
     $callback{'CloseParen'} = sub {
       $self->_CloseParen;
-      $depth--;
-      foreach my $list ( values %sub_tree ) {
-          if ( $list->[-1] eq '(' ) {
-              pop @$list;
-              pop @$list if $list->[-1] =~ /^(?:AND|OR)$/i;
-          }
-          else {
-              pop @$list while $list->[-2] ne '(';
-              $list->[-1] = pop @$list;
-          }
-      }
     };
     $callback{'EntryAggregator'} = sub {
       $ea = $_[0] || '';
-      push @$_, $ea foreach grep @$_ && $_->[-1] ne '(', values %sub_tree;
     };
     $callback{'Condition'} = sub {
         my ($key, $op, $value) = @_;
 
-        my $negative_op = ($op eq '!=' || $op =~ /\bNOT\b/i);
-        my $null_op = ( 'is not' eq lc($op) || 'is' eq lc($op) );
         # key has dot then it's compound variant and we have subkey
         my $subkey = '';
         ($key, $subkey) = ($1, $2) if $key =~ /^([^\.]+)\.(.+)$/;
@@ -3010,28 +2965,12 @@ sub _parser {
         }
         my $sub = $dispatch{ $class };
 
-        my @res; my $bundle_with;
-        if ( $class eq 'WATCHERFIELD' && $key ne 'Owner' && !$negative_op && (!$null_op || $subkey) ) {
-            if ( !$sub_tree{$key} ) {
-              $sub_tree{$key} = [ ('(')x$depth, \@res ];
-            } else {
-              $bundle_with = $self->_check_bundling_possibility( $string, @{ $sub_tree{$key} } );
-              if ( $sub_tree{$key}[-1] eq '(' ) {
-                    push @{ $sub_tree{$key} }, \@res;
-              }
-            }
-        }
-
-        # Remove our aggregator from subtrees where our condition didn't get added
-        pop @$_ foreach grep @$_ && $_->[-1] =~ /^(?:AND|OR)$/i, values %sub_tree;
-
         # A reference to @res may be pushed onto $sub_tree{$key} from
         # above, and we fill it here.
-        @res = $sub->( $self, $key, $op, $value,
+        $sub->( $self, $key, $op, $value,
                 SUBCLAUSE       => '',  # don't need anymore
                 ENTRYAGGREGATOR => $ea,
                 SUBKEY          => $subkey,
-                BUNDLE          => $bundle_with,
               );
         $ea = '';
     };
@@ -3103,29 +3042,6 @@ sub Query {
     return $self->{_sql_query};
 }
 
-sub _check_bundling_possibility {
-    my $self = shift;
-    my $string = shift;
-    my @list = reverse @_;
-    while (my $e = shift @list) {
-        next if $e eq '(';
-        if ( lc($e) eq 'and' ) {
-            return undef;
-        }
-        elsif ( lc($e) eq 'or' ) {
-            return shift @list;
-        }
-        else {
-            # should not happen
-            $RT::Logger->error(
-                "Joins optimization failed when parsing '$string'. It's bug in RT, contact Best Practical"
-            );
-            die "Internal error. Contact your system administrator.";
-        }
-    }
-    return undef;
-}
-
 RT::Base->_ImportOverlays();
 
 1;

commit 1220d6faacadb91f109cd9add4649d3ad514c7d5
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 21:37:21 2015 -0500

    Switch to parsing into a parse tree as an IR

diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 4f997c3..dd6d6ac 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2929,52 +2929,44 @@ failure.
 
 sub _parser {
     my ($self,$string) = @_;
+
+    my $tree = RT::Tickets::Tree->new;
+    $tree->ParseSQL(
+        Query => $string,
+        CurrentUser => $self->CurrentUser,
+    );
+
     my $ea = '';
+    $tree->traverse(
+        sub {
+            my $node = shift;
+            $ea = $node->getParent->getNodeValue if $node->getIndex > 0;
+            return $self->_OpenParen unless $node->isLeaf;
 
-    my %callback;
-    $callback{'OpenParen'} = sub {
-      $self->_OpenParen;
-    };
-    $callback{'CloseParen'} = sub {
-      $self->_CloseParen;
-    };
-    $callback{'EntryAggregator'} = sub {
-      $ea = $_[0] || '';
-    };
-    $callback{'Condition'} = sub {
-        my ($key, $op, $value) = @_;
-
-        # key has dot then it's compound variant and we have subkey
-        my $subkey = '';
-        ($key, $subkey) = ($1, $2) if $key =~ /^([^\.]+)\.(.+)$/;
-
-        # normalize key and get class (type)
-        my $class;
-        if (exists $LOWER_CASE_FIELDS{lc $key}) {
-            $key = $LOWER_CASE_FIELDS{lc $key};
-            $class = $FIELD_METADATA{$key}->[0];
-        }
-        die "Unknown field '$key' in '$string'" unless $class;
+            my ($key, $subkey, $meta, $op, $value)
+                = @{$node->getNodeValue}{qw/Key Subkey Meta Op Value/};
 
-        # replace __CurrentUser__ with id
-        $value = $self->CurrentUser->id if $value eq '__CurrentUser__';
+            # normalize key and get class (type)
+            my $class = $meta->[0];
 
+            # replace __CurrentUser__ with id
+            $value = $self->CurrentUser->id if $value eq '__CurrentUser__';
 
-        unless( $dispatch{ $class } ) {
-            die "No dispatch method for class '$class'"
-        }
-        my $sub = $dispatch{ $class };
+            my $sub = $dispatch{ $class }
+                or die "No dispatch method for class '$class'";
 
-        # A reference to @res may be pushed onto $sub_tree{$key} from
-        # above, and we fill it here.
-        $sub->( $self, $key, $op, $value,
-                SUBCLAUSE       => '',  # don't need anymore
-                ENTRYAGGREGATOR => $ea,
-                SUBKEY          => $subkey,
-              );
-        $ea = '';
-    };
-    RT::SQL::Parse($string, \%callback);
+            # A reference to @res may be pushed onto $sub_tree{$key} from
+            # above, and we fill it here.
+            $sub->( $self, $key, $op, $value,
+                    ENTRYAGGREGATOR => $ea,
+                    SUBKEY          => $subkey,
+                  );
+        },
+        sub {
+            my $node = shift;
+            return $self->_CloseParen unless $node->isLeaf;
+        }
+    );
 }
 
 sub FromSQL {
diff --git a/lib/RT/Tickets/Tree.pm b/lib/RT/Tickets/Tree.pm
index 1d5453f..c869b67 100644
--- a/lib/RT/Tickets/Tree.pm
+++ b/lib/RT/Tickets/Tree.pm
@@ -158,10 +158,10 @@ sub GetReferencedQueues {
                 return unless $clause;
                 if ($clause->{Key} eq "Queue") {
                     $queues->Limit(
-                        FIELD    => ($clause->{RawValue} =~ /\D/ ? "Name" : "id"),
-                        CASESENSITIVE => ($clause->{RawValue} =~ /\D/ ? 0 : 1),
+                        FIELD    => ($clause->{Value} =~ /\D/ ? "Name" : "id"),
+                        CASESENSITIVE => ($clause->{Value} =~ /\D/ ? 0 : 1),
                         OPERATOR => $clause->{Op},
-                        VALUE    => $clause->{RawValue},
+                        VALUE    => $clause->{Value},
                         ENTRYAGGREGATOR => (
                             $node->isRoot ? "OR" :
                             $node->getParent->getNodeValue),
@@ -171,7 +171,7 @@ sub GetReferencedQueues {
                     $queues->LimitCustomField(
                         CUSTOMFIELD => $clause->{CF},
                         OPERATOR    => $clause->{Op},
-                        VALUE       => $clause->{RawValue},
+                        VALUE       => $clause->{Value},
                         ENTRYAGGREGATOR => (
                             $node->isRoot ? "OR" :
                             $node->getParent->getNodeValue),
@@ -285,10 +285,20 @@ sub __LinearizeTree {
         } else {
 
             my $clause = $node->getNodeValue;
-            $str .= $clause->{Key};
-            $str .= " ". $clause->{Op};
-            $str .= " ". $clause->{Value};
+            my $key = $clause->{Key};
+            $key .= "." . $clause->{Subkey} if defined $clause->{Subkey};
+            if ($key =~ s/(['\\])/\\$1/g or $key =~ /[^{}\w\.]/) {
+                $key = "'$key'";
+            }
+            my $value = $clause->{Value};
+            if ( $clause->{Op} =~ /^IS( NOT)?$/i ) {
+                $value = 'NULL';
+            } elsif ( $value !~ /^[+-]?[0-9]+$/ ) {
+                $value =~ s/(['\\])/\\$1/g;
+                $value = "'$value'";
+            }
 
+            $str .= $key ." ". $clause->{Op} . " " . $value;
         }
         $str =~ s/^\s+|\s+$//;
 
@@ -333,32 +343,20 @@ sub ParseSQL {
     $callback{'EntryAggregator'} = sub { $node->setNodeValue( $_[0] ) };
     $callback{'Condition'} = sub {
         my ($key, $op, $value) = @_;
-        my $rawvalue = $value;
 
-        my ($main_key) = split /[.]/, $key;
+        my ($main_key, $subkey) = split /[.]/, $key, 2;
 
-        my $class;
-        if ( exists $lcfield{ lc $main_key } ) {
-            $key =~ s/^[^.]+/ $lcfield{ lc $main_key } /e;
-            ($main_key) = split /[.]/, $key;  # make the case right
-            $class = $field{ $main_key }->[0];
-        }
-        unless( $class ) {
+        unless( $lcfield{ lc $main_key} ) {
             push @results, [ $args{'CurrentUser'}->loc("Unknown field: [_1]", $key), -1 ]
         }
+        $main_key = $lcfield{ lc $main_key };
 
-        if ( lc $op eq 'is' || lc $op eq 'is not' ) {
-            $value = 'NULL'; # just fix possible mistakes here
-        } elsif ( $value !~ /^[+-]?[0-9]+$/ ) {
-            $value =~ s/(['\\])/\\$1/g;
-            $value = "'$value'";
-        }
-
-        if ($key =~ s/(['\\])/\\$1/g or $key =~ /[^{}\w\.]/) {
-            $key = "'$key'";
-        }
+        # Hardcode value for IS / IS NOT
+        $value = 'NULL' if $op =~ /^IS( NOT)?$/i;
 
-        my $clause = { Key => $key, Op => $op, Value => $value, RawValue => $rawvalue };
+        my $clause = { Key => $main_key, Subkey => $subkey,
+                       Meta => $field{ $main_key },
+                       Op => $op, Value => $value };
         $node->addChild( __PACKAGE__->new( $clause ) );
     };
     $callback{'Error'} = sub { push @results, @_ };
diff --git a/share/html/Search/Build.html b/share/html/Search/Build.html
index 587052c..8c742f1 100644
--- a/share/html/Search/Build.html
+++ b/share/html/Search/Build.html
@@ -227,30 +227,11 @@ foreach my $arg ( keys %ARGS ) {
     for ( my $i = 0; $i < @ops; $i++ ) {
         my ( $op, $value ) = ( $ops[$i], $values[$i] );
         next if !defined $value || $value eq '';
-        my $rawvalue = $value;
-
-        if ( $value =~ /^NULL$/i && $op =~ /=/ ) {
-            if ( $op eq '=' ) {
-                $op = "IS";
-            }
-            elsif ( $op eq '!=' ) {
-                $op = "IS NOT";
-            }
-        }
-        elsif ($value =~ /\D/) {
-            $value =~ s/(['\\])/\\$1/g;
-            $value = "'$value'";
-        }
-
-        if ($keyword =~ s/(['\\])/\\$1/g or $keyword =~ /[^{}\w\.]/) {
-            $keyword = "'$keyword'";
-        }
 
         my $clause = {
             Key   => $keyword,
             Op    => $op,
             Value => $value,
-            RawValue => $rawvalue,
         };
 
         push @new_values, RT::Tickets::Tree->new($clause);

commit ebd18013bd99a7652b4f5dfe3dba589a107d34ea
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jan 21 22:30:44 2015 -0500

    Bundling as an optimization pass

diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index b8c868d..134a507 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -262,7 +262,11 @@ sub RoleLimit {
     $args{FIELD} ||= 'EmailAddress';
 
     my ($groups, $group_members, $users);
-    $groups = $self->_RoleGroupsJoin( Name => $type, Class => $class, New => !$type );
+    if ( $args{'BUNDLE'} and @{$args{'BUNDLE'}}) {
+        ($groups, $group_members, $users) = @{ $args{'BUNDLE'} };
+    } else {
+        $groups = $self->_RoleGroupsJoin( Name => $type, Class => $class, New => !$type );
+    }
 
     $self->_OpenParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
     if ( $args{OPERATOR} =~ /^IS(?: NOT)?$/i ) {
@@ -389,6 +393,9 @@ sub RoleLimit {
         }
     }
     $self->_CloseParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
+    if ($args{BUNDLE} and not @{$args{BUNDLE}}) {
+        @{$args{BUNDLE}} = ($groups, $group_members, $users);
+    }
 }
 
 1;
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index dd6d6ac..9b8f32a 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2936,6 +2936,27 @@ sub _parser {
         CurrentUser => $self->CurrentUser,
     );
 
+    # Perform an optimization pass looking for watcher bundling
+    $tree->traverse(
+        sub {
+            my $node = shift;
+            return if $node->isLeaf;
+            return unless ($node->getNodeValue||'') eq "OR";
+            my %refs;
+            my @kids = grep {$_->{Meta}[0] eq "WATCHERFIELD"}
+                map {$_->getNodeValue}
+                grep {$_->isLeaf} $node->getAllChildren;
+            for (@kids) {
+                my $node = $_;
+                my ($key, $subkey, $op) = @{$node}{qw/Key Subkey Op/};
+                next if $node->{Meta}[1] and RT::Ticket->Role($node->{Meta}[1])->{Column};
+                next if $op =~ /^!=$|\bNOT\b/i;
+                next if $op =~ /^IS( NOT)?$/i and not $subkey;
+                $node->{Bundle} = $refs{$node->{Meta}[1] || ''} ||= [];
+            }
+        }
+    );
+
     my $ea = '';
     $tree->traverse(
         sub {
@@ -2943,8 +2964,8 @@ sub _parser {
             $ea = $node->getParent->getNodeValue if $node->getIndex > 0;
             return $self->_OpenParen unless $node->isLeaf;
 
-            my ($key, $subkey, $meta, $op, $value)
-                = @{$node->getNodeValue}{qw/Key Subkey Meta Op Value/};
+            my ($key, $subkey, $meta, $op, $value, $bundle)
+                = @{$node->getNodeValue}{qw/Key Subkey Meta Op Value Bundle/};
 
             # normalize key and get class (type)
             my $class = $meta->[0];
@@ -2960,6 +2981,7 @@ sub _parser {
             $sub->( $self, $key, $op, $value,
                     ENTRYAGGREGATOR => $ea,
                     SUBKEY          => $subkey,
+                    BUNDLE          => $bundle,
                   );
         },
         sub {

commit 1d22d315329668c55c67948bb626e1a01b3a1ede
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jan 22 01:41:16 2015 -0500

    Move normalization into ::Tree

diff --git a/lib/RT/Tickets/Tree.pm b/lib/RT/Tickets/Tree.pm
index c869b67..418e4c9 100644
--- a/lib/RT/Tickets/Tree.pm
+++ b/lib/RT/Tickets/Tree.pm
@@ -65,6 +65,22 @@ It is a subclass of L<Tree::Simple>.
 
 =head1 METHODS
 
+=head2 new
+
+=cut
+
+sub new {
+    my $class = shift;
+    my ($val, $parent) = @_;
+    if (ref $val) {
+        $val->{Op} = uc $val->{Op};
+        $val->{Op} = 'IS'      if uc $val->{Value} eq 'NULL' and $val->{Op} eq '=';
+        $val->{Op} = 'IS NOT'  if uc $val->{Value} eq 'NULL' and $val->{Op} eq '!=';
+        $val->{Value} = 'NULL' if $val->{Op} =~ /^IS( NOT)?$/i;
+    }
+    return $class->SUPER::new( $val, $parent );
+}
+
 =head2 traverse PREFUNC POSTFUNC
 
 Override's L<Tree::Simple/traverse>, to call its functions on the root

commit d707a03a518ddd110094faef9758c9e8b45b2d07
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 20 18:02:54 2015 +0800

    use old RT::Interface::Web::QueryBuilder::Tree namespace for RT 4.2

diff --git a/lib/RT/Interface/Web/QueryBuilder.pm b/lib/RT/Interface/Web/QueryBuilder.pm
new file mode 100644
index 0000000..b551424
--- /dev/null
+++ b/lib/RT/Interface/Web/QueryBuilder.pm
@@ -0,0 +1,56 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+package RT::Interface::Web::QueryBuilder;
+
+use strict;
+use warnings;
+
+RT::Base->_ImportOverlays();
+
+1;
diff --git a/lib/RT/Tickets/Tree.pm b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
similarity index 98%
rename from lib/RT/Tickets/Tree.pm
rename to lib/RT/Interface/Web/QueryBuilder/Tree.pm
index 418e4c9..4f3e01f 100644
--- a/lib/RT/Tickets/Tree.pm
+++ b/lib/RT/Interface/Web/QueryBuilder/Tree.pm
@@ -46,7 +46,7 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-package RT::Tickets::Tree;
+package RT::Interface::Web::QueryBuilder::Tree;
 
 use strict;
 use warnings;
@@ -56,7 +56,7 @@ use base qw/Tree::Simple/;
 
 =head1 NAME
 
-RT::Tickets::Tree - subclass of Tree::Simple used in Query Builder
+RT::Interface::Web::QueryBuilder::Tree - subclass of Tree::Simple used in Query Builder
 
 =head1 DESCRIPTION
 
diff --git a/lib/RT/Report/Tickets.pm b/lib/RT/Report/Tickets.pm
index 9ca5ef1..f230a04 100644
--- a/lib/RT/Report/Tickets.pm
+++ b/lib/RT/Report/Tickets.pm
@@ -154,7 +154,7 @@ our %GROUPINGS_META = (
 
 
             unless ( $args->{Queues} ) {
-                my $tree = RT::Tickets::Tree->new('AND');
+                my $tree = RT::Interface::Web::QueryBuilder::Tree->new('AND');
                 $tree->ParseSQL( Query => $args->{'Query'}, CurrentUser => $self->CurrentUser );
                 @$args{qw/Queues LimitedQueues/} = $tree->GetReferencedQueues( CurrentUser => $self->CurrentUser );
             }
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 9b8f32a..3dc8762 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -79,7 +79,7 @@ use Scalar::Util qw/blessed/;
 
 use RT::Ticket;
 use RT::SQL;
-use RT::Tickets::Tree;
+use RT::Interface::Web::QueryBuilder::Tree;
 
 sub Table { 'Tickets'}
 
@@ -2930,7 +2930,7 @@ failure.
 sub _parser {
     my ($self,$string) = @_;
 
-    my $tree = RT::Tickets::Tree->new;
+    my $tree = RT::Interface::Web::QueryBuilder::Tree->new;
     $tree->ParseSQL(
         Query => $string,
         CurrentUser => $self->CurrentUser,
diff --git a/share/html/Search/Build.html b/share/html/Search/Build.html
index 8c742f1..bddb6f7 100644
--- a/share/html/Search/Build.html
+++ b/share/html/Search/Build.html
@@ -54,7 +54,7 @@
 %#
 %#   After doing some stuff with default arguments and saved searches, the ParseQuery
 %#   function (which is similar to, but not the same as, _parser in lib/RT/Tickets.pm)
-%#   converts the Query into a RT::Tickets::Tree.  This mason file
+%#   converts the Query into a RT::Interface::Web::QueryBuilder::Tree.  This mason file
 %#   then adds stuff to or modifies the tree based on the actions that had been requested
 %#   by clicking buttons.  It then calls GetQueryAndOptionList on the tree to generate
 %#   the SQL query (which is saved as a hidden input) and the option list for the Clauses
@@ -163,7 +163,7 @@ if ( $NewQuery ) {
 my $ParseQuery = sub {
     my ($string, $results) = @_;
 
-    my $tree = RT::Tickets::Tree->new('AND');
+    my $tree = RT::Interface::Web::QueryBuilder::Tree->new('AND');
     @$results = $tree->ParseSQL( Query => $string, CurrentUser => $session{'CurrentUser'} );
 
     return $tree;
@@ -234,7 +234,7 @@ foreach my $arg ( keys %ARGS ) {
             Value => $value,
         };
 
-        push @new_values, RT::Tickets::Tree->new($clause);
+        push @new_values, RT::Interface::Web::QueryBuilder::Tree->new($clause);
     }
 }
 
diff --git a/share/html/Search/Elements/EditQuery b/share/html/Search/Elements/EditQuery
index ef41355..7bdded1 100644
--- a/share/html/Search/Elements/EditQuery
+++ b/share/html/Search/Elements/EditQuery
@@ -140,7 +140,7 @@ elsif ( $ARGS{"Right"} ) {
                 my $sibling = $parent->getChild( $index - 1 );
                 $newparent = $sibling unless $sibling->isLeaf;
             }
-            $newparent ||= RT::Tickets::Tree->new( $ARGS{'AndOr'} || 'AND', $parent );
+            $newparent ||= RT::Interface::Web::QueryBuilder::Tree->new( $ARGS{'AndOr'} || 'AND', $parent );
 
             $parent->removeChild($value);
             $newparent->addChild($value);

commit 39df7b8c1001705698a405f06e148d91b276b389
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 20 18:05:21 2015 +0800

    Revert "Remove the unused ParseToArray"
    
    This reverts commit 1036d48d005f137a00c134e82c10dd0cb02e3ae4.
    
    We can't remove it because RT::IR still uses it.

diff --git a/lib/RT/SQL.pm b/lib/RT/SQL.pm
index 88aff93..9f8dad8 100644
--- a/lib/RT/SQL.pm
+++ b/lib/RT/SQL.pm
@@ -70,6 +70,22 @@ my $re_op          = qr[=|!=|>=|<=|>|<|(?i:IS NOT)|(?i:IS)|(?i:NOT LIKE)|(?i:LIK
 my $re_open_paren  = qr[\(];
 my $re_close_paren = qr[\)];
 
+sub ParseToArray {
+    my ($string) = shift;
+
+    my ($tree, $node, @pnodes);
+    $node = $tree = [];
+
+    my %callback;
+    $callback{'OpenParen'} = sub { push @pnodes, $node; $node = []; push @{ $pnodes[-1] }, $node };
+    $callback{'CloseParen'} = sub { $node = pop @pnodes };
+    $callback{'EntryAggregator'} = sub { push @$node, $_[0] };
+    $callback{'Condition'} = sub { push @$node, { key => $_[0], op => $_[1], value => $_[2] } };
+
+    Parse($string, \%callback);
+    return $tree;
+}
+
 sub Parse {
     my ($string, $cb) = @_;
     my $loc = sub {HTML::Mason::Commands::loc(@_)};

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


More information about the rt-commit mailing list