[Rt-commit] rt branch, 4.2/custom-field-arg-parsing, created. rt-4.0.6-494-gc06c35b

Thomas Sibley trs at bestpractical.com
Fri Aug 24 15:47:09 EDT 2012


The branch, 4.2/custom-field-arg-parsing has been created
        at  c06c35b5188ccfbae834cf7945c86a995e83be5d (commit)

- Log -----------------------------------------------------------------
commit f9fc38e920d79c559101c946b36b14174181d903
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 21 22:54:04 2012 -0700

    Parsing OCFV form args into a data structure is useful in future functions
    
    The end goal is less cases of custom matching %ARGS to do OCFV
    processing.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index d036518..a3a0b0b 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2520,15 +2520,7 @@ sub ProcessObjectCustomFieldUpdates {
     my @results;
 
     # Build up a list of objects that we want to work with
-    my %custom_fields_to_mod;
-    foreach my $arg ( keys %$ARGSRef ) {
-
-        # format: Object-<object class>-<object id>-CustomField-<CF id>-<commands>
-        next unless $arg =~ /^Object-([\w:]+)-(\d*)-CustomField-(\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};
-    }
+    my %custom_fields_to_mod = _ParseObjectCustomFieldArgs($ARGSRef);
 
     # For each of those objects
     foreach my $class ( keys %custom_fields_to_mod ) {
@@ -2564,6 +2556,22 @@ sub ProcessObjectCustomFieldUpdates {
     return @results;
 }
 
+sub _ParseObjectCustomFieldArgs {
+    my $ARGSRef = shift || {};
+    my %custom_fields_to_mod;
+
+    foreach my $arg ( keys %$ARGSRef ) {
+
+        # format: Object-<object class>-<object id>-CustomField-<CF id>-<commands>
+        next unless $arg =~ /^Object-([\w:]+)-(\d*)-CustomField-(\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};
+    }
+
+    return wantarray ? %custom_fields_to_mod : \%custom_fields_to_mod;
+}
+
 sub _ProcessObjectCustomFieldUpdates {
     my %args    = @_;
     my $cf      = $args{'CustomField'};

commit 6a9d0bff0f6aaafc222e324ed2cc50d57a4e4ea0
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 21 22:59:20 2012 -0700

    Split OCFV normalization into a separate function for general use
    
    Ideally this would live in RT::CustomField, you'd call it on an object,
    and life would be simpler.  Due to file uploads, however, the
    normalization still has a foot planted in the web interface.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index a3a0b0b..5bf15e6 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2545,6 +2545,9 @@ sub ProcessObjectCustomFieldUpdates {
                 }
                 push @results,
                     _ProcessObjectCustomFieldUpdates(
+                    # XXX FIXME: Prefix is not quite right, as $id almost
+                    # certainly started as blank for new objects and is now 0.
+                    # Only Image/Binary CFs on new objects should be affected.
                     Prefix      => "Object-$class-$id-CustomField-$cf-",
                     Object      => $Object,
                     CustomField => $CustomFieldObj,
@@ -2607,22 +2610,14 @@ sub _ProcessObjectCustomFieldUpdates {
             $args{'ARGS'}->{'Values'} = undef;
         }
 
-        my @values = ();
-        if ( ref $args{'ARGS'}->{$arg} eq 'ARRAY' ) {
-            @values = @{ $args{'ARGS'}->{$arg} };
-        } elsif ( $cf_type =~ /text/i ) {    # Both Text and Wikitext
-            @values = ( $args{'ARGS'}->{$arg} );
-        } else {
-            @values = split /\r*\n/, $args{'ARGS'}->{$arg}
-                if defined $args{'ARGS'}->{$arg};
-        }
-        @values = grep length, map {
-            s/\r+\n/\n/g;
-            s/^\s+//;
-            s/\s+$//;
-            $_;
-            }
-            grep defined, @values;
+        my @values = _NormalizeObjectCustomFieldValue(
+            CustomField => $cf,
+            Param       => $args{'Prefix'} . $arg,
+            Value       => $args{'ARGS'}->{$arg}
+        );
+
+        # "Empty" values still don't mean anything for Image and Binary fields
+        next if $cf_type =~ /^(?:Image|Binary)$/ and not @values;
 
         if ( $arg eq 'AddValue' || $arg eq 'Value' ) {
             foreach my $value (@values) {
@@ -2633,8 +2628,7 @@ sub _ProcessObjectCustomFieldUpdates {
                 push( @results, $msg );
             }
         } elsif ( $arg eq 'Upload' ) {
-            my $value_hash = _UploadedFile( $args{'Prefix'} . $arg ) or next;
-            my ( $val, $msg ) = $args{'Object'}->AddCustomFieldValue( %$value_hash, Field => $cf, );
+            my ( $val, $msg ) = $args{'Object'}->AddCustomFieldValue( %{$values[0]}, Field => $cf, );
             push( @results, $msg );
         } elsif ( $arg eq 'DeleteValues' ) {
             foreach my $value (@values) {
@@ -2719,6 +2713,33 @@ sub _ProcessObjectCustomFieldUpdates {
     return @results;
 }
 
+sub _NormalizeObjectCustomFieldValue {
+    my %args    = (@_);
+    my $cf_type = $args{CustomField}->Type;
+    my @values  = ();
+
+    if ( ref $args{'Value'} eq 'ARRAY' ) {
+        @values = @{ $args{'Value'} };
+    } elsif ( $cf_type =~ /text/i ) {    # Both Text and Wikitext
+        @values = ( $args{'Value'} );
+    } else {
+        @values = split /\r*\n/, $args{'Value'}
+            if defined $args{'Value'};
+    }
+    @values = grep length, map {
+        s/\r+\n/\n/g;
+        s/^\s+//;
+        s/\s+$//;
+        $_;
+        }
+        grep defined, @values;
+
+    if ($cf_type eq 'Image' or $cf_type eq 'Binary') {
+        @values = _UploadedFile( $args{'Param'} ) || ();
+    }
+
+    return @values;
+}
 
 =head2 ProcessTicketWatchers ( TicketObj => $Ticket, ARGSRef => \%ARGS );
 

commit be3529af672e52077294c584701038e0cd71e34f
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Aug 21 23:00:49 2012 -0700

    Consolidate parsing OCFV form args into Create parameters for new records
    
    New records often require setting custom field values via ->Create
    parameters.  ProcessObjectCustomFieldUpdates() doesn't cut it for this
    case since it makes the CF updates directly.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 5bf15e6..d14ab3b 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1671,52 +1671,10 @@ sub CreateTicket {
             : ( $ARGS{'AttachTickets'} ) );
     }
 
-    foreach my $arg ( keys %ARGS ) {
-        next if $arg =~ /-(?:Magic|Category)$/;
-
-        if ( $arg =~ /^Object-RT::Transaction--CustomField-/ ) {
-            $create_args{$arg} = $ARGS{$arg};
-        }
-
-        # Object-RT::Ticket--CustomField-3-Values
-        elsif ( $arg =~ /^Object-RT::Ticket--CustomField-(\d+)/ ) {
-            my $cfid = $1;
-
-            my $cf = RT::CustomField->new( $session{'CurrentUser'} );
-            $cf->SetContextObject( $Queue );
-            $cf->Load($cfid);
-            unless ( $cf->id ) {
-                $RT::Logger->error( "Couldn't load custom field #" . $cfid );
-                next;
-            }
-
-            if ( $arg =~ /-Upload$/ ) {
-                $create_args{"CustomField-$cfid"} = _UploadedFile($arg);
-                next;
-            }
-
-            my $type = $cf->Type;
-
-            my @values = ();
-            if ( ref $ARGS{$arg} eq 'ARRAY' ) {
-                @values = @{ $ARGS{$arg} };
-            } elsif ( $type =~ /text/i ) {
-                @values = ( $ARGS{$arg} );
-            } else {
-                no warnings 'uninitialized';
-                @values = split /\r*\n/, $ARGS{$arg};
-            }
-            @values = grep length, map {
-                s/\r+\n/\n/g;
-                s/^\s+//;
-                s/\s+$//;
-                $_;
-                }
-                grep defined, @values;
-
-            $create_args{"CustomField-$cfid"} = \@values;
-        }
-    }
+    my %cfs = ProcessObjectCustomFieldUpdatesForCreate(
+        ARGSRef         => \%ARGS,
+        ContextObject   => $Queue,
+    );
 
     # turn new link lists into arrays, and pass in the proper arguments
     my %map = (
@@ -1733,7 +1691,7 @@ sub CreateTicket {
 
     }
 
-    my ( $id, $Trans, $ErrMsg ) = $Ticket->Create(%create_args);
+    my ( $id, $Trans, $ErrMsg ) = $Ticket->Create(%create_args, %cfs);
     unless ($id) {
         Abort($ErrMsg);
     }
@@ -2713,6 +2671,47 @@ sub _ProcessObjectCustomFieldUpdates {
     return @results;
 }
 
+sub ProcessObjectCustomFieldUpdatesForCreate {
+    my %args = (
+        ARGSRef         => {},
+        ContextObject   => undef,
+        @_
+    );
+    my %parsed;
+    my %custom_fields = _ParseObjectCustomFieldArgs( $args{'ARGSRef'} );
+
+    for my $class (keys %custom_fields) {
+        # we're only interested in new objects, so only look at $id == 0
+        for my $cfid (keys %{ $custom_fields{$class}{0} || {} }) {
+            my $cf = RT::CustomField->new( $session{'CurrentUser'} );
+            $cf->SetContextObject( $args{'ContextObject'} ); # XXX TODO: ValidateContextObject?
+            $cf->LoadById($cfid);
+
+            unless ($cf->id) {
+                RT->Logger->warning("Couldn't load custom field #$cfid");
+                next;
+            }
+
+            my @values;
+            while (my ($arg, $value) = each %{ $custom_fields{$class}{0}{$cfid} }) {
+                # 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";
+
+                push @values, _NormalizeObjectCustomFieldValue(
+                    CustomField => $cf,
+                    Param       => "Object-$class--CustomField-$cfid-$arg",
+                    Value       => $value,
+                );
+            }
+
+            $parsed{"CustomField-$cfid"} = \@values if @values;
+        }
+    }
+
+    return wantarray ? %parsed : \%parsed;
+}
+
 sub _NormalizeObjectCustomFieldValue {
     my %args    = (@_);
     my $cf_type = $args{CustomField}->Type;
diff --git a/share/html/Articles/Article/Edit.html b/share/html/Articles/Article/Edit.html
index d14c330..9ca34e9 100644
--- a/share/html/Articles/Article/Edit.html
+++ b/share/html/Articles/Article/Edit.html
@@ -149,48 +149,10 @@ elsif ( $id eq 'new' ) {
           split( /\s+/, $ARGS{'new-RefersTo'} );
     }
 
-
-    foreach my $arg (keys %ARGS) {
-        next if $arg =~ /-(?:Magic|Category)$/;
-        # Object-RT::Article--CustomField-3-Values
-        if ( $arg =~ /^Object-RT::Article--CustomField-(\d+)(.*?)$/ ) {
-            my $cfid = $1; 
-        
-            my $cf = RT::CustomField->new( $session{'CurrentUser'} );
-            $cf->SetContextObject( $ArticleObj );
-            $cf->Load( $cfid );
-            unless ( $cf->id ) {
-                $RT::Logger->error( "Couldn't load custom field #". $cfid );
-                next;
-            }
-
-            if ( $arg =~ /-Upload$/ ) {
-                $create_args{"CustomField-$cfid"} = _UploadedFile( $arg );
-                next;
-            }
-            
-            my $type = $cf->Type;
-        
-            my @values = ();
-            if ( ref $ARGS{ $arg } eq 'ARRAY' ) {
-                @values = @{ $ARGS{ $arg } };
-            } elsif ( $type =~ /text/i ) {
-                @values = ($ARGS{ $arg });
-            } else {
-                @values = split /\r*\n/, $ARGS{ $arg } || '';
-            }
-            @values = grep $_ ne '',
-                map {
-                    s/\r+\n/\n/g;
-                    s/^\s+//;
-                    s/\s+$//;
-                    $_;
-                }
-                grep defined, @values;
-
-            $create_args{"CustomField-$cfid"} = \@values;
-        }
-    }
+    my %cfs = ProcessObjectCustomFieldUpdatesForCreate(
+        ARGSRef         => \%ARGS,
+        ContextObject   => $ArticleObj,
+    );
 
     my $msg;
     ( $id, $msg ) = $ArticleObj->Create(
@@ -198,7 +160,8 @@ elsif ( $id eq 'new' ) {
         Name    => $ARGS{'Name'},
         Class   => $ARGS{'Class'},
         Topics  => $ARGS{'Topics'},
-        %create_args
+        %create_args,
+        %cfs
     );
     push( @results, $msg );
     if ($id) {

commit a6becf88cfc8899598705c4e1ca9263f0d2db93b
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Aug 22 17:48:37 2012 -0700

    Validate the ContextObject to assure correct behaviour

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index d14ab3b..e23a7a7 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2677,6 +2677,7 @@ sub ProcessObjectCustomFieldUpdatesForCreate {
         ContextObject   => undef,
         @_
     );
+    my $context = $args{'ContextObject'};
     my %parsed;
     my %custom_fields = _ParseObjectCustomFieldArgs( $args{'ARGSRef'} );
 
@@ -2684,7 +2685,19 @@ sub ProcessObjectCustomFieldUpdatesForCreate {
         # we're only interested in new objects, so only look at $id == 0
         for my $cfid (keys %{ $custom_fields{$class}{0} || {} }) {
             my $cf = RT::CustomField->new( $session{'CurrentUser'} );
-            $cf->SetContextObject( $args{'ContextObject'} ); # XXX TODO: ValidateContextObject?
+            if ($context) {
+                my $system_cf = RT::CustomField->new( RT->SystemUser );
+                $system_cf->LoadById($cfid);
+                if ($system_cf->ValidateContextObject($context)) {
+                    $cf->SetContextObject($context);
+                } else {
+                    RT->Logger->error(
+                        sprintf "Invalid context object %s (%d) for CF %d; skipping CF",
+                                ref $context, $context->id, $system_cf->id
+                    );
+                    next;
+                }
+            }
             $cf->LoadById($cfid);
 
             unless ($cf->id) {

commit e8e84b1a280a5d43a8c9ae4ff03d77bfe53749bd
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Aug 22 17:49:58 2012 -0700

    Use a valid ContextObject during article creation
    
    We don't have an Article object yet, so use the Class.
    
    Articles should always use Classes as context objects anyway since
    Articles themselves don't define any ACLEquivalenceObjects.

diff --git a/share/html/Articles/Article/Edit.html b/share/html/Articles/Article/Edit.html
index 9ca34e9..6b5c82c 100644
--- a/share/html/Articles/Article/Edit.html
+++ b/share/html/Articles/Article/Edit.html
@@ -115,6 +115,12 @@ my $title;
 my $Entries    = {};
 my $ArticleObj = RT::Article->new( $session{'CurrentUser'} );
 my $ClassObj   = RT::Class->new( $session{'CurrentUser'} );
+
+if ($Class) {
+    $ClassObj->Load($Class);
+    Abort(loc("'[_1]' isn't a valid class", $Class)) unless $ClassObj->Id;
+}
+
 my %create_args;
 my %CFContent;
 my $EditClass = 1;
@@ -129,11 +135,6 @@ if ( !$id ) {
         }
     }
 
-    $ClassObj->Load($Class);
-    unless ( $ClassObj->Id ) {
-        $m->comp( "/Elements/Error",
-            Why => loc( "'[_1]' isn't a valid class identifier", $Class ) );
-    }
     $EditClass = 0;
     $id        = 'new';
 }
@@ -151,7 +152,7 @@ elsif ( $id eq 'new' ) {
 
     my %cfs = ProcessObjectCustomFieldUpdatesForCreate(
         ARGSRef         => \%ARGS,
-        ContextObject   => $ArticleObj,
+        ContextObject   => $ClassObj,
     );
 
     my $msg;
@@ -182,12 +183,7 @@ elsif ( $id eq 'new' ) {
             );
         }
     }
-   if (!$id) {
-        $ClassObj->Load($Class);
-        unless ( $ClassObj->Id ) {
-            $m->comp( "/Elements/Error",
-                Why => loc( "'[_1]' isn't a valid class identifier", $Class ) );
-        }
+    else {
         $ArticleObj = RT::Article->new( $session{'CurrentUser'} );
         $id        = 'new';
         $EditClass = 0;

commit c06c35b5188ccfbae834cf7945c86a995e83be5d
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Fri Aug 24 11:19:07 2012 -0700

    Return ACLEquivalenceObjects rather than manually specifying EquivObjects for HasRight
    
    This gives us the ability to use a loaded RT::Article as a CF context
    object and makes the rights checking more standard.
    
    Remove RT::System objects since those are automatically added by RT.
    The duplicates were from a previous (over-zealous) search and replace on
    RT::FM::System → RT::System.

diff --git a/lib/RT/Article.pm b/lib/RT/Article.pm
index 24b952a..5172345 100644
--- a/lib/RT/Article.pm
+++ b/lib/RT/Article.pm
@@ -537,7 +537,6 @@ sub CurrentUserHasRight {
         $self->CurrentUser->HasRight(
             Right        => $right,
             Object       => $self,
-            EquivObjects => [ $RT::System, $RT::System, $self->ClassObj ]
         )
     );
 
@@ -620,6 +619,11 @@ sub _LookupId {
 
 }
 
+sub ACLEquivalenceObjects {
+    my $self = shift;
+    return $self->ClassObj;
+}
+
 =head2 LoadByInclude Field Value
 
 Takes the name of a form field from "Include Article"
diff --git a/lib/RT/Class.pm b/lib/RT/Class.pm
index 81b389e..2bed9c3 100644
--- a/lib/RT/Class.pm
+++ b/lib/RT/Class.pm
@@ -262,8 +262,7 @@ sub CurrentUserHasRight {
     return (
         $self->CurrentUser->HasRight(
             Right        => $right,
-            Object       => ( $self->Id ? $self : $RT::System ),
-            EquivObjects => [ $RT::System, $RT::System ]
+            Object       => ( $self->Id ? $self : RT->System ),
         )
     );
 

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


More information about the Rt-commit mailing list