[Rt-commit] rt branch, 4.4/default-cfv-dupes, created. rt-4.4.1-123-g1c5e67f

Shawn Moore shawn at bestpractical.com
Wed Dec 28 12:36:21 EST 2016


The branch, 4.4/default-cfv-dupes has been created
        at  1c5e67fdf07f106516fb74ac882fb4e23da01b1c (commit)

- Log -----------------------------------------------------------------
commit 1c5e67fdf07f106516fb74ac882fb4e23da01b1c
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu Nov 17 18:18:45 2016 +0000

    Avoid using NamePrefix for CF fields on Defaults admin
    
    Though NamePrefix => 'Default-' does make processing %ARGS a little bit
    simpler, it means any JavaScript which looks at the field name to
    augment CF form fields won't match. In particular the
    sync_grouped_custom_fields() function in late.js, which handles
    duplicate CF form fields in the same form (caused by
    CustomFieldGroupings), doesn't handle the Default Values admin UI.  This
    means the browser submits multiple values for the same parameter in the
    form, ending up with ARRAY(0x...) in the default value.
    sync_grouped_custom_fields() keeps the current value in each of the
    duplicate form fields in sync on each change (which is why this commit
    uses an arbitrary value out of %names), and using the regular CF
    form field name avoids the problem of duplicate parameters coalescing
    into an array reference by using distinct parameter names, as they
    include the custom field grouping name.
    
    Changing the field names on the Default Values page was judged to be
    less intrusive than opening up the JavaScript to handle Default-#-Value,
    as we wouldn't want sync_grouped_custom_fields() to run on any false
    positives. Additionally this is one fewer way that Default Values is
    different from other forms, reducing the likelihood of future one-off
    problems like this.
    
    Fixes: I#32441

diff --git a/share/html/Admin/Assets/Catalogs/DefaultValues.html b/share/html/Admin/Assets/Catalogs/DefaultValues.html
index 8a00e16..f0b2a43 100644
--- a/share/html/Admin/Assets/Catalogs/DefaultValues.html
+++ b/share/html/Admin/Assets/Catalogs/DefaultValues.html
@@ -57,7 +57,6 @@
         $catalog->AssetCustomFields->LimitToDefaultValuesSupportedTypes
     },
     TitleBoxARGS => { title_class => "inverse" },
-    NamePrefix => 'Default-',
     Object => RT::Asset->new($session{CurrentUser})
 &>
 
