95889 - Lots of spurious commas in the activity table on keywords

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Dave Miller [:justdave]]

After the b.m.o update, the activity table is showing lots of extra commas in
the entries where more than one keyword was added or removed.

I suspect this code is at fault:

            my @old = split(/[ ,]/, $oldvalue);
            my @new = split(/[ ,]/, $newvalue);

The keywords entries already had comma-space for the separators, and the above
split only matches a single character.  Thus we wound up with a lot of null
records accidently created on a lot of bugs.

The patch for this will need two parts to it.
a) fix the conversion algorithm so it doesn't bite any other installs
b) go back and scan the activity table for keywords entries and fix the
corruption it caused to the existing entries (remove extra commas from the
records) so when b.m.o updates next, it fixes it.

Comment 1

[Jacob Steenhagen]

Part 1 is included on the patch for bug 85890.  The real trick is part 2 because
it should only be run one (and I'm not sure how to guarentee that).

Comment 2

[Dave Miller [:justdave]]

SELECT added, removed
FROM bugs_activity
WHERE added LIKE '%, ,%' OR removed LIKE '%, ,%'

if (MoreSQLData()) {
   # Fix table
}

Then again, it's only been in the tree a week, maybe it's safe to assume b.m.o
is the only place that's pulled it from cvs that recently?  Maybe we should just
provide a standalone script to fix it and post to mozilla-webtools that the code
was broken for a week and if you ran the cvs tip during that timeframe that you
need to run the script....

Comment 3

[Matthew Tuck [:CodeMachine]]

Ha!  If we were willing to do _that_, we could should just convert bug activity
right now from using commas to using separate records.

Comment 4

[Zach Lipton [:zach]]

Why not just write the standalone script into checksetup.pl and have 
checksetup.pl look for the error and correct it automatically? 

Comment 5

[Dave Miller [:justdave]]

Jake, what bug was the patch that fixed the first part of this on?  The one you
referenced above is a Mozilla bug.

Comment 6

[Jacob Steenhagen]

I'm sorry, the it's bug 95890.  That patch doesn't fix anything that was
incorrectly converted, but it should prevent the problem from happening.

So I guess what we should do is post to n.p.m.webtools and ask anyone who
updated to the tip between Jul 20 and Aug 19 to respond so we can help them fix
their activity log (both this and any keywords that may have been effected by
bug 95890).

Comment 7

[Dave Miller [:justdave]]

Bug 95890 contained a patch which fixed part A.  Part B can be fixed using the
same methods outlines int he text in bug 95890.  Since this was a cosmetic error
and not a dataloss error, this is just yet another reason to remember that the
cvs code is development status.

Comment 8

[Dave Miller [:justdave]]

Moving to Bugzilla product