[Rt-commit] rt branch, 4.2/order-by-cf, created. rt-4.1.13-18-g63ca3d8

Alex Vandiver alexmv at bestpractical.com
Tue Jun 18 20:42:02 EDT 2013


The branch, 4.2/order-by-cf has been created
        at  63ca3d85675c15de8d6002f16ed037666c2b5cb1 (commit)

- Log -----------------------------------------------------------------
commit 6875c85a75e447736023f549d895966f8786e26b
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 18 17:10:49 2013 -0700

    Modernize t/cfsort-freeform-single.t
    
    This adds additional tests, but does not remove any existing tests.

diff --git a/t/ticket/cfsort-freeform-single.t b/t/ticket/cfsort-freeform-single.t
index ae109e9..817c3fe 100644
--- a/t/ticket/cfsort-freeform-single.t
+++ b/t/ticket/cfsort-freeform-single.t
@@ -1,158 +1,126 @@
 
-use RT::Test nodata => 1, tests => 89;
-
 use strict;
 use warnings;
 
-use RT::Tickets;
-use RT::Queue;
-use RT::CustomField;
-
-# Test Sorting by FreeformSingle custom field.
+use RT::Test nodata => 1, tests => undef;
 
-diag "Create a queue to test with.";
-my $queue_name = "CFSortQueue-$$";
-my $queue;
-{
-    $queue = RT::Queue->new( RT->SystemUser );
-    my ($ret, $msg) = $queue->Create(
-        Name => $queue_name,
-        Description => 'queue for custom field sort testing'
-    );
-    ok($ret, "$queue test queue creation. $msg");
-}
+my $queue = RT::Test->load_or_create_queue( Name => "sorting" );
+ok $queue && $queue->id, "Created queue";
+my $queue_name = $queue->Name;
 
 # CFs for testing, later we create another one
-my %CF;
-my $cf_name;
-
+my $cf;
+my $cf_name = "ordering";
 diag "create a CF";
 {
-    $cf_name = $CF{'CF'}{'name'} = "Order$$";
-    $CF{'CF'}{'obj'} = RT::CustomField->new( RT->SystemUser );
-    my ($ret, $msg) = $CF{'CF'}{'obj'}->Create(
-        Name  => $CF{'CF'}{'name'},
+    $cf = RT::CustomField->new( RT->SystemUser );
+    my ($ret, $msg) = $cf->Create(
+        Name  => $cf_name,
         Queue => $queue->id,
         Type  => 'FreeformSingle',
     );
-    ok($ret, "Custom Field $CF{'CF'}{'name'} created");
-}
-
-my ($total, @data, @tickets, @test) = (0, ());
-
-sub run_tests {
-    my $query_prefix = join ' OR ', map 'id = '. $_->id, @tickets;
-    foreach my $test ( @test ) {
-        my $query = join " AND ", map "( $_ )", grep defined && length,
-            $query_prefix, $test->{'Query'};
-
-        foreach my $order (qw(ASC DESC)) {
-            my $error = 0;
-            my $tix = RT::Tickets->new( RT->SystemUser );
-            $tix->FromSQL( $query );
-            $tix->OrderBy( FIELD => $test->{'Order'}, ORDER => $order );
-
-            ok($tix->Count, "found ticket(s)")
-                or $error = 1;
-
-            my ($order_ok, $last) = (1, $order eq 'ASC'? '-': 'zzzzzz');
-            my $last_id = $tix->Last->id;
-            while ( my $t = $tix->Next ) {
-                my $tmp;
-                next if $t->id == $last_id and $t->Subject eq "-"; # Nulls are allowed to come last, in Pg
-
-                if ( $order eq 'ASC' ) {
-                    $tmp = ((split( /,/, $last))[0] cmp (split( /,/, $t->Subject))[0]);
-                } else {
-                    $tmp = -((split( /,/, $last))[-1] cmp (split( /,/, $t->Subject))[-1]);
-                }
-                if ( $tmp > 0 ) {
-                    $order_ok = 0; last;
-                }
-                $last = $t->Subject;
-            }
-
-            ok( $order_ok, "$order order of tickets is good" )
-                or $error = 1;
-
-            if ( $error ) {
-                diag "Wrong SQL query:". $tix->BuildSelectQuery;
-                $tix->GotoFirstItem;
-                while ( my $t = $tix->Next ) {
-                    diag sprintf "%02d - %s", $t->id, $t->Subject;
-                }
-            }
-        }
-    }
+    ok($ret, "Custom Field created");
 }
 
- at data = (
-    { Subject => '-' },
-    { Subject => 'a', 'CustomField-' . $CF{CF}{obj}->id => 'a' },
-    { Subject => 'b', 'CustomField-' . $CF{CF}{obj}->id => 'b' },
+run_tests(
+    [
+        { Subject => '-' },
+        { Subject => 'aa', 'CustomField-' . $cf->id => 'aa' },
+        { Subject => 'bb', 'CustomField-' . $cf->id => 'bb' },
+        { Subject => 'cc', 'CustomField-' . $cf->id => 'cc' },
+    ],
+    {                                    Count => 4, Order => "CF.{$cf_name}"             },
+    {                                    Count => 4, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_name} LIKE 'a'", Count => 1, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_name} LIKE 'a'", Count => 1, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_name} != 'cc'",  Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_name} != 'cc'",  Count => 3, Order => "CF.$queue_name.{$cf_name}" },
 );
 
