291539 - Flags get duplicated if multiple inclusions apply

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Joel Peshkin]

If a Bug flag is defined to apply to All/All as well as a specific product, then
it will appear twice in the show_bug page.

Comment 1

[Frédéric Buclin]

*** Bug 292892 has been marked as a duplicate of this bug. ***

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Replace SELECT by SELECT DISTINCT. This way, we avoid flag type duplication due
to multiple entries in the flaginclusions table.

I think (hope) all other queries, including ones using flags count, are not
affected by the addition of DISTINCT (untested) as they only use the flagtypes
table or also use the flags table but with one value per flag type.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1.1

Maybe a better comment: only the flaginclusions table causes flag type
duplication, not the flagexclusions one.

Comment 4

[Frédéric Buclin]

2.19.x specific! 2.18.1 is not affected. Backing out the patch from bug 285555
fixes the problem. Adding dependency on it.

Comment 5

[Frédéric Buclin]

myk wants mkanat and joel on it. The patch on bug 285555 may have introduce some
regressions:

(22:58:59) myk: LpSolit: do you not think bug 285555 introduced a problem that
should be resolved by modifying that bug's fix?
(23:01:08) myk: LpSolit: seems like it would make sense to get the folks who
fixed 285555 to look at the problem and help figure out the solution
(23:01:41) myk: LpSolit: your patch seems to fix the problem, but it may also be
papering over a more fundamental problem with the fix for bug 285555; or maybe
not; it would be useful to know that
(23:02:15) LpSolit: myk: did you receive the mail I sent you about this bug?
(23:03:08) myk: LpSolit: yes, but we should involve the folks who fixed bug
285555 in figuring this out
(23:04:37) LpSolit: myk: but you reviewed it ;)
(23:04:37) myk: LpSolit: max and joel also participated
(23:05:22) myk: LpSolit: yes, but joel and max worked with tomas to identify the
problem and design its solution
(23:05:55) myk: i just did code review and testing

Comment 6

[Max Kanat-Alexander]

THe problem was that Tomas removed the HAVING clause entirely and switched to a
JOIN-based system. I didn't look over the code in-depth.

All he *had* to do was change the HAVING clause to contain the aggregate
function instead of the alias. If you would like to back out that patch and fix
it in a much simpler way, I wouldn't object.

Comment 7

[Joel Peshkin]

The code prior to bug 285555 was pretty awful.  I think that a DISTINCT is
preferable to usign aggregate functions in the HAVING clause.  Bug 285555 does
have the correct solution, it just missed the DISTINCT.

Comment 8

[Frédéric Buclin]

Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.17; previous revision: 1.16
done