[Rt-commit] rt branch, 4.4/default-cfv-dupes, created. rt-4.4.1-123-gcfacf29
Shawn Moore
shawn at bestpractical.com
Wed Dec 28 12:40:40 EST 2016
The branch, 4.4/default-cfv-dupes has been created
at cfacf290d01a89e2a8fccbda4e4a83fe84b045bf (commit)
- Log -----------------------------------------------------------------
commit cfacf290d01a89e2a8fccbda4e4a83fe84b045bf
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..398a732 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,20 @@ 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}
+ // $names{ (keys %names)[0] }{Values};
+
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..8ac214f 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,20 @@ 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}
+ // $names{ (keys %names)[0] }{Values};
+
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