[Rt-commit] rt branch, 4.0/apply-scrips-to-multiple-queues, updated. rt-4.0.4-170-g1dd9544
Ruslan Zakirov
ruz at bestpractical.com
Fri Jan 6 14:07:09 EST 2012
The branch, 4.0/apply-scrips-to-multiple-queues has been updated
via 1dd9544bca48822e89a8e2082235ca47f3698593 (commit)
via 28f8dec2c9afaf90601a22c73deb9c2f07a479f4 (commit)
via 0286ada6bad9ea4aa7f794a1682c1dc792e5c5f5 (commit)
from febe6646446bde5f21fa2c84b3085e531e4a8297 (commit)
Summary of changes:
lib/RT/Record/ApplyAndSort.pm | 180 +++++++++++++++++++++--------------------
t/api/scrip_order.t | 79 +++++++++++++++++-
2 files changed, 168 insertions(+), 91 deletions(-)
- Log -----------------------------------------------------------------
commit 0286ada6bad9ea4aa7f794a1682c1dc792e5c5f5
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Wed Jan 4 23:27:47 2012 +0400
more tests around scrips' order
diff --git a/t/api/scrip_order.t b/t/api/scrip_order.t
index 44d9da9..30c6b2e 100644
--- a/t/api/scrip_order.t
+++ b/t/api/scrip_order.t
@@ -3,7 +3,7 @@
use strict;
use RT;
-use RT::Test tests => 110;
+use RT::Test tests => 204;
my $queue = RT::Test->load_or_create_queue( Name => 'General' );
ok $queue && $queue->id, 'loaded or created queue';
@@ -70,6 +70,32 @@ note "move around two local scrips";
main->check_scrips_order(\@scrips, [$queue]);
}
+note "move around two global scrips";
+{
+ main->delete_all_scrips();
+
+ my @scrips;
+ push @scrips, main->create_scrip_ok( Queue => 0 );
+ push @scrips, main->create_scrip_ok( Queue => 0 );
+ main->check_scrips_order(\@scrips, [$queue]);
+
+ main->move_scrip_ok( $scrips[0], 0, 'down' );
+ @scrips = @scrips[1, 0];
+ main->check_scrips_order(\@scrips, [$queue]);
+
+ main->move_scrip_ok( $scrips[0], 0, 'down' );
+ @scrips = @scrips[1, 0];
+ main->check_scrips_order(\@scrips, [$queue]);
+
+ main->move_scrip_ok( $scrips[1], 0, 'up' );
+ @scrips = @scrips[1, 0];
+ main->check_scrips_order(\@scrips, [$queue]);
+
+ main->move_scrip_ok( $scrips[1], 0, 'up' );
+ @scrips = @scrips[1, 0];
+ main->check_scrips_order(\@scrips, [$queue]);
+}
+
note "move local scrip below global";
{
main->delete_all_scrips();
@@ -77,14 +103,30 @@ note "move local scrip below global";
push @scrips, main->create_scrip_ok( Queue => $queue->id );
push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
push @scrips, main->create_scrip_ok( Queue => 0 );
- push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
+ push @scrips, main->create_scrip_ok( Queue => $queue->id );
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
main->move_scrip_ok( $scrips[0], $queue->id, 'down' );
@scrips = @scrips[1, 2, 0, 3];
main->check_scrips_order(\@scrips, [$queue, $queue_B]);
}
-note "move global scrip";
+note "move local scrip above global";
+{
+ main->delete_all_scrips();
+ my @scrips;
+ push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
+ push @scrips, main->create_scrip_ok( Queue => 0 );
+ push @scrips, main->create_scrip_ok( Queue => $queue->id );
+ push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
+
+ main->move_scrip_ok( $scrips[-1], $queue_B->id, 'up' );
+ @scrips = @scrips[0, 3, 1, 2];
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
+}
+
+note "move global scrip down with local in between";
{
main->delete_all_scrips();
my @scrips;
@@ -93,12 +135,29 @@ note "move global scrip";
push @scrips, main->create_scrip_ok( Queue => $queue->id );
push @scrips, main->create_scrip_ok( Queue => 0 );
push @scrips, main->create_scrip_ok( Queue => $queue->id );
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
main->move_scrip_ok( $scrips[0], 0, 'down' );
@scrips = @scrips[1, 2, 3, 0, 4];
main->check_scrips_order(\@scrips, [$queue, $queue_B]);
}
+note "move global scrip up with local in between";
+{
+ main->delete_all_scrips();
+ my @scrips;
+ push @scrips, main->create_scrip_ok( Queue => $queue->id );
+ push @scrips, main->create_scrip_ok( Queue => 0 );
+ push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
+ push @scrips, main->create_scrip_ok( Queue => $queue->id );
+ push @scrips, main->create_scrip_ok( Queue => 0 );
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
+
+ main->move_scrip_ok( $scrips[-1], 0, 'up' );
+ @scrips = @scrips[0, 4, 1, 2, 3];
+ main->check_scrips_order(\@scrips, [$queue, $queue_B]);
+}
+
note "delete scrips one by one";
{
main->delete_all_scrips();
@@ -109,7 +168,6 @@ note "delete scrips one by one";
push @scrips, main->create_scrip_ok( Queue => $queue_B->id );
push @scrips, main->create_scrip_ok( Queue => $queue->id );
push @scrips, main->create_scrip_ok( Queue => 0 );
-
main->check_scrips_order(\@scrips, [$queue, $queue_B]);
foreach my $idx (3, 2, 0 ) {
@@ -119,6 +177,7 @@ note "delete scrips one by one";
}
sub create_scrip_ok {
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
my $self = shift;
my %args = (
ScripCondition => 'On Create',
@@ -138,6 +197,7 @@ sub create_scrip_ok {
}
sub delete_all_scrips {
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
my $self = shift;
my $scrips = RT::Scrips->new( RT->SystemUser );
$scrips->UnLimit;
@@ -145,6 +205,7 @@ sub delete_all_scrips {
}
sub move_scrip_ok {
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
my $self = shift;
my ($scrip, $queue, $dir) = @_;
@@ -158,6 +219,7 @@ sub move_scrip_ok {
}
sub check_scrips_order {
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
my $self = shift;
my $scrips = shift;
my $queues = shift;
@@ -199,4 +261,13 @@ sub check_scrips_order {
}
}
+sub dump_sort_order {
+
+ diag " id oid so";
+ diag join "\n", map { join "\t", @$_ } map @$_, $RT::Handle->dbh->selectall_arrayref(
+ "select Scrip, ObjectId, SortOrder from ObjectScrips ORDER BY SortOrder"
+ );
+
+}
+
commit 28f8dec2c9afaf90601a22c73deb9c2f07a479f4
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Wed Jan 4 23:28:40 2012 +0400
generalize, fix and comment Move{Up,Down} methods
Split moving into three primary cases:
1) global moving
2) local with local
3) local with global
First case was broken and other had corner cases.
Comment every branch of code. Use one method for
moving up and down.
diff --git a/lib/RT/Record/ApplyAndSort.pm b/lib/RT/Record/ApplyAndSort.pm
index 989efb1..df460d3 100644
--- a/lib/RT/Record/ApplyAndSort.pm
+++ b/lib/RT/Record/ApplyAndSort.pm
@@ -252,52 +252,7 @@ Moves scrip up. See </Sorting scrips applications>.
=cut
-sub MoveUp {
- my $self = shift;
-
- my $siblings = $self->Siblings;
- $siblings->Limit( FIELD => 'SortOrder', OPERATOR => '<', VALUE => $self->SortOrder );
- $siblings->OrderByCols( { FIELD => 'SortOrder', ORDER => 'DESC' } );
-
- my @above = ($siblings->Next, $siblings->Next);
- unless ($above[0]) {
- return (0, "Can not move up. It's already at the top");
- }
-
- my $new_sort_order;
- if ( $above[0]->ObjectId == $self->ObjectId ) {
- $new_sort_order = $above[0]->SortOrder;
- my ($status, $msg) = $above[0]->SetSortOrder( $self->SortOrder );
- unless ( $status ) {
- return (0, "Couldn't move scrip");
- }
- }
- elsif ( $above[1] && $above[0]->SortOrder == $above[1]->SortOrder + 1 ) {
- my $move_siblings = $self->Neighbors;
- $move_siblings->Limit(
- FIELD => 'SortOrder',
- OPERATOR => '>=',
- VALUE => $above[0]->SortOrder,
- );
- $move_siblings->OrderByCols( { FIELD => 'SortOrder', ORDER => 'DESC' } );
- while ( my $record = $move_siblings->Next ) {
- my ($status, $msg) = $record->SetSortOrder( $record->SortOrder + 1 );
- unless ( $status ) {
- return (0, "Couldn't move scrip");
- }
- }
- $new_sort_order = $above[0]->SortOrder;
- } else {
- $new_sort_order = $above[0]->SortOrder - 1;
- }
-
- my ($status, $msg) = $self->SetSortOrder( $new_sort_order );
- unless ( $status ) {
- return (0, "Couldn't move scrip");
- }
-
- return (1,"Moved scrip up");
-}
+sub MoveUp { return shift->Move( Up => @_ ) }
=head3 MoveDown
@@ -305,51 +260,98 @@ Moves scrip down. See </Sorting scrips applications>.
=cut
-sub MoveDown {
+sub MoveDown { return shift->Move( Down => @_ ) }
+
+sub Move {
my $self = shift;
+ my $dir = lc(shift || 'up');
+
+ my %meta;
+ if ( $dir eq 'down' ) {
+ %meta = qw(
+ next_op >
+ next_order ASC
+ prev_op <=
+ diff +1
+ );
+ } else {
+ %meta = qw(
+ next_op <
+ next_order DESC
+ prev_op >=
+ diff -1
+ );
+ }
my $siblings = $self->Siblings;
- $siblings->Limit( FIELD => 'SortOrder', OPERATOR => '>', VALUE => $self->SortOrder );
- $siblings->OrderByCols( { FIELD => 'SortOrder', ORDER => 'ASC' } );
-
- my @below = ($siblings->Next, $siblings->Next);
- unless ($below[0]) {
- return (0, "Can not move down. It's already at the bottom");
+ $siblings->Limit( FIELD => 'SortOrder', OPERATOR => $meta{'next_op'}, VALUE => $self->SortOrder );
+ $siblings->OrderBy( FIELD => 'SortOrder', ORDER => $meta{'next_order'} );
+
+ my @next = ($siblings->Next, $siblings->Next);
+ unless ($next[0]) {
+ return $dir eq 'down'
+ ? (0, "Can not move down. It's already at the bottom")
+ : (0, "Can not move up. It's already at the top")
+ ;
}
- my $new_sort_order;
- if ( $below[0]->ObjectId == $self->ObjectId ) {
- $new_sort_order = $below[0]->SortOrder;
- my ($status, $msg) = $below[0]->SetSortOrder( $self->SortOrder );
- unless ( $status ) {
- return (0, "Couldn't move scrip");
+ my ($new_sort_order, $move);
+
+ unless ( $self->ObjectId ) {
+ # moving global, it can not share sort order, so just move it
+ # on place of next global and move everything in between one number
+
+ $new_sort_order = $next[0]->SortOrder;
+ $move = $self->Neighbors;
+ $move->Limit(
+ FIELD => 'SortOrder', OPERATOR => $meta{'next_op'}, VALUE => $self->SortOrder,
+ );
+ $move->Limit(
+ FIELD => 'SortOrder', OPERATOR => $meta{'prev_op'}, VALUE => $next[0]->SortOrder,
+ ENTRYAGGREGATOR => 'AND',
+ );
+ }
+ elsif ( $next[0]->ObjectId == $self->ObjectId ) {
+ # moving two locals, just swap them, they should follow 'so = so+/-1' rule
+ $new_sort_order = $next[0]->SortOrder;
+ $move = $next[0];
+ }
+ else {
+ # moving local behind global
+ unless ( $self->IsSortOrderShared ) {
+ # not shared SO allows us to swap
+ $new_sort_order = $next[0]->SortOrder;
+ $move = $next[0];
+ }
+ elsif ( $next[1] ) {
+ # more records there and shared SO, we have to move everything
+ $new_sort_order = $next[0]->SortOrder;
+ $move = $self->Neighbors;
+ $move->Limit(
+ FIELD => 'SortOrder', OPERATOR => $meta{prev_op}, VALUE => $next[0]->SortOrder,
+ );
+ }
+ else {
+ # shared SO and place after is free, so just jump
+ $new_sort_order = $next[0]->SortOrder + $meta{'diff'};
}
}
- elsif ( $below[1] && $below[0]->SortOrder + 1 == $below[1]->SortOrder ) {
- my $move_siblings = $self->Neighbors;
- $move_siblings->Limit(
- FIELD => 'SortOrder',
- OPERATOR => '<=',
- VALUE => $below[0]->SortOrder,
- );
- $move_siblings->OrderByCols( { FIELD => 'SortOrder', ORDER => 'ASC' } );
- while ( my $record = $move_siblings->Next ) {
- my ($status, $msg) = $record->SetSortOrder( $record->SortOrder - 1 );
- unless ( $status ) {
- return (0, "Couldn't move scrip");
- }
+
+ if ( $move ) {
+ foreach my $record ( $move->isa('RT::Record')? ($move) : @{ $move->ItemsArrayRef } ) {
+ my ($status, $msg) = $record->SetSortOrder(
+ $record->SortOrder - $meta{'diff'}
+ );
+ return (0, "Couldn't move: $msg") unless $status;
}
- $new_sort_order = $below[0]->SortOrder;
- } else {
- $new_sort_order = $below[0]->SortOrder + 1;
}
my ($status, $msg) = $self->SetSortOrder( $new_sort_order );
unless ( $status ) {
- return (0, "Couldn't move scrip");
+ return (0, "Couldn't move: $msg");
}
- return (1,"Moved scrip down");
+ return (1,"Moved");
}
sub NextSortOrder {
@@ -372,6 +374,16 @@ sub NextSortOrder {
return $first->SortOrder + 1;
}
+sub IsSortOrderShared {
+ my $self = shift;
+ return 0 unless $self->ObjectId;
+
+ my $neighbors = $self->Neighbors;
+ $neighbors->Limit( FIELD => 'id', OPERATOR => '!=', VALUE => $self->id );
+ $neighbors->Limit( FIELD => 'SortOrder', VALUE => $self->SortOrder );
+ return $neighbors->Count;
+}
+
sub TargetObj {
my $self = shift;
my $id = shift;
commit 1dd9544bca48822e89a8e2082235ca47f3698593
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Wed Jan 4 23:35:37 2012 +0400
use IsSortOrderShared in Delete
current version has some tiny performance benefits,
win is so small that it doesn't worth code complexity.
diff --git a/lib/RT/Record/ApplyAndSort.pm b/lib/RT/Record/ApplyAndSort.pm
index df460d3..a63a243 100644
--- a/lib/RT/Record/ApplyAndSort.pm
+++ b/lib/RT/Record/ApplyAndSort.pm
@@ -180,23 +180,17 @@ sub _AppliedTo {
sub Delete {
my $self = shift;
+ return $self->SUPER::Delete if $self->IsSortOrderShared;
+
+ # Move everything below us up
my $siblings = $self->Neighbors;
$siblings->Limit( FIELD => 'SortOrder', OPERATOR => '>=', VALUE => $self->SortOrder );
$siblings->OrderBy( FIELD => 'SortOrder', ORDER => 'ASC' );
-
- my $sort_order = $self->SortOrder;
- while (my $record = $siblings->Next) {
- next if $record->id == $self->id;
- return $self->SUPER::Delete if $self->SortOrder == $record->SortOrder;
- last;
- }
-
- # Move everything below us up
foreach my $record ( @{ $siblings->ItemsArrayRef } ) {
$record->SetSortOrder($record->SortOrder - 1);
}
- $self->SUPER::Delete;
+ return $self->SUPER::Delete;
}
sub DeleteAll {
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list