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

r/a=LpSolit

6 files changed, 20 insertions, 15 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index f47ed81c9..8c4045c13 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1686,7 +1686,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};
@@ -1695,13 +1694,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
@@ -3419,9 +3419,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 b8321cbf9..4491b48cf 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -352,6 +352,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'});
@@ -531,7 +533,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 8d6fc5df2..2dcbe9de9 100644
--- a/Bugzilla/WebService/Product.pm
+++ b/Bugzilla/WebService/Product.pm
@@ -236,8 +236,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 =>
@@ -456,7 +457,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 1105e2517..3b0bc093b 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -828,8 +828,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 d93ed537e..f151c3dd4 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 d1c9ac4d0..2a5d2399b 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>