Eliminate use of $::userid from Bugzilla

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Frédéric Buclin)

Tracking

2.21
Bugzilla 2.22
Bug Flags:
approval +

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

13 years ago
Created attachment 192133 [details] [diff] [review]
Remove $::userid

OK, this was a pretty simple patch, generally. :-) It passes runtests, but it
might need some smoke-testing.
Assignee: general → mkanat
Status: NEW → ASSIGNED
Attachment #192133 - Flags: review?(LpSolit)
(Reporter)

Updated

13 years ago
Target Milestone: --- → Bugzilla 2.22
(Reporter)

Updated

13 years ago
Keywords: relnote
(Assignee)

Comment 2

13 years ago
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.
Attachment #192133 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 3

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

Comment 4

13 years ago
Yeah, I'm too busy at the moment. :-)
Assignee: mkanat → LpSolit
Status: ASSIGNED → NEW
(Assignee)

Updated

13 years ago
Blocks: 87411
(Assignee)

Comment 5

13 years ago
Created attachment 200333 [details] [diff] [review]
patch, v2
Attachment #192133 - Attachment is obsolete: true
Attachment #200333 - Flags: review?(wicked)
(Assignee)

Comment 6

13 years ago
Created attachment 200683 [details] [diff] [review]
patch, v2.1

unbitrotten patch
Attachment #200333 - Attachment is obsolete: true
Attachment #200683 - Flags: review?(wicked)
Attachment #200333 - Flags: review?(wicked)
(Assignee)

Comment 7

13 years ago
Created attachment 200848 [details] [diff] [review]
patch, v3

Small improvements + unbitrot
Attachment #200683 - Attachment is obsolete: true
Attachment #200848 - Flags: review?(wicked)
Attachment #200683 - Flags: review?(wicked)
(Assignee)

Comment 8

13 years ago
Created attachment 201114 [details] [diff] [review]
patch, v3.5

unbitrotten patch. wicked, I'm in a hurry so I couldn't test it. So it really needs testing.
Attachment #200848 - Attachment is obsolete: true
Attachment #201114 - Flags: review?(wicked)
Attachment #200848 - Flags: review?(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 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.
Attachment #201114 - Flags: review?(wicked) → review+
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 11

13 years ago
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
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Reporter)

Comment 12

13 years ago
Added to the Bugzilla 2.22 Release Notes in bug 322960.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.