274428 - checksetup should fixup flagtypes with spaces or commas

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Nick Barnes]

We now disallow flag names which contain spaces or commas in their names (see
bug 182375).  Users who upgrade an installation will have difficulty unless they
rename their flagtypes (see also bug 262268).  Checksetup should either error or
fix up bad flag names.  I have a patch, tested against the 2.18 branch and the
trunk, which renames flag types by replacing either spaces or commas with
underscores (when possible), printing messages so that the administrator can see
what is happening.  I'll attach my patch.

Comment 1

[Nick Barnes]

Attachment: patch v1 for 2.18 branch or for trunk

Herewith the patch.  Output generated looks like this, when run against a
database with lots of bad cases (when the underscore substitution hits an
existing flag name).  Most databases won't have any bad cases, of course.

Creating table whine_events ...
Checking flag type names for spaces and commas...
  Bad flag type name "a b" ...
  ... can't rename as "a_b" ...
  renamed flag type "a b" as "a_b'"
  Bad flag type name "b,c" ...
  ... can't rename as "b_c" ...
  ... can't rename as "b_c'" ...
  ... can't rename as "b_c''" ...
  renamed flag type "b,c" as "b_c'''"
  Bad flag type name "123456789 123456789 123456789 123456789 1234567890" ...
  ... can't rename as "123456789_123456789_123456789_123456789_1234567890" ...
  renamed flag type "123456789 123456789 123456789 123456789 1234567890" as
"123456789_123456789_123456789_123456789_1234567..."
  Bad flag type name "foo, bar baz" ...
  renamed flag type "foo, bar baz" as "foo__bar_baz"
... done.
Adding new field grant_type to table group_group_map ...

Maybe adding single-quote characters to uniquify a flag name is ugly, but it
works.	I could easily change it to do something else.

Comment 2

[Marc Schumann [:Wurblzap]]

Comment on attachment 168614 [details] [diff] [review]
patch v1 for 2.18 branch or for trunk

You need to put $tryflagname into %flagtypes on a successful rename.

+	     $dbh->do("UPDATE flagtypes SET name = \"$tryflagname\" " .
+		      "WHERE id = " . $flagtypes{$flagname});

Nit: this is nicer with a bound parameter.

Comment 3

[Nick Barnes]

Attachment: v2

Fixed those two problems.  How's that?

Comment 4

[Marc Schumann [:Wurblzap]]

Comment on attachment 168619 [details] [diff] [review]
v2

That's good. Thank you!

Comment 5

[Vlad Dascalu]

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

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