The default bug view has changed. See this FAQ.

Duplicate bugs are created when requestee confirmation is required and SKIP_REQUESTEE_ON_ERROR is disabled

NEW
Unassigned

Status

()

bugzilla.mozilla.org
General
4 years ago
4 years ago

People

(Reporter: jrmuizel, Unassigned)

Tracking

Production

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
This seems to happen when you create a bug with an attachment and request review on someone who doesn't exist. Bugzilla gives you a long complainy message and asks you to reattach the file. Doing so seems to create two bugs.

This has happened to me twice in the past two days with the following bugs:
823149, 823148
822143, 822142

Comment 1

4 years ago
Definitely not an upstream bug. If you ask someone who doesn't exist, your requestee is ignored and the bug + attachment submitted anyway.
Assignee: create-and-change → nobody
Component: Creating/Changing Bugs → General
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Version: unspecified → Production
(In reply to Frédéric Buclin from comment #1)
> Definitely not an upstream bug. If you ask someone who doesn't exist, your
> requestee is ignored and the bug + attachment submitted anyway.

you forgot about SKIP_REQUESTEE_ON_ERROR.



i can reproduce this on trunk with SKIP_REQUESTEE_ON_ERROR set to 0

if you enter a requestee which matches more than one user, you're presented with two confirmation pages.

the first page is the "select the correct user" page.
the second is the "invalid token" page.
Assignee: nobody → create-and-change
Component: General → Creating/Changing Bugs
Product: bugzilla.mozilla.org → Bugzilla
QA Contact: default-qa
Version: Production → unspecified
Flags: blocking4.4?
OS: Mac OS X → All
Hardware: x86 → All
Summary: Duplicate bugs are created → Duplicate bugs are created when requestee confirmation is required and SKIP_REQUESTEE_ON_ERROR is disabled

Comment 3

4 years ago
This is a constant which is not supposed to be edited. In fact, this constant is not editable. So you hacked the source code in bmo. Not an upstream bug (upstream, I would mark it INVALID).
Assignee: create-and-change → nobody
Component: Creating/Changing Bugs → General
Flags: blocking4.4?
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Version: unspecified → Production
(In reply to Frédéric Buclin from comment #3)
> This is a constant which is not supposed to be edited. In fact, this
> constant is not editable. So you hacked the source code in bmo. Not an
> upstream bug (upstream, I would mark it INVALID).

Then why do we have the constant there at all if setting it to the opposite value causes issues. If what you are saying is true, then we should just remove the constant and have the core code always skip on requestee error. 

At BMO, it was requested that we fail if the requestee does not exist so that the proper requestee can always be set. So we changed the value to 0 and thought it would work as expected by the description of the constant. Apparently it doesn't so we will have to fix it on our end and push the fix upstream. 

I just dont think that INVALID is appropriate here unless you are saying the constant should be removed which I don't think it should. It should work as expected based on the value set by SKIP_REQUESTEE_ON_ERROR.

dkl

Comment 5

4 years ago
(In reply to David Lawrence [:dkl] from comment #4)
> Then why do we have the constant there at all if setting it to the opposite
> value causes issues. If what you are saying is true, then we should just
> remove the constant and have the core code always skip on requestee error. 

We have a constant because we don't want to throw an error when attaching a new file. For existing attachments, it's fine to throw an error, because we are not loosing the attachment being uploaded. That's why this code is written like this:

    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
                                  $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);

which is clearer than written like that:

    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
                                  $bug, $attachment, $vars, 1);

where one wouldn't understand what 1 means. But in no way is this constant supposed to be removed nor changed to 0.


> At BMO, it was requested that we fail if the requestee does not exist so

Upstream, it was a clear decision that we didn't want to stop the upload process just for that. The requestee can be set again once the attachment is fully uploaded.


> that the proper requestee can always be set. So we changed the value to 0
> and thought it would work as expected by the description of the constant.

There is no description for this constant which says that you can change it to 0 to force errors to be thrown.


> I just dont think that INVALID is appropriate here unless you are saying the
> constant should be removed which I don't think it should. It should work as
> expected based on the value set by SKIP_REQUESTEE_ON_ERROR.

You are misinterpreting what you can do with this constant. It's like editing USER_MATCH_SUCCESS => 1 in User.pm to 0 and expecting that everything will work as expected. The code would fail miserably, because this constant is not supposed to be set to a false value. So yes, I'm really saying that this bug is invalid despite I say the constant should not be removed.
Thanks. I understand better now the reasoning for the constant. As for BMO, we either need 

1) revert back to allowing the attachment to be created even with an incorrect requestee user and make it more obvious what needs to be done to fix it by the user, or
2) figure out a way to upload the attachment to a staging area on the filesystem, throw the error asking for another requestee value or to drop the flag, then resubmitting the bug using the attachment previously uploaded. 

1) will obviously be easier but 2) should probably be done eventually for other reasons but will be more work involved.

dkl
looks like the cure is worse than the disease here.
let's revert SKIP_REQUESTEE_ON_ERROR to 1 before this week's push.
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.0
modified Bugzilla/Flag.pm
Committed revision 8431.

Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.2
modified Bugzilla/Flag.pm
Committed revision 8480. 

Leaving this bug open as we still have an issue that needs to be addressed here.
dkl
Created attachment 702556 [details]
screenshot 1
Created attachment 702557 [details]
screenshot 2

Updated

4 years ago
Duplicate of this bug: 833960
You need to log in before you can comment on or make changes to this bug.