[Rt-commit] rt branch, 4.0/cf-repeated-values, created. rt-4.0.20-1-gcdcc492

[? sunnavy]

The branch, 4.0/cf-repeated-values has been created
        at  cdcc4929605d37dfe84d264199b219872573f6c0 (commit)

- Log -----------------------------------------------------------------
commit cdcc4929605d37dfe84d264199b219872573f6c0
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 15 21:49:49 2014 +0800

    avoid adding repeated cf values
    
    now RT::ObjectCustomFieldValues::HasEntry compares LargeContent and also
    canonicalizes values before comparision when possible.
    
    see also #4553
    
    btw, there was a bug that if you change a date cf from "2014-05-15" to
    "today"(which also means "2014-05-15" at that time), you will get a confusing
    message saying "2014-05-15 changed to 2014-05-15". this commit fixes it too.

diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index 486265e..2ab8b08 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -115,10 +115,10 @@ sub LimitToObject {
 }
 
 
-=head2 HasEntry VALUE
+=head2 HasEntry CONTENT LARGE_CONTENT
 
-If this collection has an entry with content that eq VALUE then
-returns the entry, otherwise returns undef.
+If this collection has an entry with content that eq CONTENT and large content
+that eq LARGE_CONTENT then returns the entry, otherwise returns undef.
 
 =cut
 
@@ -126,11 +126,41 @@ returns the entry, otherwise returns undef.
 sub HasEntry {
     my $self = shift;
     my $value = shift;
+    my $large_content = shift;
     return undef unless defined $value && length $value;
 
+    my %canon_value;
     #TODO: this could cache and optimize a fair bit.
     foreach my $item ( @{$self->ItemsArrayRef} ) {
-        return $item if lc $item->Content eq lc $value;
+        my $cf = $item->CustomFieldObj;
+        my $args = $canon_value{ $cf->Type };
+        if ( !$args ) {
+            $args = { Content => $value, LargeContent => $large_content };
+            if ( my $canonicalizer =
+                $cf->can( '_CanonicalizeValue' . $cf->Type ) )
+            {
+                $canonicalizer->( $cf, $args );
+            }
+
+            $canon_value{ $cf->Type } = $args;
+        }
+
+        if ( $cf->Type eq 'Select' ) {
+            # select is case insensitive
+            return $item if lc $item->Content eq lc $args->{Content};
+        }
+        else {
+            if ( $item->Content eq $args->{Content} ) {
+                if ( defined $item->LargeContent ) {
+                    return $item
+                      if defined $args->{LargeContent}
+                      && $item->LargeContent eq $args->{LargeContent};
+                }
+                else {
+                    return $item unless defined $args->{LargeContent};
+                }
+            }
+        }
     }
     return undef;
 }
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index a89c39c..45ec7cf 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1743,31 +1743,14 @@ sub _AddCustomFieldValue {
             $values->RedoSearch if $i; # redo search if have deleted at least one value
         }
 
-        my ( $old_value, $old_content );
-        if ( $old_value = $values->First ) {
-            $old_content = $old_value->Content;
-            $old_content = undef if defined $old_content && !length $old_content;
-
-            my $is_the_same = 1;
-            if ( defined $args{'Value'} ) {
-                $is_the_same = 0 unless defined $old_content
-                    && $old_content eq $args{'Value'};
-            } else {
-                $is_the_same = 0 if defined $old_content;
-            }
-            if ( $is_the_same ) {
-                my $old_content = $old_value->LargeContent;
-                if ( defined $args{'LargeContent'} ) {
-                    $is_the_same = 0 unless defined $old_content
-                        && $old_content eq $args{'LargeContent'};
-                } else {
-                    $is_the_same = 0 if defined $old_content;
-                }
-            }
-
-            return $old_value->id if $is_the_same;
+        if ( my $entry = $values->HasEntry($args{'Value'}, $args{'LargeContent'}) ) {
+            return $entry->id;
         }
 
+        my $old_value = $values->First;
+        my $old_content;
+        $old_content = $old_value->Content if $old_value;
+
         my ( $new_value_id, $value_msg ) = $cf->AddValueForObject(
             Object       => $self,
             Content      => $args{'Value'},
@@ -1834,6 +1817,13 @@ sub _AddCustomFieldValue {
 
     # otherwise, just add a new value and record "new value added"
     else {
+        if ( !$cf->Repeated ) {
+            my $values = $cf->ValuesForObject($self);
+            if ( my $entry = $values->HasEntry($args{'Value'}, $args{'LargeContent'}) ) {
+                return $entry->id;
+            }
+        }
+
         my ($new_value_id, $msg) = $cf->AddValueForObject(
             Object       => $self,
             Content      => $args{'Value'},

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