[Rt-commit] rt branch, 4.2/custom-field-groupings, updated. rt-4.0.8-481-g581d05e

Alex Vandiver alexmv at bestpractical.com
Fri Nov 16 23:05:59 EST 2012


The branch, 4.2/custom-field-groupings has been updated
       via  581d05e450d94728ccbb359a62d64fe953518f45 (commit)
       via  f19d36c0a18df993b5cc7f34c81397db3bcd5bab (commit)
       via  32a75a6f5721a8ee85a2cb9de9169098a3316fc9 (commit)
       via  8d87f2ce8a2ac882127983c5aa1fbcb3d4742b3f (commit)
      from  b4ca13dbb9849296f28a0b2798801643ca54715e (commit)

Summary of changes:
 lib/RT/Interface/Web.pm                  | 34 +++++++++++++++++++++++++++-----
 share/html/Elements/EditCustomField      |  6 +++++-
 share/html/Elements/ValidateCustomFields |  4 +++-
 share/html/NoAuth/js/late.js             | 34 ++++++++++++++++++++++++++++++++
 t/web/cf_groups.t                        | 34 ++++++++++++++++++--------------
 t/web/cf_groups_users.t                  | 26 +++++++++++++-----------
 6 files changed, 104 insertions(+), 34 deletions(-)

- Log -----------------------------------------------------------------
commit 8d87f2ce8a2ac882127983c5aa1fbcb3d4742b3f
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri Nov 16 22:29:51 2012 -0500

    Push the CF grouping into the NamePrefix, to allow for separation
    
    If a custom field is rendered in multiple groupings on one page, and is
    a select-multiple CF, the values between the two will be intermingled;
    if a single-valued CF, the rendered defaults will become ARRAY(0xBEEF).
    
    Avoid this by splitting -Value, -Magic, etc, on a per-grouping basis.
    This allows us to associate them on a per-widget basis on the back-end,
    and at least ensure that a consistent value is seen from what is
    submitted.  In this case, we arbitrarily choose the alphabetically first
    grouping as the one whose value is used.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 8c4d6d6..48b368c 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2610,6 +2610,7 @@ sub ProcessObjectCustomFieldUpdates {
                     $RT::Logger->warning("Couldn't load custom field #$cf");
                     next;
                 }
+                my @groupings = sort keys %{ $custom_fields_to_mod{$class}{$id}{$cf} };
                 push @results,
                     _ProcessObjectCustomFieldUpdates(
                     # XXX FIXME: Prefix is not quite right, as $id almost
@@ -2618,7 +2619,7 @@ sub ProcessObjectCustomFieldUpdates {
                     Prefix      => "Object-$class-$id-CustomField-$cf-",
                     Object      => $Object,
                     CustomField => $CustomFieldObj,
-                    ARGS        => $custom_fields_to_mod{$class}{$id}{$cf},
+                    ARGS        => $custom_fields_to_mod{$class}{$id}{$cf}{$groupings[0]},
                     );
             }
         }
