Make sure TicketType and ObjectType limits in transaction searches ar…

…e AND-ed

To add TicketType='ticket' and ObjectType="RT::Ticket" preconditions,
previously we wrapped user's TxnSQL with only if it doesn't contain outer
parens, of which the regex check was not quite accurate. E.g.

    ( Type = 'Set' ) OR ( Type = 'Correspond' )

It matches the regex(qr/^\s*\(.*\)$/) but actually we still need to wrap it
so the final TxnSQL could be:

    TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND ( ( Type = 'Set' ) OR ( Type = 'Correspond' ) )

Otherwise all Set/Correspond transactions would be included in search
result.

To fix the issue above, this commit parses user's TxnSQL to wrap it as long
as the root condition is "OR".

lib/RT/Interface/Web.pm

@@ -5870,10 +5870,25 @@ sub PreprocessTransactionSearchQuery {
     my @limits;
     if ( $args{ObjectType} eq 'RT::Ticket' ) {
         if ( $args{Query} !~ /^TicketType = 'ticket' AND ObjectType = '$args{ObjectType}' AND (.+)/ ) {
+            require RT::Interface::Web::QueryBuilder::Tree;
+            my $tree = RT::Interface::Web::QueryBuilder::Tree->new;
+            my @results = $tree->ParseSQL(
+                Query       => $args{Query},
+                CurrentUser => $session{CurrentUser},
+                Class       => 'RT::Transactions',
+            );
+
+            # Errors will be handled in FromSQL later, so it's safe to simply return here
+            return $args{Query} if @results;
+
+            if ( lc( $tree->getNodeValue // '' ) eq 'or' ) {
+                $args{Query} = "( $args{Query} )";
+            }
+
             @limits = (
                 q{TicketType = 'ticket'},
                 qq{ObjectType = '$args{ObjectType}'},
-                $args{Query} =~ /^\s*\(.*\)$/ ? $args{Query} : "($args{Query})"
+                $args{Query},
             );
         }
         else {

t/transaction/search.t

@@ -136,4 +136,24 @@ is( $txns->Count, 1, 'Found the txn with id limit' );
 $txns->FromSQL("id > 10000");
 is( $txns->Count, 0, 'No txns with big ids yet' );
 
+diag 'Test HTML::Mason::Commands::PreprocessTransactionSearchQuery';
+
+my %processed = (
+    q{Type = 'Set'}                        => q{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND Type = 'Set'},
+    q{Type = 'Set' OR Type = 'Correspond'} =>
+        q{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND ( Type = 'Set' OR Type = 'Correspond' )},
+    q{( Type = 'Set' ) OR ( Type = 'Correspond' )} =>
+        q{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND ( ( Type = 'Set' ) OR ( Type = 'Correspond' ) )},
+    q{Type = 'Set' AND Field = 'Status'} =>
+        q{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND Type = 'Set' AND Field = 'Status'},
+    q{( Type = 'Set' AND Field = 'Status' ) OR ( Type = 'Correspond' )} =>
+        q{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND ( ( Type = 'Set' AND Field = 'Status' ) OR ( Type = 'Correspond' ) )},
+);
+
+local $HTML::Mason::Commands::session{CurrentUser} = RT->SystemUser;
+for my $query ( sort keys %processed ) {
+    is( HTML::Mason::Commands::PreprocessTransactionSearchQuery( Query => $query ),
+        $processed{$query}, "Processed query: $query" );
+}
+
 done_testing;

t/web/search_txns.t

@@ -35,7 +35,7 @@ diag "Query builder";
 
     $m->follow_link_ok( { text => 'Edit Search' }, 'Build Query' );
     my $form = $m->form_name('BuildQuery');
-    is($form->find_input('Query')->value, qq{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND ( TicketId = 1 )});
+    is($form->find_input('Query')->value, qq{TicketType = 'ticket' AND ObjectType = 'RT::Ticket' AND TicketId = 1});
 
     $m->field( TypeOp      => '=' );
     $m->field( ValueOfType => 'Create' );