- at tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @data);
- at test = (
-    { Order => "CF.{$cf_name}" },
-    { Order => "CF.$queue_name.{$cf_name}" },
-);
-run_tests();
-
- at data = (
-    { Subject => '-' },
-    { Subject => 'aa', 'CustomField-' . $CF{CF}{obj}->id => 'aa' },
-    { Subject => 'bb', 'CustomField-' . $CF{CF}{obj}->id => 'bb' },
-);
- at tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @data);
- at test = (
-    { Query => "CF.{$cf_name} LIKE 'a'", Order => "CF.{$cf_name}" },
-    { Query => "CF.{$cf_name} LIKE 'a'", Order => "CF.$queue_name.{$cf_name}" },
-);
-run_tests();
-
- at data = (
-    { Subject => '-', },
-    { Subject => 'a', CF => 'a' },
-    { Subject => 'b', CF => 'b' },
-    { Subject => 'c', CF => 'c' },
-);
- at tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @data);
- at test = (
-    { Query => "CF.{$cf_name} != 'c'", Order => "CF.{$cf_name}" },
-    { Query => "CF.{$cf_name} != 'c'", Order => "CF.$queue_name.{$cf_name}" },
-);
-run_tests();
-
 
 
+my $other_cf;
+my $other_name = "othercf";
 diag "create another CF";
 {
-    $CF{'AnotherCF'}{'name'} = "OrderAnother$$";
-    $CF{'AnotherCF'}{'obj'} = RT::CustomField->new( RT->SystemUser );
-    my ($ret, $msg) = $CF{'AnotherCF'}{'obj'}->Create(
-        Name  => $CF{'AnotherCF'}{'name'},
+    $other_cf = RT::CustomField->new( RT->SystemUser );
+    my ($ret, $msg) = $other_cf->Create(
+        Name  => $other_name,
         Queue => $queue->id,
         Type  => 'FreeformSingle',
     );
-    ok($ret, "Custom Field $CF{'AnotherCF'}{'name'} created");
+    ok($ret, "Other Custom Field created");
 }
 
-# test that order is not affect by other fields (had such problem)
- at data = (
-    { Subject => '-', },
-    { Subject => 'a', CF => 'a', AnotherCF => 'za' },
-    { Subject => 'b', CF => 'b', AnotherCF => 'ya' },
-    { Subject => 'c', CF => 'c', AnotherCF => 'xa' },
-);
- at tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @data);
- at test = (
-    { Order => "CF.{$cf_name}" },
-    { Order => "CF.$queue_name.{$cf_name}" },
-    { Query => "CF.{$cf_name} != 'c'", Order => "CF.{$cf_name}" },
-    { Query => "CF.{$cf_name} != 'c'", Order => "CF.$queue_name.{$cf_name}" },
+# Test that order is not affected by other CFs
+run_tests(
+    [
+        { Subject => '-', },
+        { Subject => 'aa', "CustomField-" . $cf->id => 'aa', "CustomField-" . $other_cf->id => 'za' },
+        { Subject => 'bb', "CustomField-" . $cf->id => 'bb', "CustomField-" . $other_cf->id => 'ya' },
+        { Subject => 'cc', "CustomField-" . $cf->id => 'cc', "CustomField-" . $other_cf->id => 'xa' },
+    ],
+    {                                      Count => 4, Order => "CF.{$cf_name}"             },
+    {                                      Count => 4, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_name} LIKE 'a'",   Count => 1, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_name} LIKE 'a'",   Count => 1, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_name} != 'cc'",    Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_name} != 'cc'",    Count => 3, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$other_name} != 'za'", Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$other_name} != 'za'", Count => 3, Order => "CF.$queue_name.{$cf_name}" },
 );
