[Rt-commit] rt branch, 4.4/disabled-cf-sortorder, created. rt-4.4.2-89-g7fd4f490e

? sunnavy sunnavy at bestpractical.com
Mon Mar 5 17:21:26 EST 2018


The branch, 4.4/disabled-cf-sortorder has been created
        at  7fd4f490e1d0eeb6a6d5fd94a06ed896004bbe8a (commit)

- Log -----------------------------------------------------------------
commit 64addf7b62d8b0faea3340803406889bc808fa9d
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Nov 9 10:03:47 2017 -0500

    Test showing incorrect sort order on re-enabled CF

diff --git a/t/api/customfield.t b/t/api/customfield.t
index dedeaa236..916699434 100644
--- a/t/api/customfield.t
+++ b/t/api/customfield.t
@@ -382,5 +382,61 @@ $cf = RT::CustomField->new( RT->SystemUser );
 $cf->LoadByName(Name => 'TestingCF', Queue => 1, IncludeDisabled => 0 );
 ok( ! $cf->id, "Doesn't find it if IncludeDisabled => 0" );
 
+$cf = RT::CustomField->new( RT->SystemUser );
+$cf->LoadByName(Name => 'TestingCF', Queue => 1, IncludeDisabled => 1 );
+($ok, $msg) = $cf->SetDisabled(0);
+ok($ok, "Re-enabled CF " . $cf->Name);
+
+diag 'Test sort ordering when disabling and re-enabling CFs';
+
+# Create more CFs
+foreach my $i (2..5) {
+    ($ok, $msg) = $cf->Create(
+        Name        => "TestingCF$i",
+        Queue       => '0',
+        Description => 'Global CF',
+        Type        => 'SelectSingle'
+    );
+    ok($ok, "Created " . $cf->Name . " successfully");
+
+    $ocf = RT::ObjectCustomField->new( RT->SystemUser );
+    ( $ok, $msg ) = $ocf->LoadByCols( CustomField => $cf->id, ObjectId => 0 );
+    ok( $ok, "Found OCF " . $ocf->Id);
+    is( $ocf->SortOrder, $i, "Sort order is $i for OCF " . $ocf->Id);
+}
+
+diag 'Disable TestingCF4';
+$cf = RT::CustomField->new( RT->SystemUser );
+$cf->LoadByName(Name => 'TestingCF4', Queue => 0 );
+($ok, $msg) = $cf->SetDisabled(1);
+ok($ok, "Disabled " . $cf->Name);
+
+diag 'MoveUp TestingCF5';
+$cf = RT::CustomField->new( RT->SystemUser );
+$cf->LoadByName(Name => 'TestingCF5', Queue => 0 );
+$ocf = RT::ObjectCustomField->new( RT->SystemUser );
+( $ok, $msg ) = $ocf->LoadByCols( CustomField => $cf->id, ObjectId => 0 );
+ok( $ok, "Found OCF " . $ocf->Id . " for " . $cf->Name );
+is( $ocf->SortOrder, 5, 'Sort order before MoveUp');
+$ocf->MoveUp;
+is( $ocf->SortOrder, 3, 'Sort order after MoveUp');
+
+$cf = RT::CustomField->new( RT->SystemUser );
+$cf->LoadByName(Name => 'TestingCF3', Queue => 0 );
+$ocf = RT::ObjectCustomField->new( RT->SystemUser );
+( $ok, $msg ) = $ocf->LoadByCols( CustomField => $cf->id, ObjectId => 0 );
+ok( $ok, "Found OCF " . $ocf->Id . " for " . $cf->Name );
+is( $ocf->SortOrder, 4, 'Sort order is 4 for OCF ' . $ocf->Id);
+
+
+# Sort order should become 5 when re-enabled and not stay at 4
+$cf = RT::CustomField->new( RT->SystemUser );
+$cf->LoadByName(Name => 'TestingCF4', Queue => 0 );
+($ok, $msg) = $cf->SetDisabled(0);
+ok($ok, "Re-enabled " . $cf->Name);
+$ocf = RT::ObjectCustomField->new( RT->SystemUser );
+( $ok, $msg ) = $ocf->LoadByCols( CustomField => $cf->id, ObjectId => 0 );
+ok( $ok, "Found OCF for " . $cf->Name );
+is( $ocf->SortOrder, 5, 'Sort order is 5, CF moved to bottom of list on re-enable');
 
 done_testing;

