321834 - Existing QA Contact on bug can not be deleted/erased

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Ryan Thompson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20051010 Firefox/1.0.7 (Ubuntu package 1.0.7)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20051010 Firefox/1.0.7 (Ubuntu package 1.0.7)

Once a QA Contact has been set on a bug, it can not be removed, if the Component does not have a default QA contact. This can be reproduced by editing an existing bug with the QA Contact field set; delete the text in the QA Contact field, and Commit the changes. I see: "Email sent to: no one, Excluding: qa_email, assignee_email", and, when the bug is viewed again, the QA Contact field still has the old value.

Changing the QA Contact to another valid QA Contact does work.

This was not an issue in 2.16.7.

If the Component does have a default QA contact, it is possible to erase the QA contact for a bug and have it stay blank.

Reproducible: Always

Steps to Reproduce:
1. Enable QA Contacts
2. Create a Component with a blank default QA Contact
3. File/edit a bug, and fill in the QA Contact field
4. Re-edit the bug, and attempt to delete the QA Contact by erasing the text in the input box and Commit the change.
5. Reload the bug
Actual Results:  
The QA Contact was still set to the original value.

Expected Results:  
After Step 4, The QA Contact should be blank.

Comment 1

[Frédéric Buclin]

You can only delete the QA contact when changing several bugs at once (even if it's the only one selected), not when editing a single bug. This behavior is so by design.

Comment 2

[Ryan Thompson]

Summary update to remove "if Component has no default QA"; I've reproduced this both ways, now.

Please have another look at this bug. Attempting to remove the QA Contact field when using the "Change several bugs..." does not work as described in Comment #1. The problem is evident in the following snippet from process_bug.cgi:

        my $assignee;
        my $qacontact;

899:    if (defined $cgi->param('qa_contact')
            && $cgi->param('knob') ne "reassignbycomponent")
        {   
            my $name = trim($cgi->param('qa_contact'));
            # The QA contact cannot be deleted from show_bug.cgi for a single bug!
905:        if ($name ne $cgi->param('dontchange')) {

I've observed that $cgi->param('qa_contact') is undef (not "") if it was blank in the user input, so this conditional bails out immediately.

Comment 3

[Ryan Thompson]

Attachment: Minimal patch to fix the undef qa_contact problem

This tiny patch removes the "defined" check for $cgi->param("qa_contact"), and sets $name to "" if trim() returns undef. With this patch, the QA Contact field can be removed when editing multiple bugs, as described in Comment #1.

Comment 4

[Ryan Thompson]

Attachment: Patch to lift restriction on removing QA Contact

Here is an alternate patch that incorporates the previous patch, but also always allows the QA Contact field to be removed when editing single bugs.

If this patch isn't accepted into the Bugzilla sources, at least other folks who find this bug can apply the patch, if desired.

Comment 5

[victory <[e-mail]>]

WORKSFORME on 2.20 & tip
btw, what version do you talking?
rev. 1.1 has less than 300 lines and 7 years old and i couldn't found your patch matchs up to.

Comment 6

[Teemu Mannermaa (:wicked)]

Managed succesfully to remove QA Contact on the bug mentioned in URL using the method described in comment #1.

Comment 7

[Marc Schumann [:Wurblzap]]

(In reply to comment #1)
> You can only delete the QA contact when changing several bugs at once (even if
> it's the only one selected), not when editing a single bug. This behavior is so
> by design.

Why don't we want the QA contact removed from a bug? Removing the QA contact via a mass change is a workaround, but the bug persists.

Comment 8

[Ryan Thompson]

Sorry... Apparently I didn't specify the version when filing the bug. All testing and patches against BUGZILLA-2_20-STABLE (process_bug.cgi 1.263.2.3)

Interestingly, the Change Several method works on the 2.20 landfill, but not on two separate installs on two machines on our private network, both freshly checked out from BUGZILLA-2_20-STABLE.

On our installs, as described in Comment #2, $cgi->param('qa_contact') returns undef, which short-circuits the conditional, preventing the qa_contact from ever being deleted, whether editing one bug or multiple bugs.

Either of my provided patches fix these installs, and I can't imagine either one breaking anything else.

I then checked out HEAD on one of these systems, and the problem did *not* occur, even though the conditional in process_bug.cgi is identical.

Both are running Ubuntu 5.10 (Breezy Badger), Apache 2.0.54, Perl 5.8.7.

$ ./checksetup.pl 

Checking perl modules ...
Checking for       AppConfig (v1.52)   ok: found v1.56
Checking for             CGI (v2.93)   ok: found v3.10
Checking for    Data::Dumper (any)     ok: found v2.121_04
Checking for    Date::Format (v2.21)   ok: found v2.22
Checking for             DBI (v1.38)   ok: found v1.48
Checking for      File::Spec (v0.84)   ok: found v3.05
Checking for      File::Temp (any)     ok: found v0.16
Checking for        Template (v2.08)   ok: found v2.14
Checking for      Text::Wrap (v2001.0131) ok: found v2001.09293
Checking for    Mail::Mailer (v1.65)   ok: found v1.67
Checking for        Storable (any)     ok: found v2.13

The following Perl modules are optional:
Checking for              GD (v1.20)    not found
Checking for     Chart::Base (v1.0)     not found
Checking for     XML::Parser (any)      not found
Checking for       GD::Graph (any)      not found
Checking for GD::Text::Align (any)      not found
Checking for     PatchReader (v0.9.4)   not found
[...]
Precompiling templates ...
Checking for      DBD::mysql (v2.9003) ok: found v2.9007
Checking for           MySQL (v3.23.41) ok: found v4.0.24_Debian-10ubuntu2-log

Populating duplicates table...

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> Interestingly, the Change Several method works on the 2.20 landfill, but not on
> two separate installs on two machines on our private network, both freshly
> checked out from BUGZILLA-2_20-STABLE.

That's because 2.20 has a bug which has been fixed after its release, see bug 315339. This fix will be included in 2.20.1.


(In reply to comment #7)
> Why don't we want the QA contact removed from a bug? Removing the QA contact
> via a mass change is a workaround, but the bug persists.

Marc, I don't remember the whole discussion I had with justdave about this (this was a long time ago), but the idea was that we didn't want to let the QA contact to be removed too easily. As mass-change requires editbugs privs, it's not a workaround, but a way to require some privs to do this kind of changes.

Comment 10

[Ryan Thompson]

(In reply to comment #9)
> That's because 2.20 has a bug which has been fixed after its release, see bug
> 315339. This fix will be included in 2.20.1.

Ahh. I didn't find that bug in my initial search. That explains what I'm seeing. Thanks.


> (In reply to comment #7)
> > Why don't we want the QA contact removed from a bug? Removing the QA contact
> > via a mass change is a workaround, but the bug persists.
> 
> Marc, I don't remember the whole discussion I had with justdave about this
> (this was a long time ago), but the idea was that we didn't want to let the QA
> contact to be removed too easily. As mass-change requires editbugs privs, it's
> not a workaround, but a way to require some privs to do this kind of changes.

Understood. So, given that reasoning, why not just require editbugs privs to change/remove the QA Contact, then? The way it is now is confusing, because process_bug.cgi just silently ignores a blank QA Contact. Having to go through mass-change really feels like a kludge.

It's not necessarily more secure, either, since it only applies to deletion of the QA Contact field, yet allows add/change. (So one could still, say, set the QA Contact to something else, which will do about as much harm as deleting the value).

Comment 11

[Max Kanat-Alexander]

I hate to re-open this after it's VERIFIED, but Ryan's right. If you have editbugs, you should be able to clear the QA Contact from show_bug, just like any other field.

It just confuses users. Nobody's going to guess that you have to go to the mass-change page. That's like telling people that if they want to put oil in their car, they have to open the glove compartment.

Comment 12

[Frédéric Buclin]

Not a bug as it is currently per design. That's not something I want to see in 2.22. A good way to get regressions.

Comment 13

[Kevin Benton]

The problem in our case is, if a QA contact is specified and we really do need to remove it, users now have to go to the DBA who has to clear the field manually.  To me, that seems to be a train wreck waiting to happen.  It's not that big of a problem if it's done correctly, but it does create a support issue that I think the software should take care of by itself.  I know that we're frozen, but as far as I'm concerned, this isn't really a minor issue because a user clears the QA contact field in the browser, but it doesn't get cleared in the DB.  In other words, to really remove a QA contact from Bugzilla requires a DBA to remove the entry from the record in the back end.

Comment 14

[Frédéric Buclin]

Read on my lips: it's so by design, and anyone with editbugs privs can already delete the QA contact from the mass-change form. No need to ask an DB admin to do it. That's not a regression, not a bug, only a request to relax the way to delete the QA contact. This won't go into 3.0.

Comment 15

[Ryan Thompson]

(In reply to comment #14)
> Read on my lips: it's so by design, 

Then we are proposing a design change.

> and anyone with editbugs privs can already
> delete the QA contact from the mass-change form. 

As discussed, this is confusing (and, IMHO, doesn't gain us anything anyway).

> [...]
> This won't go into 3.0.

That is disappointing.

Comment 16

[Max Kanat-Alexander]

I think that you should be able to clear the QA Contact from show_bug.

The rule is that programs should do what people expect them to do, not something tricky. If people shouldn't be clearing the QA Contact, they shouldn't have editbugs.

Comment 17

[Denis Roy]

(In reply to comment #11)
> I hate to re-open this after it's VERIFIED, but Ryan's right. If you have
> editbugs, you should be able to clear the QA Contact from show_bug, just like
> any other field.
> 
> It just confuses users. Nobody's going to guess that you have to go to the
> mass-change page.

meetoo.  See https://bugs.eclipse.org/bugs/show_bug.cgi?id=199090 (the person who opened that *does* have editbugs).

The current design is confusing.

Comment 18

[Frédéric Buclin]

A side effect of bug 396497 is to allow this now.

Comment 19

[Frédéric Buclin]

Bug 396497 has been checked in. This feature will be available in Bugzilla 3.1.2.

Comment 20

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.