Bug 440072 - Cloned certifications not assigned correct reviewers and TAM
Summary: Cloned certifications not assigned correct reviewers and TAM
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Hardware Certification Program
Classification: Retired
Component: Hardware Catalog
Version: 5
Hardware: All
OS: Linux
high
high
Target Milestone: ---
: ---
Assignee: XINSUN
QA Contact: Yu Shao
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-04-01 16:38 UTC by Gary Case
Modified: 2008-10-28 05:51 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-28 05:51:23 UTC
Embargoed:


Attachments (Terms of Use)
Patch Fix: (963 bytes, patch)
2008-08-07 08:40 UTC, XINSUN
no flags Details | Diff
Patch Improve: (2.06 KB, patch)
2008-08-08 02:33 UTC, XINSUN
no flags Details | Diff
Patch Improve: (1.95 KB, patch)
2008-08-11 02:07 UTC, XINSUN
no flags Details | Diff

Description Gary Case 2008-04-01 16:38:04 UTC
Description of problem:
Cloned certifications aren't being assigned the correct TAM and reviewer. 
You can see what I'm talking about in this example:

Parent - https://hardware.redhat.com/show.cgi?id=366811

Clone - https://hardware.redhat.com/show.cgi?id=439824

I was the TAM for Egenera when the parent cert was created. Now, Chris Tatman is
the Egenera TAM. The clone code is setting me as the reviewer and the reviewers
as the TAM, so I think there are two problems here.

Version-Release number of selected component (if applicable):
Current version of BZ (2.18-rh)

How reproducible:
Create a cloned cert based on an existing cert that has an "old" TAM assigned to
it. For the example posted above, I was the Egenera TAM at the time of the
original cert's creation, but since that period the ownership has passed to
Chris Tatman. 

Steps to Reproduce:
1.Create a clone of a cert where the TAM entry is no longer valid (a new TAM has
taken over the relationship since the parent cert was opened)
  
Actual results:
Parent cert's TAM set to reviewer of clone, reviewer alias set to TAM on clone.

Expected results:
Current TAM data from from TAM DB used to determine TAM and that data put into
TAM field (QA Contact). Reviewer alias assigned to reviewer field (Assigned To).
That's the way it works for new cert entries.

Additional info:

Comment 1 XINSUN 2008-08-07 08:22:23 UTC
Reason caused this bug:
When clone cert:

In the template file "clone.html.tmpl":

**********************************************************************************
<input type="hidden" name="qa_contact" value="[% bug.qa_contact.email FILTER html %]">
      <input type="hidden" name="assigned_to" value="[% bug.qa_contact.email FILTER html %]">
**********************************************************************************


In the post.cgi : 

1.Clone cert's qa_contact comes from the initialqacontact in the components.(hwcert-reviwers is the initialqacontact) 
**********************************************************************************
if (Param("useqacontact")) {
 
    SendSQL("SELECT initialqacontact FROM components WHERE id = $component_id");
    my $qa_contact = FetchOneColumn();

    if (defined $qa_contact && $qa_contact != 0) {
        $::FORM{'qa_contact'} = $qa_contact;
        push(@bug_fields, "qa_contact");
    }
}
***********************************************************************************


2.Clone cert's assigned_to comes form the $::FORM{'assigned_to'} which is posted by clone.html.tmpl, but in the clone.html.tmpl, the name="assigned_to" value="[% bug.qa_contact.email FILTER html . So this is reason caused this bug.
***********************************************************************************
if (!defined $::FORM{'assigned_to'} || $::FORM{'assigned_to'} eq "") {
    SendSQL("SELECT initialowner FROM components " .
            "WHERE id = $component_id");
    $::FORM{'assigned_to'} = FetchOneColumn();
} else {
    $::FORM{'assigned_to'} = DBNameToIdAndCheck(trim($::FORM{'assigned_to'}));
}
********************************************************************************

Comment 2 XINSUN 2008-08-07 08:40:10 UTC
Created attachment 313669 [details]
Patch Fix:

Correct the <input name = "assigned_to"> 's value from "qa_contact" to "assgined_to". this caused the reviewers' field was filled with Tam's name (alias).

BTW. Seems the bugbot.org is helping us to assign TAM and Reviewer for a new cert. but bugbot.org doesn't handle and assign the Tam/Review to these clone cert. means when clone cert was created, the "qa_contact" field was filled with the initialqa_contact of our component (hwcert-reviewers).

Pls review this patch. Thanks a lot.

Best Regards!
Nicho

Comment 3 Rob Landry 2008-08-07 13:46:50 UTC
Is there a server where this patch can be seen in action?  I would suspect this resolves the issue; bugbot.landfill actually isn't the assigner, bugzilla does have a default which if we don't set it'll use as the ownerships however the auto assigner cron jobs are run from seg. server in GSS which then connects to the GSS TAM db.

Comment 4 XINSUN 2008-08-07 15:39:00 UTC
Patch Test Link is:
http://bugdev.devel.redhat.com/hwcert-xisun2/show.cgi?id=366811
We can see this patch in action. :)

Best Regards!
Nicho

Comment 5 Rob Landry 2008-08-07 19:36:59 UTC
The result seemed to be that the TAM was assigned, we should probably go ahead and set that to the originals TAM and not count on an assigner script, since that's not going to happen because clones are opened as closed.

Comment 6 XINSUN 2008-08-08 02:33:18 UTC
Created attachment 313773 [details]
Patch Improve:

Refer to the assigned part codes:
  If create a cert (left::FROM{'qa_contact'} blank), then set it to initialqa_contact.
  If clone a cert (:FROM{'qa_contact'} was post as an original TAM of parent cert.), then set it to the original TAM.

Test link:
 http://bugdev.devel.redhat.com/hwcert-xisun2/show.cgi?id=366811

Best Regards!
Nicho

Comment 7 Rob Landry 2008-08-08 15:25:28 UTC
Looks like most of the code in the patch is trying to set the default qa_contact value which works today it's the case of when it's set to a value that not working, my guess from teh patch attached is the email to id call would cover the basis or is the additional if structure present for the other values and this is simplay correcting an oversimplified IF?

Comment 8 XINSUN 2008-08-11 02:07:36 UTC
Created attachment 313902 [details]
Patch Improve:

Add a additional IF structure:

if (($cgi->param('dup_id')) #### clone a cert.
{

   $::FORM{'qa_contact'} = DBNameToIdAndCheck(trim($::FORM{'qa_contact'}));

} else {                 #### create a new cert or didn't post FORM{'qa_contact'}

   #fetch the initialqa_contact from db.
   SendSQL("SELECT initialqacontact FROM components WHERE id = $component_id");
   my $qa_contact = FetchOneColumn();

}

Test link http://bugdev.devel.redhat.com/hwcert-xisun2/show.cgi?id=366811

pls review, :)

BestRegards!
Nicho

Comment 9 Rob Landry 2008-08-11 16:02:50 UTC

Technically the "push(@bug_fields, "qa_contact");" could appear only after the conditional closure instead of the last line of each resulting in one less line of code and I think it would be equally as clear to understand.

Patch looks good to me though, please commit to CVS


Note You need to log in before you can comment on or make changes to this bug.