-run_tests();
 
- at tickets = ();
+sub run_tests {
+    my $tickets = shift;
+    my @tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @{ $tickets });
+    my $base_query = join(" OR ", map {"id = ".$_->id} @tickets) || "id > 0";
+
+    my @tests = @_;
+    for my $test ( @tests ) {
+        $test->{'Query'} ||= "id > 0";
+        my $query = "( $base_query ) AND " . $test->{'Query'};
+        for my $order (qw(ASC DESC)) {
+            subtest $test->{'Query'} . " ORDER BY ".$test->{'Order'}. " $order" => sub {
+                my $error = 0;
+                my $tix = RT::Tickets->new( RT->SystemUser );
+                $tix->FromSQL( $query );
+                $tix->OrderBy( FIELD => $test->{'Order'}, ORDER => $order );
+
+                is($tix->Count, $test->{'Count'}, "found right number of tickets (".$test->{Count}.")")
+                    or $error = 1;
+
+                my ($order_ok, $last) = (1, $order eq 'ASC'? '-': 'zzzzzz');
+                if ($tix->Count) {
+                    my $last_id = $tix->Last->id;
+                    while ( my $t = $tix->Next ) {
+                        my $tmp;
+                        next if $t->id == $last_id and $t->Subject eq "-"; # Nulls are allowed to come last, in Pg
+
+                        if ( $order eq 'ASC' ) {
+                            $tmp = ((split( /,/, $last))[0] cmp (split( /,/, $t->Subject))[0]);
+                        } else {
+                            $tmp = -((split( /,/, $last))[-1] cmp (split( /,/, $t->Subject))[-1]);
+                        }
+                        if ( $tmp > 0 ) {
+                            $order_ok = 0; last;
+                        }
+                        $last = $t->Subject;
+                    }
+                }
+
+                ok( $order_ok, "$order order of tickets is good" )
+                    or $error = 1;
+
+                if ( $error ) {
+                    diag "Wrong SQL query:". $tix->BuildSelectQuery;
+                    $tix->GotoFirstItem;
+                    while ( my $t = $tix->Next ) {
+                        diag sprintf "%02d - %s", $t->id, $t->Subject;
+                    }
+                }
+            };
+        }
+    }
+}
 
+done_testing;

commit ea4446e57bf5ef618b8d0af93fcbba6d6fe7e3d2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 18 17:12:17 2013 -0700

    Add failing tests for ordering by a CF when a duplicate CF exists
    
    Queries appear to return 0 results.

diff --git a/t/ticket/cfsort-freeform-single.t b/t/ticket/cfsort-freeform-single.t
index 817c3fe..262f84a 100644
--- a/t/ticket/cfsort-freeform-single.t
+++ b/t/ticket/cfsort-freeform-single.t
@@ -70,6 +70,49 @@ run_tests(
     { Query => "CF.{$other_name} != 'za'", Count => 3, Order => "CF.$queue_name.{$cf_name}" },
 );
 