commit 9e4e31abcc5a214f6612819eaea1ea4318aac49a
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Nov 9 11:44:25 2017 -0500

    Reset ObjectCustomField sort order when re-enabling a Custom Field
    
    When disabling and re-enabling custom fields, the sort order stored
    in the ObjectCustomFields table is retained. If the sort order in a list
    of CFs is modified with MoveUp and MoveDown and a disabled CF is
    then re-enabled, the previous sort order can be the same as one of
    the moved OCFs.
    
    On re-enabling a custom field, move it to the bottom of the sort
    order for all ObjectCustomField records to avoid duplicate sort
    order values.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 97010cbe5..f21749240 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1181,6 +1181,19 @@ sub SetDisabled {
         return ($status, $msg);
     }
 
+    # Set to the end of the sort list when re-enabling to prevent duplicate
+    # sort order values.
+    if ( $val == 0 ) {
+        my $ocfs = RT::ObjectCustomFields->new( $self->CurrentUser );
+        $ocfs->LimitToCustomField( $self->id );
+
+        while ( my $ocf = $ocfs->Next ) {
+            my $sort = $ocf->NextSortOrder();
+            $ocf->SetSortOrder($sort);
+            RT::Logger->debug("Set Sort Order to $sort for Object Custom Field " . $ocf->Id);
+        }
+    }
+
     if ( $val == 1 ) {
         return (1, $self->loc("Disabled"));
     } else {

commit 7fd4f490e1d0eeb6a6d5fd94a06ed896004bbe8a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Mar 6 06:11:09 2018 +0800

    update ObjectCustomField sort order only if necessary when re-enabling a Custom Field
    
    If an ObjectCustomField object is already the last one and the sort
    order is not shared, then there is no need to update.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index f21749240..0309cdfc9 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1188,9 +1188,19 @@ sub SetDisabled {
         $ocfs->LimitToCustomField( $self->id );
 
         while ( my $ocf = $ocfs->Next ) {
-            my $sort = $ocf->NextSortOrder();
-            $ocf->SetSortOrder($sort);
-            RT::Logger->debug("Set Sort Order to $sort for Object Custom Field " . $ocf->Id);
+            my $last_object = $ocf->LastObject() || $ocf;
+            # no need to update if it's already the last one.
+            if ( $ocf->id != $last_object->id || $last_object->IsSortOrderShared ) {
+                my $sort = $last_object->SortOrder + 1;
+                my ( $status, $msg ) = $ocf->SetSortOrder( $sort );
+                if ( $status ) {
+                    RT::Logger->debug( "Set Sort Order to $sort for Object Custom Field " . $ocf->Id );
+                }
+                else {
+                    RT->Logger->error(
+                        "Failed to set Sort Order to $sort for ObjectCustomField " . $ocf->id . ": $msg" );
+                }
+            }
         }
     }
 
diff --git a/lib/RT/Record/AddAndSort.pm b/lib/RT/Record/AddAndSort.pm
index cbacf270a..e3fa95b23 100644
--- a/lib/RT/Record/AddAndSort.pm
+++ b/lib/RT/Record/AddAndSort.pm
@@ -491,6 +491,22 @@ calls ObjectId if it's not provided.
 =cut
 
 sub NextSortOrder {
+    my $self        = shift;
+    my $last_object = $self->LastObject( @_ );
+    return 0 unless $last_object;
+    return $last_object->SortOrder + 1;
+}
+
+
+=head3 LastObject
+
+Returns the object with maximum SortOrder in the L<neighborhood|/Neighbors>.
+Pass arguments to L</Neighbors> and can take optional ObjectId argument,
+calls ObjectId if it's not provided.
+
+=cut
+
+sub LastObject {
     my $self = shift;
     my %args = (@_);
 
@@ -506,8 +522,7 @@ sub NextSortOrder {
         $neighbors->UnLimit;
     }
     $neighbors->OrderBy( FIELD => 'SortOrder', ORDER => 'DESC' );
-    return 0 unless my $first = $neighbors->First;
-    return $first->SortOrder + 1;
+    return $neighbors->First;
 }
 
 =head3 IsSortOrderShared

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


More information about the rt-commit mailing list