169822 - data corruption moving bugs between products with attachment statuses that have the same name

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect, P1)

Description

[Myk Melez [:myk] [@mykmelez]]

If you define two attachment statuses that have the same name but are valid for
different products, and you move a bug with an attachment that has the status
from one product to another, the bug's attachment will appear to have the status
in "show bug", but the status will not be checked in "edit attachment", and
loading and submitting the "edit attachment" form will cause the status to be
cleared.

The solution to this is to migrate the status ID from one number to the other if
a bug is moved between products with the same status name.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

The alternate solution that Asa just proposed to me is to compare names rather
than IDs when determining which checkboxes are checked in "edit attachment". 
Then, when someone submitted the form, the underlying record would automatically
get changed from one ID to the other (because the processing code would delete
all that attachment's status records from the database and then re-insert them).

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Hmm, Asa's approach only solves half the problem, because "show bug" will still
display attachment statuses that don't exist in the new product.

Here are the bugs where the bug's product doesn't match the product for which
the attachment statuses are valid:

mysql> select distinct bugs.bug_id AS 'Bug', attachments.attach_id AS
'Attachment', attachstatusdefs.product AS "Old Product", bugs.product AS "New
Product" from attachstatuses, attachstatusdefs, attachments, bugs WHERE
attachstatuses.statusid = attachstatusdefs.id AND attachstatuses.attach_id =
attachments.attach_id and attachments.bug_id = bugs.bug_id and
attachstatusdefs.product != bugs.product order by bugs.bug_id;
+--------+------------+-------------+-----------------+
| Bug    | Attachment | Old Product | New Product     |
+--------+------------+-------------+-----------------+
|  74248 |      67091 | Browser     | MailNews        |
|  76416 |      84576 | Browser     | MailNews        |
|  85543 |      48953 | MailNews    | Browser         |
|  85725 |      79170 | Browser     | Tech Evangelism |
|  92892 |      55484 | Browser     | mozilla.org     |
| 105016 |      53733 | Browser     | PSM             |
| 105126 |      53821 | MailNews    | Browser         |
| 107425 |      72826 | MailNews    | Browser         |
| 122149 |      66709 | Browser     | mozilla.org     |
| 123554 |      69085 | Browser     | MailNews        |
| 123554 |      70307 | Browser     | MailNews        |
| 128586 |      78489 | PSM         | NSS             |
| 128915 |      72488 | PSM         | NSS             |
| 129366 |      93005 | Bugzilla    | mozilla.org     |
| 132697 |      75506 | Browser     | MailNews        |
| 136958 |      80062 | NSPR        | Browser         |
| 147977 |      86293 | MailNews    | Browser         |
| 153622 |      88794 | Chimera     | Browser         |
| 164925 |      97047 | Chimera     | Browser         |
| 168302 |      98944 | MailNews    | Browser         |
+--------+------------+-------------+-----------------+
20 rows in set (14.28 sec)

Comment 3

[Bradley Baetz (:bbaetz)]

I'm sure we already had this discussion, mainly wrt the request tracker. I can't
see it in the bug, so it was probably on IRC.

When moving between products, foreach attachment, foreach status, if one exists
in the new product with the same name, then rename it, else drop it.

I think thats what we decided, no?

Midairs not firing if someone adds a flag while someone else changes he product
are probably the business of the other bug we have on attachments + midairs.

Comment 4

[Gervase Markham [:gerv]]

Can we just have it so that in the underlying table which maps names to IDs, the
names are unique? If you want another status somewhere else with the same name,
it reuses the ID.

Gerv

Comment 5

[Myk Melez [:myk] [@mykmelez]]

>Can we just have it so that in the underlying table which maps names to IDs, the
>names are unique? If you want another status somewhere else with the same name,
>it reuses the ID.

For the current system, that's a complicated hack.  In the new system (request
tracker), that's the way it works.

Comment 6

[Matthew Tuck [:CodeMachine]]

The only reasonable solution to this is to pop up some text on the product
change page indicating which statuses will disappear and which are suddenly
available.

The only name matching that should be done is to provide a default set of ticked
checkboxes in the new status section.  It shouldn't be automatic.

The product change page exists for a reason, we have been *way* too slack at
adding to it when we should.

Comment 7

[Matthew Tuck [:CodeMachine]]

See bug #170019 for a sanity check.

Comment 8

[Matthew Tuck [:CodeMachine]]

I've been told the new flag types system that replaces attachment statuses is
component specific - this complicates matters somewhat since component isn't
even known until product change page, and therefore we probably need a second
product change page.

The following options come to mind:

(a) Make product and component into one field on show_bug, then we know
everything on the product change page.
(b) Have a second change page for the flag types.
(c) Attempt to prove if, for the source/destination products & source component,
it doesn't matter what the destination component is, for all flags.  If you find
this for all flags, place all the flag types on the first page.  Otherwise,
place all the flag types on a second page.
(d) Attempt to prove this for specific flags.  For those it's proven for,
display them on the first page.  For those it's not, save them for the second
page, and hope a component will be changed such that there won't need to be a
second page at all.

Comment 9

[Gervase Markham [:gerv]]

Do we need to do anything about this bug for the b.m.o. upgrade?

Gerv

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Actually this bug is fixed by the request tracker checkin, which removes flags
that are inapplicable to the bug's new product