@@ -2632,11 +2633,12 @@ sub _ParseObjectCustomFieldArgs {
 
     foreach my $arg ( keys %$ARGSRef ) {
 
-        # format: Object-<object class>-<object id>-CustomField-<CF id>-<commands>
-        next unless $arg =~ /^Object-([\w:]+)-(\d*)-CustomField-(\d+)-(.*)$/;
+        # format: Object-<object class>-<object id>-CustomField[:<grouping>]-<CF id>-<commands>
+        next unless $arg =~ /^Object-([\w:]+)-(\d*)-CustomField(?::(\w+))?-(\d+)-(.*)$/;
 
         # For each of those objects, find out what custom fields we want to work with.
-        $custom_fields_to_mod{$1}{ $2 || 0 }{$3}{$4} = $ARGSRef->{$arg};
+        #                   Class     ID     CF  grouping command
+        $custom_fields_to_mod{$1}{ $2 || 0 }{$4}{$3 || ''}{$5} = $ARGSRef->{$arg};
     }
 
     return wantarray ? %custom_fields_to_mod : \%custom_fields_to_mod;
@@ -2814,8 +2816,9 @@ sub ProcessObjectCustomFieldUpdatesForCreate {
                 next;
             }
 
+            my @groupings = sort keys %{ $custom_fields{$class}{0}{$cfid} };
             my @values;
-            while (my ($arg, $value) = each %{ $custom_fields{$class}{0}{$cfid} }) {
+            while (my ($arg, $value) = each %{ $custom_fields{$class}{0}{$cfid}{$groupings[0]} }) {
                 # Values-Magic doesn't matter on create; no previous values are being removed
                 # Category is irrelevant for the actual value
                 next if $arg eq "Values-Magic" or $arg eq "Category";
diff --git a/share/html/Elements/EditCustomField b/share/html/Elements/EditCustomField
index 02f9d10..f3b992e 100644
--- a/share/html/Elements/EditCustomField
+++ b/share/html/Elements/EditCustomField
@@ -56,8 +56,11 @@ unless ( $Type ) {
 
 my $Values;
 if ( $Object ) {
+    $Grouping =~ s/\W//g if $Grouping;
     $NamePrefix ||= join '-',
-        'Object', ref($Object), ($Object->Id || ''), 'CustomField', '';
+        'Object', ref($Object), ($Object->Id || ''),
+        'CustomField' . ($Grouping ? ":$Grouping" : ""),
+        '';
 
     if ( $Object->Id ) {
         $Values = $Object->CustomFieldValues( $CustomField->id );
@@ -108,6 +111,7 @@ return $m->comp(
 );
 </%INIT>
 <%ARGS>
+$Grouping    => undef
 $Object      => undef
 $CustomField => undef
 $NamePrefix  => undef
diff --git a/share/html/Elements/ValidateCustomFields b/share/html/Elements/ValidateCustomFields
index e725055..ca2be2a 100644
--- a/share/html/Elements/ValidateCustomFields
+++ b/share/html/Elements/ValidateCustomFields
@@ -52,7 +52,9 @@ $CustomFields->GotoFirstItem;
 my $CFArgs = _ParseObjectCustomFieldArgs( $ARGSRef )->{ref($Object)}{$Object->Id || 0} || {};
 
 while ( my $CF = $CustomFields->Next ) {
-    my $submitted = $CFArgs->{$CF->Id} || {};
+    my $submitted = $CFArgs->{$CF->Id};
+    # Pick the first grouping
+    $submitted = $submitted ? $submitted->{(keys %$submitted)[0]} : {};
 
     # We only validate Single Combos -- multis can never be user input
     next if $submitted->{"Values-Magic"} and exists $submitted->{"Values"}
diff --git a/t/web/cf_groups.t b/t/web/cf_groups.t
index dfbbb7e..4fed5b6 100644
--- a/t/web/cf_groups.t
+++ b/t/web/cf_groups.t
@@ -15,7 +15,8 @@ RT->Config->Set( 'CustomFieldGroupings',
 
 my %CF;
 
-foreach my $name ( map { @$_ } values %{ RT->Config->Get('CustomFieldGroupings')->{'RT::Ticket'} } ) {
+while ( my ($grouping, $cfs) = each %{ RT->Config->Get('CustomFieldGroupings')->{'RT::Ticket'} } ) {
+    my $name = $cfs->[0];
     my $cf = RT::CustomField->new( RT->SystemUser );
     my ($id, $msg) = $cf->Create(
         Name => $name,
@@ -25,7 +26,8 @@ foreach my $name ( map { @$_ } values %{ RT->Config->Get('CustomFieldGroupings')
         Pattern => qr{^(?!bad value).*$},
     );
     ok $id, "custom field '$name' correctly created";
-    $CF{$name} = $cf;
+    $grouping =~ s/\W//g;
+    $CF{$name} = "$grouping-" . $cf->Id;
 }
 
 my $queue = RT::Test->load_or_create_queue( Name => 'General' );
@@ -37,31 +39,31 @@ ok $m->login, 'logged in as root';
     note "testing Create";
     $m->goto_create_ticket($queue);
 
-    my $prefix = 'Object-RT::Ticket--CustomField-';
+    my $prefix = 'Object-RT::Ticket--CustomField:';
     my $dom = $m->dom;
     $m->form_name('TicketCreate');
 
-    my $input_name = $prefix . $CF{'TestBasics'}->id .'-Value';
+    my $input_name = $prefix . $CF{'TestBasics'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.ticket-info-basics input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestBasicsValue' );
 
-    $input_name = $prefix . $CF{'TestPeople'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestPeople'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{#ticket-create-message input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestPeopleValue' );
 
-    $input_name = $prefix . $CF{'TestDates'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestDates'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.ticket-info-dates input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestDatesValue' );
 
-    $input_name = $prefix . $CF{'TestLinks'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestLinks'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.ticket-info-links input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestLinksValue' );
 
-    $input_name = $prefix . $CF{'TestMore'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestMore'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.ticket-info-cfs input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestMoreValue' );
@@ -74,26 +76,28 @@ ok $m->login, 'logged in as root';
     $dom = $m->dom;
 
     foreach my $name ( qw(Basics People Dates Links) ) {
-        my $row_id = 'CF-'. $CF{"Test$name"}->id .'-ShowRow';
+        my $row_id = 'CF-'. $CF{"Test$name"} .'-ShowRow';
+        $row_id =~ s/^CF-(\w+)-/CF-/;
         is $dom->find(qq{#$row_id})->size, 1, "CF on the page";
         is $dom->at(qq{#$row_id})->all_text, "Test$name: Test${name}Value", "value is set";
         ok $dom->at(qq{.ticket-info-\L$name\E #$row_id}), "CF is in the right place";
     }
     {
-        my $row_id = 'CF-'. $CF{"TestMore"}->id .'-ShowRow';
+        my $row_id = 'CF-'. $CF{"TestMore"} .'-ShowRow';
+        $row_id =~ s/^CF-(\w+)-/CF-/;
         is $dom->find(qq{#$row_id})->size, 1, "CF on the page";
         is $dom->at(qq{#$row_id})->all_text, "TestMore: TestMoreValue", "value is set";
         ok $dom->at(qq{.ticket-info-cfs #$row_id}), "CF is in the right place";
     }
 
-    $prefix = 'Object-RT::Ticket-'. $id .'-CustomField-';
+    $prefix = 'Object-RT::Ticket-'. $id .'-CustomField:';
 
     note "testing Basics/People/Dates/Links pages";
     { # Basics
         $m->follow_link_ok({id => 'page-basics'}, 'Ticket -> Basics');
         is $m->dom->find(qq{input[name^="$prefix"][name\$="-Value"]})->size, 2,
             "only one CF input on the page";
-        my $input_name = $prefix . $CF{'TestBasics'}->id .'-Value';
+        my $input_name = $prefix . $CF{'TestBasics'} .'-Value';
         ok $m->dom->at(qq{.ticket-info-basics input[name="$input_name"]}),
             "CF is in the right place";
         $m->submit_form_ok({
@@ -110,7 +114,7 @@ ok $m->login, 'logged in as root';
     }
     { # Custom group 'More'
         $m->follow_link_ok({id => 'page-basics'}, 'Ticket -> Basics');
-        my $input_name = $prefix . $CF{'TestMore'}->id .'-Value';
+        my $input_name = $prefix . $CF{'TestMore'} .'-Value';
         ok $m->dom->at(qq{.ticket-info-cfs input[name="$input_name"]}),
             "CF is in the right place";
         $m->submit_form_ok({
@@ -130,7 +134,7 @@ ok $m->login, 'logged in as root';
         $m->follow_link_ok({id => "page-\L$name"}, "Ticket's $name page");
         is $m->dom->find(qq{input[name^="$prefix"][name\$="-Value"]})->size, 1,
             "only one CF input on the page";
-        my $input_name = $prefix . $CF{"Test$name"}->id .'-Value';
+        my $input_name = $prefix . $CF{"Test$name"} .'-Value';
         $m->submit_form_ok({
             with_fields => { $input_name => "Test${name}Changed" },
             button      => 'SubmitTicket',
@@ -150,7 +154,7 @@ ok $m->login, 'logged in as root';
     $m->form_name("TicketModifyAll");
 
     foreach my $name ( qw(Basics People Dates Links More) ) {
-        my $input_name = $prefix . $CF{"Test$name"}->id .'-Value';
+        my $input_name = $prefix . $CF{"Test$name"} .'-Value';
         is $dom->find(qq{input[name="$input_name"]})->size, 1,
             "only one CF input on the page";
         $m->field( $input_name, "Test${name}Again" );
diff --git a/t/web/cf_groups_users.t b/t/web/cf_groups_users.t
index 3aeb096..5cd6590 100644
--- a/t/web/cf_groups_users.t
+++ b/t/web/cf_groups_users.t
@@ -15,7 +15,8 @@ RT->Config->Set( 'CustomFieldGroupings',
 
 my %CF;
 
-foreach my $name ( map { @$_ } values %{ RT->Config->Get('CustomFieldGroupings')->{'RT::User'} } ) {
+while (my ($group,$cfs) = each %{ RT->Config->Get('CustomFieldGroupings')->{'RT::User'} } ) {
+    my $name = $cfs->[0];
     my $cf = RT::CustomField->new( RT->SystemUser );
     my ($id, $msg) = $cf->Create(
         Name => $name,
@@ -29,7 +30,8 @@ foreach my $name ( map { @$_ } values %{ RT->Config->Get('CustomFieldGroupings')
     ($id, $msg) = $cf->AddToObject( RT::User->new( $cf->CurrentUser ) );
     ok $id, "applied custom field" or diag "error: $msg";
 
-    $CF{$name} = $cf;
+    $group =~ s/\W//g;
+    $CF{$name} = "$group-" . $cf->Id;
 }
 
 my ( $baseurl, $m ) = RT::Test->started_ok;
@@ -46,28 +48,28 @@ my $index = 1;
 
     $m->field( 'Name', 'user'. $index++ );
 
-    my $prefix = 'Object-RT::User--CustomField-';
-    my $input_name = $prefix . $CF{'TestIdentity'}->id .'-Value';
+    my $prefix = 'Object-RT::User--CustomField:';
+    my $input_name = $prefix . $CF{'TestIdentity'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.user-info-identity input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestIdentityValue' );
 
-    $input_name = $prefix . $CF{'TestAccessControl'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestAccessControl'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.user-info-access-control input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestAccessControlValue' );
 
-    $input_name = $prefix . $CF{'TestLocation'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestLocation'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.user-info-location input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestLocationValue' );
 
-    $input_name = $prefix . $CF{'TestPhones'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestPhones'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.user-info-phones input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestPhonesValue' );
 
-    $input_name = $prefix . $CF{'TestMore'}->id .'-Value';
+    $input_name = $prefix . $CF{'TestMore'} .'-Value';
     is $dom->find(qq{input[name="$input_name"]})->size, 1, "only one CF input on the page";
     ok $dom->at(qq{.user-info-cfs input[name="$input_name"]}), "CF is in the right place";
     $m->field( $input_name, 'TestMoreValue' );
@@ -87,8 +89,8 @@ my $index = 1;
         foreach my $cf_name ( keys %CF ) {
             is $user->FirstCustomFieldValue($cf_name), "${cf_name}Value",
                 "correct value of $cf_name CF";
-            my $input = 'Object-RT::User-'. $id .'-CustomField-'
-                . $CF{$cf_name}->id .'-Value';
+            my $input = 'Object-RT::User-'. $id .'-CustomField:'
+                . $CF{$cf_name} .'-Value';
             is $m->value($input), "${cf_name}Value",
                 "correct value in UI";
             $m->field( $input, "${cf_name}Changed" );
@@ -106,8 +108,8 @@ my $index = 1;
         foreach my $cf_name ( keys %CF ) {
             is $user->FirstCustomFieldValue($cf_name), "${cf_name}Changed",
                 "correct value of $cf_name CF";
-            my $input = 'Object-RT::User-'. $id .'-CustomField-'
-                . $CF{$cf_name}->id .'-Value';
+            my $input = 'Object-RT::User-'. $id .'-CustomField:'
+                . $CF{$cf_name} .'-Value';
             is $m->value($input), "${cf_name}Changed",
                 "correct value in UI";
         }

commit 32a75a6f5721a8ee85a2cb9de9169098a3316fc9
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri Nov 16 22:34:17 2012 -0500

    Synchronize CF values in the browser using JavaScript
    
    If a CF appears more than once on a page, synchronize its values such
    that the same value is submitted for all occurances.

diff --git a/share/html/NoAuth/js/late.js b/share/html/NoAuth/js/late.js
index 7a9688f..57d504c 100644
--- a/share/html/NoAuth/js/late.js
+++ b/share/html/NoAuth/js/late.js
@@ -47,3 +47,33 @@
 %# END BPS TAGGED BLOCK }}}
 // Lower the speed limit for hover intent event
 jQuery.event.special.hover.speed = 80; // pixels per second
+
+jQuery(function() {
+    var all_inputs = jQuery("input,textarea,select");
+    var parse_cf = /^Object-([\w:]+)-(\d*)-CustomField(?::\w+)?-(\d+)-(.*)$/;
+    all_inputs.each(function() {
+        var elem = jQuery(this);
+        var parsed = parse_cf.exec(elem.attr("name"));
+        if (parsed == null)
+            return;
+        if (/-Magic$/.test(parsed[4]))
+            return;
+        var name_filter_regex = new RegExp(
+            "^Object-"+parsed[1]+"-"+parsed[2]+
+             "-CustomField(?::\\w+)?-"+parsed[3]+"-"+parsed[4]+"$"
+        );
+        var update_elems = all_inputs.filter(function () {
+            return name_filter_regex.test(jQuery(this).attr("name"));
+        }).not(elem);
+        elem.change( function() {
+            var curval = elem.val();
+            if ((elem.attr("type") == "checkbox") || (elem.attr("type") == "radio")) {
+                curval = [ ];
+                jQuery('[name="'+elem.attr("name")+'"]:checked').each( function() {
+                    curval.push( jQuery(this).val() );
+                });
+            }
+            update_elems.val(curval);
+        } );
+    });
+});

commit f19d36c0a18df993b5cc7f34c81397db3bcd5bab
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri Nov 16 22:35:52 2012 -0500

    Live-synchronize textareas and text entry boxes as you type

diff --git a/share/html/NoAuth/js/late.js b/share/html/NoAuth/js/late.js
index 57d504c..2e846b2 100644
--- a/share/html/NoAuth/js/late.js
+++ b/share/html/NoAuth/js/late.js
@@ -65,7 +65,7 @@ jQuery(function() {
         var update_elems = all_inputs.filter(function () {
             return name_filter_regex.test(jQuery(this).attr("name"));
         }).not(elem);
-        elem.change( function() {
+        var trigger_func = function() {
             var curval = elem.val();
             if ((elem.attr("type") == "checkbox") || (elem.attr("type") == "radio")) {
                 curval = [ ];
@@ -74,6 +74,10 @@ jQuery(function() {
                 });
             }
             update_elems.val(curval);
-        } );
+        };
+        if ((elem.attr("type") == "text") || (elem.attr("tagName") == "TEXTAREA"))
+            elem.keyup( trigger_func );
+        else
+            elem.change( trigger_func );
     });
 });

commit 581d05e450d94728ccbb359a62d64fe953518f45
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri Nov 16 22:36:14 2012 -0500

    Warn if the browser submits inconsistent data for a CF rendered twice

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 48b368c..a5ea10e 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2611,6 +2611,16 @@ sub ProcessObjectCustomFieldUpdates {
                     next;
                 }
                 my @groupings = sort keys %{ $custom_fields_to_mod{$class}{$id}{$cf} };
+                if (@groupings > 1) {
+                    # Check for consistency, in case of JS fail
+                    for my $key (qw/AddValue Value Values DeleteValues DeleteValueIds/) {
+                        warn "CF $cf submitted with multiple differing $key"
+                            if grep {($custom_fields_to_mod{$class}{$id}{$cf}{$_}{$key} || '')
+                                 ne  ($custom_fields_to_mod{$class}{$id}{$cf}{$groupings[0]}{$key} || '')}
+                                @groupings;
+                    }
+                    # We'll just be picking the 1st grouping in the hash, alphabetically
+                }
                 push @results,
                     _ProcessObjectCustomFieldUpdates(
                     # XXX FIXME: Prefix is not quite right, as $id almost
@@ -2817,6 +2827,17 @@ sub ProcessObjectCustomFieldUpdatesForCreate {
             }
 
             my @groupings = sort keys %{ $custom_fields{$class}{0}{$cfid} };
+            if (@groupings > 1) {
+                # Check for consistency, in case of JS fail
+                for my $key (qw/AddValue Value Values DeleteValues DeleteValueIds/) {
+                    warn "CF $cfid submitted with multiple differing $key"
+                        if grep {($custom_fields{$class}{0}{$cfid}{$_}{$key} || '')
+                             ne  ($custom_fields{$class}{0}{$cfid}{$groupings[0]}{$key} || '')}
+                            @groupings;
+                }
+                # We'll just be picking the 1st grouping in the hash, alphabetically
+            }
+
             my @values;
             while (my ($arg, $value) = each %{ $custom_fields{$class}{0}{$cfid}{$groupings[0]} }) {
                 # Values-Magic doesn't matter on create; no previous values are being removed

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


More information about the Rt-commit mailing list