277504 - quips migrated from quip file have an (invalid) userid of 0

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Max Kanat-Alexander]

So, justdave pointed out on IRC that if you migrate quips from the "quips" file
to the DB (that is, you run checksetup and it does that part of an upgrade),
then all the added quips get a userid of 0.

Sanity Check complains about this, and that's because it should. 0 is an invalid
userid.

Instead, the userid should just be NULL, and the quips system should be able to
deal with that.

It's just a minor schema change and then some "fixit" code in checksetup, I think.

Comment 1

[Ulrich Windl]

I'd like to know how to get the problem ("Bad value 0 found in quips.uderid")
fixed once it's in the database. "Sanitycheck.cgi isn't very useful there.

Comment 2

[Frédéric Buclin]

Some users are complaining on IRC that sanitycheck.cgi complains about quips
when upgrading from 2.16 to 2.18. It's not good! It gives them the impression
that something is wrong with their installation. We should fix this problem
before 2.18.1 and 2.20rc1, IMO.

mkanat, are you going to fix it?

Comment 3

[Max Kanat-Alexander]

LpSolit: If you want to fix it, feel free to take it, and I'll review it.

Comment 4

[Frédéric Buclin]

Attachment: convert '0' -> 'NULL', v1

This patch convert all userid = 0 to userid = NULL. sanitycheck.cgi does not
care of 'NULL' values so tests are successful again.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 174804 [details] [diff] [review]
convert '0' -> 'NULL', v1

>-     userid mediumint not null default 0, 
>+     userid mediumint default null, 

   Nit: I think you actually don't need a "default null," you can just leave
that off.

>+    $dbh->do('UPDATE quips SET userid = NULL WHERE userid = 0');

  Print a little message saying you're doing this, so admins know it's getting
fixed.

Comment 6

[Frédéric Buclin]

Attachment: convert '0' -> 'NULL', v2

Comment 7

[Max Kanat-Alexander]

Comment on attachment 174907 [details] [diff] [review]
convert '0' -> 'NULL', v2

OK. But don't add the bug-number comments to the table definitions. That's what
cvs blame is for.

That can be removed on checkin.

Comment 8

[Shane H. W. Travis]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.352; previous revision: 1.351
done

Comment 9

[Shane H. W. Travis]

I guess we should do 2.18 as well...

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.289.2.26; previous revision: 1.289.2.25
done