[Rt-commit] rt branch 4.4/customfieldvalue-sortorder-integer-checking created. rt-4.4.5-32-gf6751bf990

[BPS Git Server]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 4.4/customfieldvalue-sortorder-integer-checking has been created
        at  f6751bf990689b6137ac28c4c5bab47e7381ca3d (commit)

- Log -----------------------------------------------------------------
commit f6751bf990689b6137ac28c4c5bab47e7381ca3d
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Mar 28 14:55:39 2022 -0500

    Improve checking of CustomFieldValue SortOrder
    
    Previously there was no proactive validation on the SortOrder for
    CustmField values.  Some databases are more picky than others and throw
    errors for non-integer values.  In one case the DBD error message was
    even returned to the user.
    
    Now SortOrder is validated to be either empty or contain an integer
    value (negative values are fine), plus a (more) user-friendly and
    localisable error message is generated.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index e449c86bbc..ff4b972fda 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -665,6 +665,11 @@ sub AddValue {
         return (0, $self->loc("Can't add a custom field value without a name"));
     }
 
+    # disallow non-integer sort order
+    if ( defined $args{'SortOrder'} and $args{'SortOrder'} !~ /^(?:-?\d+)?$/ ) {
+        return (0, $self->loc("SortOrder must be an integer value."));
+    }
+
     my $newval = RT::CustomFieldValue->new( $self->CurrentUser );
     return $newval->Create( %args, CustomField => $self->Id );
 }
diff --git a/lib/RT/CustomFieldValue.pm b/lib/RT/CustomFieldValue.pm
index 8eac5c7e3e..645c401fd6 100644
--- a/lib/RT/CustomFieldValue.pm
+++ b/lib/RT/CustomFieldValue.pm
@@ -123,6 +123,11 @@ sub _Set {
         return (0, $self->loc('Permission Denied')); 
     } 
 
+    # disallow non-integer sort order
+    if ( $args{Field} eq 'SortOrder' and $args{Value} !~ /^(?:-?\d+)?$/ ) {
+        return (0, $self->loc("SortOrder must be an integer value."));
+    }
+
     my ($ret, $msg) = $self->SUPER::_Set( @_ ); 
     if ( $args{Field} eq 'Name' ) {
         $cf->CleanupDefaultValues;
diff --git a/t/customfields/api.t b/t/customfields/api.t
index f3d186c321..cda126628c 100644
--- a/t/customfields/api.t
+++ b/t/customfields/api.t
@@ -58,6 +58,43 @@ $local_cf4->AddValue( Name => 'RecordCustomFieldValues41' );
 $local_cf4->AddValue( Name => 'RecordCustomFieldValues42' );
 
 
+# test value sort order
+my $local_cf5 = RT::CustomField->new( RT->SystemUser );
+$local_cf5->Create( Name => 'RecordCustomFields5', Type => 'SelectSingle', Queue => $queue2->id );
+{
+    my $i = 0;
+    my $first_cfv;
+    foreach my $valid (undef, '', 0, 1, -1, 10, 1.0) {
+        $i++;
+        my ($cfv_id, $msg) = $local_cf5->AddValue( Name => "RecordCustomFieldValues5$i", (defined $valid ? (SortOrder => $valid) : ()) );
+        ok($cfv_id, 'create custom field value with valid SortOrder');
+        my $cfv = RT::CustomFieldValue->new( RT->SystemUser );
+        $cfv->Load($cfv_id);
+        is($cfv->SortOrder, ($valid ? $valid : 0), 'SortOrder preserved');
+        $first_cfv ||= $cfv if $cfv->id;
+        if ($first_cfv->id != $cfv_id) {
+            my ($rc, $set_msg) = $first_cfv->SetSortOrder($valid);
+            if ($valid) {
+                ok($rc, 'successful modification');
+                is($first_cfv->SortOrder, $valid, 'SortOrder modified');
+            }
+            else {
+                is($rc, 0, 'expected failure');
+                is($set_msg, 'That is already the current value', 'expected failure reason');
+            }
+        }
+    }
+    foreach my $invalid ('1.0') {
+        $i++;
+        my ($cfv_id, $msg) = $local_cf5->AddValue( Name => "RecordCustomFieldValues5$i", SortOrder => $invalid );
+        is($cfv_id, 0, "custom field value not created with invalid SortOrder '$invalid'");
+        is($msg, 'SortOrder must be an integer value.', 'expected failure reason');
+        my ($rc, $set_msg) = $first_cfv->SetSortOrder($invalid);
+        is($rc, 0, 'expected failure');
+        is($set_msg, 'SortOrder must be an integer value.', 'expected failure reason');
+    }
+}
+
 my @custom_fields = ($local_cf1, $local_cf2, $global_cf3);
 
 

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


hooks/post-receive
-- 
rt