Bug 440072 - Cloned certifications not assigned correct reviewers and TAM
Cloned certifications not assigned correct reviewers and TAM
Product: Red Hat Hardware Certification Program
Classification: Red Hat
Component: Hardware Catalog (Show other bugs)
All Linux
high Severity high
: ---
: ---
Assigned To: XINSUN
Yu Shao
Depends On:
  Show dependency treegraph
Reported: 2008-04-01 12:38 EDT by Gary Case
Modified: 2008-10-28 01:51 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-10-28 01:51:23 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

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

  None (edit)
Description Gary Case 2008-04-01 12:38:04 EDT
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 04:22:23 EDT
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@redhat.com 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 04:40:10 EDT
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@landfill.bugzilla.org is helping us to assign TAM and Reviewer for a new cert. but bugbot@landfill.bugzilla.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@redhat.com).

Pls review this patch. Thanks a lot.

Best Regards!
Comment 3 Rob Landry 2008-08-07 09:46:50 EDT
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 11:39:00 EDT
Patch Test Link is:
We can see this patch in action. :)

Best Regards!
Comment 5 Rob Landry 2008-08-07 15:36:59 EDT
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-07 22:33:18 EDT
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:

Best Regards!
Comment 7 Rob Landry 2008-08-08 11:25:28 EDT
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-10 22:07:36 EDT
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, :)

Comment 9 Rob Landry 2008-08-11 12:02:50 EDT

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.