+# And then add a CF with a duplicate name, on a different queue
+{
+    my $other_queue = RT::Test->load_or_create_queue( Name => "other_queue" );
+    ok $other_queue && $other_queue->id, "Created queue";
+
+    my $dup = RT::CustomField->new( RT->SystemUser );
+    my ($ret, $msg) = $dup->Create(
+        Name  => $cf_name,
+        Queue => $other_queue->id,
+        Type  => 'FreeformSingle',
+    );
+    ok($ret, "Custom Field created");
+}
+
+my $cf_id = $cf->id;
+run_tests(
+    [
+        { Subject => '-', },
+        { Subject => 'aa', "CustomField-" . $cf->id => 'aa', "CustomField-" . $other_cf->id => 'za' },
+        { Subject => 'bb', "CustomField-" . $cf->id => 'bb', "CustomField-" . $other_cf->id => 'ya' },
+        { Subject => 'cc', "CustomField-" . $cf->id => 'cc', "CustomField-" . $other_cf->id => 'xa' },
+    ],
+    {                                                Count => 4, Order => "CF.{$cf_name}"             },
+    {                                                Count => 4, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_id} LIKE 'a'",               Count => 1, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_id} LIKE 'a'",               Count => 1, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$cf_id} != 'cc'",                Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$cf_id} != 'cc'",                Count => 3, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.$queue_name.{$cf_name} LIKE 'a'", Count => 1, Order => "CF.{$cf_name}"             },
+    { Query => "CF.$queue_name.{$cf_name} LIKE 'a'", Count => 1, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.$queue_name.{$cf_name} != 'cc'",  Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.$queue_name.{$cf_name} != 'cc'",  Count => 3, Order => "CF.$queue_name.{$cf_name}" },
+    { Query => "CF.{$other_name} != 'za'",           Count => 3, Order => "CF.{$cf_name}"             },
+    { Query => "CF.{$other_name} != 'za'",           Count => 3, Order => "CF.$queue_name.{$cf_name}" },
+
+    { Query => "CF.{$cf_id} != 'cc'",                Count => 3, Order => "CF.{$cf_id}"               },
+    { Query => "CF.{$cf_id} != 'cc'",                Count => 3, Order => "CF.$queue_name.{$cf_id}"   },
+    { Query => "CF.$queue_name.{$cf_name} != 'cc'",  Count => 3, Order => "CF.{$cf_id}"               },
+    { Query => "CF.$queue_name.{$cf_name} != 'cc'",  Count => 3, Order => "CF.$queue_name.{$cf_id}"   },
+    { Query => "CF.{$other_name} != 'za'",           Count => 3, Order => "CF.{$cf_id}"               },
+    { Query => "CF.{$other_name} != 'za'",           Count => 3, Order => "CF.$queue_name.{$cf_id}"   },
+);
+
 sub run_tests {
     my $tickets = shift;
     my @tickets = RT::Test->create_tickets( { Queue => $queue->id, RandomOrder => 1 }, @{ $tickets });

commit 63ca3d85675c15de8d6002f16ed037666c2b5cb1
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 18 17:14:03 2013 -0700

    CF key and CF to order by must be passed as two different values
    
    0f6ee87 refactored CF ordering into _OrderByCF; in doing so, the new
    _OrderByCF function was written to take a set of ordering information
    and a custom field.  However, in cases where the CF could not be
    uniquely determined, what it was _passed_ was "$queue.$field".  This was
    in turn used internally to JOIN against CustomFields.Name, and found no
    results.  Because the JOIN was not a LEFT JOIN, this caused no rows to
    be returned.
    
    Like _CustomFieldJoin itself, the CF key must be passed separately from
    the customfield name or object.  Undo the slightly overzealous
    refactoring to force callers to use their additional information to
    separate out different CF keys.

diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index 3dd82b7..f9894e9 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -138,9 +138,8 @@ sub JoinTransactions {
 
 sub _OrderByCF {
     my $self = shift;
-    my ($row, $cf) = @_;
+    my ($row, $cfkey, $cf) = @_;
 
-    my $cfkey = blessed($cf) ? $cf->id : $cf;
     $cfkey .= ".ordering" if !blessed($cf) || ($cf->MaxValues||0) != 1;
     my ($ocfvs, $CFs) = $self->_CustomFieldJoin( $cfkey, $cf );
     # this is described in _LimitCustomField
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 126bdc3..fd7e957 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -1239,7 +1239,8 @@ sub OrderByCols {
             push @res, { %$row, ALIAS => $users, FIELD => $subkey };
        } elsif ( defined $meta->[0] && $meta->[0] eq 'CUSTOMFIELD' ) {
            my ($queue, $field, $cf, $column) = $self->_CustomFieldDecipher( $subkey );
-           push @res, $self->_OrderByCF( $row, ($cf || "$queue.$field") );
+           my $cfkey = $cf ? $cf->id : "$queue.$field";
+           push @res, $self->_OrderByCF( $row, $cfkey, ($cf || $field) );
        } elsif ( $field eq "Custom" && $subkey eq "Ownership") {
            # PAW logic is "reversed"
            my $order = "ASC";

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


More information about the Rt-commit mailing list