304075 - Eliminate use of $::userid from Bugzilla

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

We've removed most of the instances of $::userid in Bugzilla, now, and replaced
them with Bugzilla->user->id. There are few enough instances left that I think
we could get them all in one patch.

Comment 1

[Max Kanat-Alexander]

Attachment: Remove $::userid

OK, this was a pretty simple patch, generally. :-) It passes runtests, but it
might need some smoke-testing.

Comment 2

[Frédéric Buclin]

Comment on attachment 192133 [details] [diff] [review]
Remove $::userid

>Index: buglist.cgi

>             @legal_severity
>             @settable_resolution
>             @target_milestone
>-            $userid
>             @versions);

From what I see (without testing), $userid at line 387 is now undefined.


>Index: chart.cgi

Bugzilla->login(LOGIN_REQUIRED);

>+ $series->$action(Bugzilla->user->id);

>+ Bugzilla->user->id, $series_id);

Suggestion (nit):

write:	      my $user = Bugzilla->login(LOGIN_REQUIRED);
and then use: $user->id;


>Index: editcomponents.cgi

This file has already the user ID defined, lines 115-116:

my $user = Bugzilla->login(LOGIN_REQUIRED);
my $whoid = $user->id;

>+   $sdata->[0], Bugzilla->user->id, 1,

You can use it here.


>Index: editproducts.cgi

The same remark applies here.


>Index: post_bug.cgi

the user is already defined at line 54:

my $user = Bugzilla->login(LOGIN_REQUIRED);

So you can now use $user->id instead.


>Index: request.cgi

>-                  ON ccmap.who = $::userid
>+                  ON ccmap.who = " . Bugzilla->user->id . " 

When the user ID is used so many times in a file, I think it makes sense to put
it into a variable, for instance using the same trick as above.


Note that I didn't apply the patch and didn't check if all occurences were
correctly caught.

Comment 3

[Frédéric Buclin]

mkanat, any progress with this patch? I could do it if you are too busy. ;)

Comment 4

[Max Kanat-Alexander]

Yeah, I'm too busy at the moment. :-)

Comment 5

[Frédéric Buclin]

Attachment: patch, v2

Comment 6

[Frédéric Buclin]

Attachment: patch, v2.1

unbitrotten patch

Comment 7

[Frédéric Buclin]

Attachment: patch, v3

Small improvements + unbitrot

Comment 8

[Frédéric Buclin]

Attachment: patch, v3.5

unbitrotten patch. wicked, I'm in a hurry so I couldn't test it. So it really needs testing.

Comment 9

[Teemu Mannermaa (:wicked)]

Comment on attachment 201114 [details] [diff] [review]
patch, v3.5

>Index: userprefs.cgi
>===================================================================
>@@ -48,8 +45,10 @@
>+             "SELECT realname FROM profiles WHERE userid = ?", undef, $user->id);

Nit: This goes a bit over the 80 chars. Please, indent it less to fix this.

>@@ -165,9 +167,10 @@
>+    my $settings = $user->settings;
>+    my @setting_list = keys %{$user->settings};

Nit: In the second line, you should use $settings variable directly here. Just like in DoSettings sub.

>@@ -218,10 +222,10 @@
>+    my $sth = $dbh->prepare("SELECT relationship, event " . 
>                                      "FROM email_setting " . 
>+                                     "WHERE user_id = ?");

Nit: Also fix the SELECT intendation here.

Comment 10

[Teemu Mannermaa (:wicked)]

Comment on attachment 201114 [details] [diff] [review]
patch, v3.5

Seems to work in my tests. Please consider fixing my nits in previous comment before commit.

Comment 11

[Frédéric Buclin]

Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.23; previous revision: 1.22
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.317; previous revision: 1.316
done
Checking in chart.cgi;
/cvsroot/mozilla/webtools/bugzilla/chart.cgi,v  <--  chart.cgi
new revision: 1.17; previous revision: 1.16
done
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.446; previous revision: 1.445
done
Checking in duplicates.cgi;
/cvsroot/mozilla/webtools/bugzilla/duplicates.cgi,v  <--  duplicates.cgi
new revision: 1.50; previous revision: 1.49
done
Checking in editcomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.64; previous revision: 1.63
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.106; previous revision: 1.105
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.125; previous revision: 1.124
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.347; previous revision: 1.346
done
Checking in index.cgi;
/cvsroot/mozilla/webtools/bugzilla/index.cgi,v  <--  index.cgi
new revision: 1.17; previous revision: 1.16
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.131; previous revision: 1.130
done
Checking in quips.cgi;
/cvsroot/mozilla/webtools/bugzilla/quips.cgi,v  <--  quips.cgi
new revision: 1.32; previous revision: 1.31
done
Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v  <--  show_bug.cgi
new revision: 1.38; previous revision: 1.37
done
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <--  showdependencygraph.cgi
new revision: 1.46; previous revision: 1.45
done
Checking in showdependencytree.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencytree.cgi,v  <--  showdependencytree.cgi
new revision: 1.37; previous revision: 1.36
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.91; previous revision: 1.90
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.57; previous revision: 1.56
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.116; previous revision: 1.115
done
Checking in Bugzilla/Auth/Login/WWW.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Login/WWW.pm,v  <--  WWW.pm
new revision: 1.8; previous revision: 1.7
done

Comment 12

[Max Kanat-Alexander]

Added to the Bugzilla 2.22 Release Notes in bug 322960.