From: Koosha Khajeh Moogahi <koosha.khajeh@gmail.com>
Date: Wed, 19 Sep 2012 19:13:05 +0000 (+0200)
Subject: Bug 445861: $component->default_qa_contact should not return the default user object... 
X-Git-Tag: bugzilla-4.4rc1~65
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bdd3053df4eb4097c52ac550d71c96ef4a2ba25f;p=thirdparty%2Fbugzilla.git

Bug 445861: $component->default_qa_contact should not return the default user object when no default QA contact is defined
r/a=LpSolit
---

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 2e9ed52e00..0dd9def14d 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1685,7 +1685,6 @@ sub _check_qa_contact {
     $qa_contact = trim($qa_contact) if !ref $qa_contact;
     my $component = blessed($invocant) ? $invocant->component_obj
                                        : $params->{component};
-    my $id;
     if (!ref $invocant) {
         # Bugs get no QA Contact on creation if useqacontact is off.
         return undef if !Bugzilla->params->{useqacontact};
@@ -1694,13 +1693,14 @@ sub _check_qa_contact {
         if (!Bugzilla->user->in_group('editbugs', $component->product_id)
             || !$qa_contact)
         {
-            $id = $component->default_qa_contact->id;
+            return $component->default_qa_contact ? $component->default_qa_contact->id : undef;
         }
     }
-    
+
     # If a QA Contact was specified or if we're updating, check
     # the QA Contact for validity.
-    if (!defined $id && $qa_contact) {
+    my $id;
+    if ($qa_contact) {
         $qa_contact = Bugzilla::User->check($qa_contact) if !ref $qa_contact;
         $id = $qa_contact->id;
         # create() checks this another way, so we don't have to run this
@@ -3418,9 +3418,6 @@ sub qa_contact {
     if (Bugzilla->params->{'useqacontact'} && $self->{'qa_contact'}) {
         $self->{'qa_contact_obj'} = new Bugzilla::User($self->{'qa_contact'});
     } else {
-        # XXX - This is somewhat inconsistent with the assignee/reporter 
-        # methods, which will return an empty User if they get a 0. 
-        # However, we're keeping it this way now, for backwards-compatibility.
         $self->{'qa_contact_obj'} = undef;
     }
     return $self->{'qa_contact_obj'};
diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index f189639468..2c8658564f 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -349,6 +349,8 @@ sub default_assignee {
 sub default_qa_contact {
     my $self = shift;
 
+    return if !$self->{'initialqacontact'};
+
     if (!defined $self->{'default_qa_contact'}) {
         $self->{'default_qa_contact'} =
             new Bugzilla::User($self->{'initialqacontact'});
@@ -528,7 +530,8 @@ Component.pm represents a Product Component object.
 
  Params:      none.
 
- Returns:     A Bugzilla::User object.
+ Returns:     A Bugzilla::User object if the default QA contact is defined for
+              the component. Otherwise, returns undef.
 
 =item C<initial_cc>
 
diff --git a/Bugzilla/WebService/Product.pm b/Bugzilla/WebService/Product.pm
index 18594d7260..298a258ec5 100644
--- a/Bugzilla/WebService/Product.pm
+++ b/Bugzilla/WebService/Product.pm
@@ -234,8 +234,9 @@ sub _component_to_hash {
             $self->type('string' , $component->description),
         default_assigned_to =>
             $self->type('string' , $component->default_assignee->login),
-        default_qa_contact =>
-            $self->type('string' , $component->default_qa_contact->login),
+        default_qa_contact => 
+            $self->type('string' , $component->default_qa_contact ?
+                                   $component->default_qa_contact->login : ''),
         sort_key =>  # sort_key is returned to match Bug.fields
             0,
         is_active =>
@@ -454,7 +455,7 @@ default.
 =item C<default_qa_contact>
 
 C<string> The login name of the user who will be set as the QA Contact for
-new bugs by default.
+new bugs by default. Empty string if the QA contact is not defined.
 
 =item C<sort_key>
 
diff --git a/importxml.pl b/importxml.pl
index f34317d5ae..5b13d30be5 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -827,8 +827,10 @@ sub process_bug {
             push( @values, $qa_contact );
         }
         else {
-            push( @values, $component->default_qa_contact->id || undef );
-            if ($component->default_qa_contact->id){
+            push(@values, $component->default_qa_contact ?
+                          $component->default_qa_contact->id : undef);
+
+            if ($component->default_qa_contact) {
                 $err .= "Setting qa contact to the default for this product.\n";
                 $err .= "   This bug either had no qa contact or an invalid one.\n";
             }
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index d93ed537e0..f151c3dd46 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -204,7 +204,7 @@
         <li>Default assignee updated to '[% comp.default_assignee.login FILTER html %]'</li>
       [% END %]
       [% IF changes.initialqacontact.defined %]
-        [% IF comp.default_qa_contact.id %]
+        [% IF comp.default_qa_contact %]
           <li>Default QA contact updated to '[% comp.default_qa_contact.login FILTER html %]'</li>
         [% ELSE %]
           <li>Default QA contact deleted</li>
diff --git a/template/en/default/reports/components.html.tmpl b/template/en/default/reports/components.html.tmpl
index d1c9ac4d0a..2a5d2399b6 100644
--- a/template/en/default/reports/components.html.tmpl
+++ b/template/en/default/reports/components.html.tmpl
@@ -80,7 +80,9 @@
     </td>
     [% IF Param("useqacontact") %]
       <td class="component_qa_contact">
-        [% INCLUDE global/user.html.tmpl who = comp.default_qa_contact %]
+        [% IF comp.default_qa_contact %]
+          [% INCLUDE global/user.html.tmpl who = comp.default_qa_contact %]
+        [% END %]
       </td>
     [% END %]
   </tr>