Last Comment Bug 365302 - email userprefs doesn't tell you if you are a globalwatcher
: email userprefs doesn't tell you if you are a globalwatcher
Status: RESOLVED FIXED
: ue
Product: Bugzilla
Classification: Server Software
Component: User Accounts (show other bugs)
: 2.23.3
: All All
: -- normal (vote)
: Bugzilla 3.0
Assigned To: timeless
: default-qa
Mentors:
userprefs.cgi?tab=email
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-12-29 00:13 PST by timeless
Modified: 2008-01-04 06:14 PST (History)
3 users (show)
LpSolit: approval+
LpSolit: approval3.0+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
something like this? (913 bytes, patch)
2006-12-30 14:07 PST, timeless
LpSolit: review-
Details | Diff | Splinter Review
something like that (1.69 KB, patch)
2007-07-18 01:08 PDT, timeless
LpSolit: review-
Details | Diff | Splinter Review
use term.inology (2.08 KB, patch)
2007-07-19 18:55 PDT, timeless
LpSolit: review+
Details | Diff | Splinter Review

Description timeless 2006-12-29 00:13:42 PST
steps:
1. log out
2. log in as timeless@gmail (or another global watcher)
3. load mail user prefs

actual results:
User Watching

If you watch a user, it is as if you are standing in their shoes for the purposes of getting email. Email is sent or not according to your preferences for their relationship to the bug (e.g. Assignee).

You are currently not watching any users. 

expected results:
Some indication that "You are currently configured to receive mail for all bugs that you can see" (or "You are a global watcher") plus a note explaining to contact the admin if you need to be removed from this role.
Comment 1 timeless 2006-12-30 14:07:07 PST
Created attachment 250026 [details] [diff] [review]
something like this?

not sure i like the placement, but.... oh, and i have no idea if this works :)
Comment 2 Frédéric Buclin 2007-01-06 16:45:30 PST
Comment on attachment 250026 [details] [diff] [review]
something like this?

We should have a $user->is_global_watcher() method, so that we can easily reuse it, e.g. in BugMail.pm
Comment 3 timeless 2007-07-18 01:08:45 PDT
Created attachment 272750 [details] [diff] [review]
something like that

i can't find any way to use this in BugMail, but i'm certainly fine with a method.
Comment 4 Frédéric Buclin 2007-07-19 09:23:30 PDT
Comment on attachment 272750 [details] [diff] [review]
something like that

>Index: Bugzilla/User.pm

>+sub is_global_watcher {
>+    my $self = shift;
>+
>+    my @watchers = split /[,\s]+/, Bugzilla->params->{'globalwatchers'};
>+    my $index = grep $watchers[$_] eq $self->login, 0 .. $#watchers;
>+    return $index >= 0;
>+}

- Add parens to split(//, foo).
- Write: @foo = grep { $_ eq $self->login } @watchers;
         return scalar(@foo) ? 1 : 0;
  so that it's a bit more readable.
- Cache the result in $self->{'is_global_watcher'} as we do for ->is_insider.



>Index: template/en/default/account/prefs/email.html.tmpl

>+      You are watching all bugs. To stop watching all bugs you will need to contact

bugs -> [% terms.bugs %]
Comment 5 timeless 2007-07-19 18:55:34 PDT
Created attachment 273071 [details] [diff] [review]
use term.inology
Comment 6 Frédéric Buclin 2007-07-20 06:13:29 PDT
Comment on attachment 273071 [details] [diff] [review]
use term.inology

>+        my @foo = grep { $_ eq $self->login } @watchers;

No comment about @foo as a variable name. :) r=LpSolit for 3.0 and 3.1.
Comment 7 Max Kanat-Alexander 2007-07-20 17:40:03 PDT
Comment on attachment 273071 [details] [diff] [review]
use term.inology

Well, I have a comment @foo is an unacceptable variable name.

We're trying to make Bugzilla's code *clearer*, not unreadable.

You could just do "my $is_in_array = grep".

You don't even need to cache that, it's an unnecessary optimization, all it does is read Bugzilla->params.
Comment 8 Frédéric Buclin 2007-07-21 03:16:24 PDT
(In reply to comment #7)
> (From update of attachment 273071 [details] [diff] [review])
> Well, I have a comment @foo is an unacceptable variable name.

Max, you misunderstood my review comment. My "no comment" was clearly an incentive to change this variable name. I r+ it because I think timeless is able to do it on checkin.


> You don't even need to cache that, it's an unnecessary optimization

This optimization is fine and I asked him to do it. No reason to remove it.
Comment 9 Max Kanat-Alexander 2007-07-21 05:53:07 PDT
Comment on attachment 273071 [details] [diff] [review]
use term.inology

Okay. As long as it's changed on checkin.

The optimization is premature and useless, though.
Comment 10 Frédéric Buclin 2007-07-22 08:55:35 PDT
tip:

Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.155; previous revision: 1.154
done

3.0:

Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.148.2.2; previous revision: 1.148.2.1
done
Comment 11 timeless 2007-07-23 02:39:02 PDT
this was the other half

mozilla/webtools/bugzilla/Bugzilla/User.pm 	1.154
mozilla/webtools/bugzilla/template/en/default/account/prefs/email.html.tmpl 	1.29

BUGZILLA-3_0-BRANCH:
mozilla/webtools/bugzilla/Bugzilla/User.pm 	1.148.2.1 	mozilla/webtools/bugzilla/template/en/default/account/prefs/email.html.tmpl 	1.28.2.1
Comment 12 Frédéric Buclin 2008-01-04 06:14:06 PST
Has been relnoted in 3.0.1.

Note You need to log in before you can comment on or make changes to this bug.