@@ -80,13 +79,19 @@ if ( $ARGS{Reset} ) {
     }
 }
 elsif ( $ARGS{Update} ) {
-    for my $cf_id ( map { /^Default-(\d+)-/ ? $1 : ()  } keys %ARGS ) {
+    my $cfs = _ParseObjectCustomFieldArgs(\%ARGS)->{'RT::Asset'}{0};
+    for my $cf_id (keys %$cfs) {
+        # there may be multiple values submitted, pull out only the first,
+        # as our JS will make sure the values are synced
+        my %names = %{ $cfs->{$cf_id} };
+        my $value = $names{ (keys %names)[0] }{Value};
+
         my $cf = RT::CustomField->new($session{CurrentUser});
         $cf->Load($cf_id);
         if ( $cf->id && $cf->SupportDefaultValues ) {
             my ($ret, $msg) = $cf->SetDefaultValues(
                 Object => $catalog,
-                Values => $ARGS{'Default-' . $cf->id . '-Value'} // $ARGS{'Default-' . $cf->id . '-Values'},
+                Values => $value,
             );
             push @results, $msg;
         }
diff --git a/share/html/Admin/Queues/DefaultValues.html b/share/html/Admin/Queues/DefaultValues.html
index 0eccee7..d5591ba 100644
--- a/share/html/Admin/Queues/DefaultValues.html
+++ b/share/html/Admin/Queues/DefaultValues.html
@@ -66,7 +66,6 @@
         Default => $queue->DefaultValue('FinalPriority'),
     &><br /><span><em><&|/l&>requires running rt-crontool</&></em></span></td></tr>
     <& /Elements/EditCustomFields,
-        NamePrefix => 'Default-',
         Object => RT::Ticket->new($session{CurrentUser}),
         CustomFields => $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes,
         Grouping => 'Basics',
@@ -82,7 +81,6 @@
     <tr><td class="label"><&|/l&>Starts</&>:</td><td><& /Elements/SelectDate, Name => "Starts", Default => $queue->DefaultValue('Starts') || '' &></td></tr>
     <tr><td class="label"><&|/l&>Due</&>:</td><td><& /Elements/SelectDate, Name => "Due", Default => $queue->DefaultValue('Due') || '' &></td></tr>
     <& /Elements/EditCustomFields,
-        NamePrefix => 'Default-',
         Object => RT::Ticket->new($session{CurrentUser}),
         CustomFields => $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes,
         Grouping => 'Dates',
@@ -97,7 +95,6 @@
     <&|/Widgets/TitleBox, title => loc("People") &>
     <table>
     <& /Elements/EditCustomFields,
-        NamePrefix => 'Default-',
         Object => RT::Ticket->new($session{CurrentUser}),
         CustomFields => $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes,
         Grouping => 'People',
@@ -113,7 +110,6 @@
     <&|/Widgets/TitleBox, title => loc("Links") &>
     <table>
     <& /Elements/EditCustomFields,
-        NamePrefix => 'Default-',
         Object => RT::Ticket->new($session{CurrentUser}),
         CustomFields => $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes,
         Grouping => 'Links',
@@ -124,12 +120,12 @@
 </div>
 % }
 
-<& /Elements/EditCustomFieldCustomGroupings, CustomFieldGenerator => sub { $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes }, NamePrefix => 'Default-', Object => RT::Ticket->new($session{CurrentUser}) &>
+<& /Elements/EditCustomFieldCustomGroupings, CustomFieldGenerator => sub { $queue->TicketCustomFields->LimitToDefaultValuesSupportedTypes }, Object => RT::Ticket->new($session{CurrentUser}) &>
 
 <div class="ticket-info-cfs">
     <&|/Widgets/TitleBox, title => loc("Transaction Custom Fields") &>
     <table>
-    <& /Elements/EditCustomFields, CustomFields => $queue->TicketTransactionCustomFields->LimitToDefaultValuesSupportedTypes, NamePrefix => 'Default-', Object => RT::Transaction->new($session{CurrentUser}), QueueObj => $queue, InTable => 1 &>
+    <& /Elements/EditCustomFields, CustomFields => $queue->TicketTransactionCustomFields->LimitToDefaultValuesSupportedTypes, Object => RT::Transaction->new($session{CurrentUser}), QueueObj => $queue, InTable => 1 &>
     </table>
     </&>
 </div>
@@ -160,13 +156,19 @@ elsif ( $ARGS{Update} ) {
         );
         push @results, $msg;
     }
-    for my $cf_id ( map { /^Default-(\d+)-/ ? $1 : ()  } keys %ARGS ) {
+    my $cfs = _ParseObjectCustomFieldArgs(\%ARGS)->{'RT::Ticket'}{0};
+    for my $cf_id (keys %$cfs) {
+        # there may be multiple values submitted, pull out only the first,
+        # as our JS will make sure the values are synced
+        my %names = %{ $cfs->{$cf_id} };
+        my $value = $names{ (keys %names)[0] }{Value};
+
         my $cf = RT::CustomField->new($session{CurrentUser});
         $cf->Load($cf_id);
         if ( $cf->id && $cf->SupportDefaultValues ) {
             my ($ret, $msg) = $cf->SetDefaultValues(
                 Object => $queue,
-                Values => $ARGS{'Default-' . $cf->id . '-Value'} // $ARGS{'Default-' . $cf->id . '-Values'},
+                Values => $value,
             );
             push @results, $msg;
         }

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


More information about the rt-commit mailing list