Closed Bug 36257 Opened 24 years ago Closed 19 years ago

Have the QA Contact field appear in New Bug submission form

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 2.20

People

(Reporter: lchiang, Assigned: shane.h.w.travis)

References

Details

Attachments

(2 files, 7 obsolete files)

Have the QA Contact field be editable and appear in New Bug submission form

When submitting a new bug report, the QA Contact field does not appear in the 
new bug form.  

Sometimes the QA contact may differ than the default QA contact that gets 
assigned.  Being able to change it at bug submission time would reduce bug email 
notification.  

If someone knows who the QA contact should be, it would be less steps to change 
it.

Suggest:

QA contact field be added with the text "(Leave blank to assign to default 
component QA contact)"

Thanks!
I agree. Also help on test bugs etc.

Not sure, what Lisa means with "editable".
by "editable", I mean that I can put text in that field.  I guess it is already 
assumed to be so.
Depends on who you mean with *I*... :-). I can edit it (just tried it), but not
everybody: <http://bugzilla.mozilla.org/confirmhelp.html>. Do you suggest any
changes to the current system? If not, please adjust the summary, so I can
recover from my confusion :).
I will relieve you from your confusion. :-) I really just want the QA contact 
field to be in the new bug submission form.
Summary: Have the QA Contact field be editable and appear in New Bug submission form → Have the QA Contact field appear in New Bug submission form
there's been a long standing terry philosophy about making the enter bug form as 
easy as possible. not sure i agree in this case though.
We can have several of those, targetted at different user groups. I usually have
to change a bug right after filing it to add keywords etc.. IMO, we should have
/a/ bug enter form which allows me to set all (appropriate) fields. Should I
widen this bug?
We need an advanced user section in the enter_bug page.
you have to log in before you can see the enter bug screen anyway.  Why not 
remove both fields unless the user has editbugs, and then let them edit either?
Severity: normal → enhancement
Bug #76770 and bug #40970 are related since the reporter is supposedly supposed
to be able to edit bugs regardless of editbugs.  If we decide to remove this, I
agree we don't need assignee and QA.

See also bug #25521 about adding keywords to enter_bug.
Target Milestone: --- → Bugzilla 2.16
-> Bugzilla product, Creating Bugs component, reassigning.
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → unspecified
Whiteboard: [people:qa] enter_bug
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Blocks: newbug
I actually made that change to our copy of 2.17 for this. it was very simple 
to do this and we found it usefull. problem is i have never made a patch for 
such a change before once i figure this out i will be glad to post my changes 
as a patch but for now here they are:

create.html.tmpl (@ line 146 or so basically right after the assigned to field:

   [% IF Param('useqacontact') %]
     <tr>
       <td align="right">
       <b><u>Q</u>A Contact:</b>
       </td>
       <td colspan="7">
         <input name="qa_contact" accesskey="q"
                value="[% bug.qa_contact.email FILTER html %]" size="32">
        (Leave blank to assign to default component QA contact)
       </td>
     </tr>
   [% END %]



post_bug.cgi: i changed the section on adding the qa_contact info to this


if (Param("useqacontact")) {
    SendSQL("SELECT initialqacontact FROM components " .
            "WHERE id = $component_id");
    my $qa_contact = FetchOneColumn();

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

Enhancements which don't currently have patches on them which are targetted at
2.18 are being retargetted to 2.20 because we're about to freeze for 2.18. 
Consideration will be taken for moving items back to 2.18 on a case-by-case
basis (but is unlikely for enhancements)
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Attached patch suggested patch (2.18 and 2.19) (obsolete) — Splinter Review
My company needed this feature so here's what I've done to
enable qa-contacts in the enter_bug form. Note that this depends on
the default-cc patches from bug #38922. That is, it auto-fills the
qa-contact field based on the component you select, but if you
manually enter a qa-contact, it'll leave it alone.

My company is using 2.18 but have confirmed the patch also works in
2.19.
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Attached patch Code patch for tip (obsolete) — Splinter Review
Here's a patch that doesn't require anything else to land first. Pretty
straightforward, actually... or maybe it just feels that way after all my
studying of this template to implement clone bug. :)

(Albert, I noticed that you didn't change enter_bug or post_bug at all. For the
former, I believe that means that QA Contact won't be saved through
bookmarkable templates; for the latter, I'm not sure that a modified QA Contact
would be saved at all -- the default would always be pulled from the DB!)
Assignee: myk → travis
Attachment #155894 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #174419 - Flags: review?
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
Comment on attachment 174419 [details] [diff] [review]
Code patch for tip

>+SendSQL("SELECT name, description, initialowner, initialqacontact
>+           FROM components
>+          WHERE product_id = $product_id
>+       ORDER BY name");
> while (MoreSQLData()) {
>-    my ($name, $description, $login, $realname) = FetchSQLData();
>+    my ($name, $description, $owner, $qacontact) = FetchSQLData();
> 
>     push @components, {
>         name => $name,
>         description => $description,
>-        default_login => $login,
>-        default_realname => $realname,
>+        initialowner => DBID_to_name($owner),
>+        initialqacontact => DBID_to_name($qacontact),
>     };
> }

An error message is displayed if the QA contact is not defined: "The name
__UNKNOWN__ is not a valid username. Either you misspelled it, or the person
has not registered for a Bugzilla account."

Before calling DBID_to_name(), you should be sure that $owner, respectively
$qacontact, is non-zero. A better way to do these both steps into a single one
is to do something like this:

"SELECT name, description, p1.login_name, case when initialqacontact>0 then
p2.login_name else '' end FROM components, profiles p1, profiles p2 where
p1.userid=initialowner and case when initialqacontact>0 then
p2.userid=initialqacontact else 1=1 end ORDER BY name".

This way, you don't need to call DBID_to_name() and an empty string is returned
if the initial QA contact is not defined.

Moreover, please use Bugzilla->dbh stuff instead of SendSQL(). ;)


>+    my $qa_contact;
>+    if ($::FORM{'assigned_to'} eq "") {
>+        SendSQL("SELECT initialqacontact FROM components " .
>+                "WHERE id = $component_id");
>+        $qa_contact = FetchOneColumn();
>+    } else {
>+        $qa_contact = DBNameToIdAndCheck(trim($::FORM{'qa_contact'}));
>+    }

I think if (trim($::FORM{'assigned_to'}) eq "") is better. Moreover, please use
$cgi->param() instead of $::FORM{}.


>+[% IF Param("useqacontact") %]
>+    <tr>
>+      <td align="right"><strong>QA Contact:</strong></td>
>+      <td colspan="3">
>+      [% INCLUDE global/userselect.html.tmpl
>+         name => "qa_contact"
>+         value => qa_contact
>+         disabled => qa_contact_disabled
>+         size => 32
>+         emptyok => 1
>+       %]
>+        <noscript>(Leave blank to assign to default qa contact)</noscript>
>+      </td>
>+    </tr>
>+[% END %]
>+
>   <tr>
>     <td align="right"><strong>URL:</strong></td>
>     <td colspan="3">

IMHO, the QA contact field should be just below the "Assign To" field instead
of being between the "Deadline" and "URL" fields.


I did not go through all the code so there may be some other issues.
Attachment #174419 - Flags: review? → review-
Attached patch Code patch for tip, take 2 (obsolete) — Splinter Review
Fixed, as per comments and IRC discussion.
Attachment #174419 - Attachment is obsolete: true
Attachment #174526 - Flags: review?(LpSolit)
Attached patch Code patch for tip, take 3 (obsolete) — Splinter Review
SQL breakthrough; new patch.
Attachment #174526 - Attachment is obsolete: true
Attachment #174528 - Flags: review?(LpSolit)
Attachment #174526 - Flags: review?(LpSolit)
Attachment #174528 - Flags: review?(justdave)
Attachment #174528 - Flags: review?(justdave)
Attachment #174528 - Flags: review?(LpSolit)
Attached patch Code patch for tip, take 4 (obsolete) — Splinter Review
Fixed, as per IRC review
Attachment #174528 - Attachment is obsolete: true
Attachment #175075 - Flags: review?(LpSolit)
Attached patch Code patch for tip, take 5 (obsolete) — Splinter Review
Oops... in addition to too much patch, introduced a new bug into v4. Here's a
cleaned up patch for v5 with just the stuff it should have.
Attachment #175075 - Attachment is obsolete: true
Attachment #175077 - Flags: review?(LpSolit)
Attachment #175075 - Flags: review?(LpSolit)
Comment on attachment 175077 [details] [diff] [review]
Code patch for tip, take 5

> [%- END %]
>-var last_default_owner;
> function set_assign_to() {

Please leave an empty line before the function declaration.


>     var form = document.Create;
>     var assigned_to = form.assigned_to.value;
>+    var qa_contact = form.qa_contact.value;

If Param("useqacontact") is 0, form.qa_contact is not defined and the JS script
stops working. You have to put var qa_contact = form.qa_contact.value; in a [%
IF Param("useqacontact") %] [% END %].

Else it's good, and all my comments in IRC have been reported, so next patch
will be the final one. ;)
Attachment #175077 - Flags: review?(LpSolit) → review-
Fixed as per comments above.
Attachment #175077 - Attachment is obsolete: true
Attachment #175150 - Flags: review?(LpSolit)
I haven't installed/played with this patch, but does this not have the potential
to 'break' sites that use QA-Contact for 'component watching'?  In other words,
if someone who doesn't necessarily know what they're doing opens a ticket and
provides a non-default QA-contact that isn't on people's watchlists, is it going
to fail to notify people who think they'll be notified because they're watching
the default QA-Contact?

It seems dumb to not provide this patch based solely on that concern, but it
also seems not-to-smart to break admittedly 'kludgey' functionality that we know
is being used by alot of sites (b.m.o. included).
(In reply to comment #24)
> does this not have the potential
> to 'break' sites that use QA-Contact for 'component watching'?

It does. In fact, it would break my own site, as we use QA for the Design 
Authority field, which is set at the component level and is never changed 
throughout the life of the bug.

HOWEVER... the fix for this is completely trivial.

--- create.html.tmpl    2005-02-22 10:36:26.143706179 -0600
+++ create.html.tmpl.qa_disabled        2005-02-22 10:36:43.407063575 -0600
@@ -233,7 +233,7 @@
       [% INCLUDE global/userselect.html.tmpl
          name => "qa_contact"
          value => qa_contact
-         disabled => qa_contact_disabled
+         disabled => 1
          size => 32
          emptyok => 1
        %]

Apply that patch, and the QA field still shows up on enter_bug.cgi, and still 
changes to be appropriate to the component, but is also disabled from user 
modification. 

This is what I'll be doing locally, and since it takes place in a template file 
it is an easy change for local Bugzilla admins to make too. You raise a valid 
point, however, and this probably shouldn't be added without letting people 
know how to disable it, so I've added the 'relnote' keyword to facilitate 
getting the message out.
Keywords: relnote
Comment on attachment 175150 [details] [diff] [review]
Code patch for tip, take 6

Nice work! r=LpSolit
Attachment #175150 - Flags: review?(LpSolit) → review+
Flags: approval?
Flags: documentation?
Flags: approval?
Flags: approval+
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.107; previous revision: 1.106
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.103; previous revision: 1.102
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tm
pl,v  <--  create.html.tmpl
new revision: 1.47; previous revision: 1.46
done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [people:qa] enter_bug
Added to the release notes in bug 286274.
Keywords: relnote
Attached patch documentation patch, v1 (obsolete) — Splinter Review
There is no specific place with fields available on bug creation. So I only updated the "Anatomy of a Bug" section where the QA contact field was missing.
Attachment #246085 - Flags: review?(documentation)
Attachment #246085 - Attachment is obsolete: true
Attachment #246225 - Flags: review?(documentation)
Attachment #246085 - Flags: review?(documentation)
Attachment #246225 - Flags: review?(documentation) → review+
Attachment #246225 - Flags: review+ → review?
Attachment #246225 - Flags: review? → review+
tip:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.52; previous revision: 1.51
done

2.22.1:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.37.2.11; previous revision: 1.37.2.10
done

2.20.3:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.33.2.15; previous revision: 1.33.2.14
done
Flags: documentation?
Flags: documentation2.22+
Flags: documentation2.20+
Flags: documentation+
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: