Let me know who is watching my account

RESOLVED FIXED in Bugzilla 2.20

Status

()

RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: LpSolit, Assigned: LpSolit)

Tracking

2.19.2
Bugzilla 2.20
Bug Flags:
approval +
documentation +
documentation2.22 +
documentation2.20 +

Details

(URL)

Attachments

(2 attachments, 8 obsolete attachments)

(Assignee)

Description

14 years ago
We can watch any user's account but these users are not aware of who is
currently watching them. The list of users watching you should be displayed in
the "Email Settings" page below the "Users to watch" textbox.

The goal here is not to grant or deny permissions to these users to watch your
account (bug 71785). I only want to know who is watching my account, if any.
Else a message like "Nobody is watching you" should be displayed.
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 1

14 years ago
Created attachment 176936 [details] [diff] [review]
patch, v1

This patch displays a list of all users watching your account.
Attachment #176936 - Flags: review?(myk)
Comment on attachment 176936 [details] [diff] [review]
patch, v1

>Index: userprefs.cgi

>+        my $watchers_ref = $dbh->selectcol_arrayref(
>+            "SELECT profiles.login_name FROM watch, profiles " .
>+            "WHERE watched = ? AND watch.watcher = profiles.userid " .
>+            "ORDER BY profiles.login_name",
>+            undef, $userid);
>+        $vars->{'nb_watchers'} = scalar(@$watchers_ref);
>+        $vars->{'watchers'} = join(', ', @$watchers_ref);

Instead of assigning nb_watchers and joining watchers here, just pass the array
reference to the template and do the counting and joining there, i.e.:

$vars->{watchers} = $dbh->selectcol_arrayref(...);

Then use watchers.size in place of nb_watchers and the following code in place
of the join:

[% FOREACH watcher = watchers %]
  [% watcher %][% ", " UNLESS loop.last() %]
[% END %]

As for appearance, this would look better if it were part of the table that
contains the "Users to watch" label and textbox, i.e. something like:

	 Users to watch: [			  ]
     Users watching you: someone
			 someone else
			 a third person

I'm not sure how the number of users fits into this UI, but it's probably not
necessary, since there are rarely more than a handful of watchers, even in
large public Bugzilla installations (the most-watched person on b.m.o has 12
watchers, and no other user has more than 9 watchers).

Finally, ideally the list of watchers would show identity rather than just
email address, but that's just icing on the cake.
Attachment #176936 - Flags: review?(myk) → review-
> [% FOREACH watcher = watchers %]
>   [% watcher %][% ", " UNLESS loop.last() %]
> [% END %]

Err, this is actually much simpler as [% watchers.join(", ") %].
(Assignee)

Comment 4

14 years ago
Created attachment 177306 [details] [diff] [review]
patch, v2

I tried to use $vars->{'watchers'} = selectall_hashref() without success in the
sense that the ORDER BY statement was not taken into account and the list was
then out of order. :( I expected to be able to find a solution without the
while() loop. Anyway, the list now is sorted by user (real)name.
Attachment #176936 - Attachment is obsolete: true
Attachment #177306 - Flags: review?(myk)

Comment 5

14 years ago
That's because order is not preserved in a hash. You would need to either use a
"sort" statement when referencing it instead of in the query or use an array ref.
Comment on attachment 177306 [details] [diff] [review]
patch, v2

Jacob's right, the problem here is that you were using *_hashref instead of
*_arrayref.  The following code should work:

$vars->{'watchers'} = $dbh->selectall_arrayref(...);


>Index: userprefs.cgi

>+        my $sth = $dbh->prepare(
>+            "SELECT profiles.realname, profiles.login_name " .
>+            "FROM watch, profiles " .

It would be good to add a note here that at some point we want to switch this
code to using Bugzilla::User objects rather than recreating
Bugzilla::User::identity manually.


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

>+      <th align="right" valign="top">Users watching you:</th>

This should be valign="baseline".


>+            [% watcher.realname FILTER html %] &lt;[% watcher.login_name FILTER html %]&gt; <br>

To make this consistent with Bugzilla::User::identity, don't quote the user's
email address if their name is missing, i.e.:

[% IF watcher.realname %]
  [% watcher.realname FILTER html %] &lt;[% watcher.login_name FILTER html
%]&gt;
[% ELSE %]
  [% watcher.login_name FILTER html %]
[% END %]<br>
Attachment #177306 - Flags: review?(myk) → review-
(Assignee)

Comment 7

14 years ago
Created attachment 177409 [details] [diff] [review]
patch, v3
Attachment #177306 - Attachment is obsolete: true
Attachment #177409 - Flags: review?(myk)
Comment on attachment 177409 [details] [diff] [review]
patch, v3

Yup, this is great. r=myk
Attachment #177409 - Flags: review?(myk) → review+
Flags: approval+

Comment 9

14 years ago
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.71; previous revision: 1.70
done
Checking in template/en/default/account/prefs/email.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/email.html.
tmpl,v  <--  email.html.tmpl
new revision: 1.17; previous revision: 1.16
done
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Flags: documentation?
Resolution: --- → FIXED

Updated

14 years ago
Keywords: relnote
Added to the release notes in bug 286274.
Keywords: relnote
Created attachment 207873 [details] [diff] [review]
docs patch v1

wonders if Users to watch was The first item on Email Settings on 2004/11 though,
this will have to be improved anyway.
Attachment #207873 - Flags: review?(documentation)

Comment 12

13 years ago
Comment on attachment 207873 [details] [diff] [review]
docs patch v1

there should probably be 2 blank lines between </section> and <section

<title>settings</title>

This should probably be Initial Capitals to match <title>Email Settings</title> and friends. (same applies for later <title> blocks)

> On this page, you can select csv separator, comment order,

write "CSV" in all caps.

> and enable/disable showing quips. 

specify whether to show quips.

--
      <para>
        The first item on this page is marked <quote>Users to watch</quote>.
        When you enter one or more comma-delineated user accounts (usually email
        addresses) into the text entry box, you will receive a copy of all the
        bugmail those users are sent (security settings permitting).
        This powerful functionality enables seamless transitions as developers
        change projects or users go on holiday.
      </para>

This isn't your block, but a bug needs to be filed to indicate that you don't receive a copy of bugmail they are sent, you have a chance to receive a copy of the same bugmails for which they have a chance to receive bugmail with the exception of request mails.

The exception is a bug, but documentation should be accurate (and the bug should be annotated such that when it's fixed, the documentation is redacted).

The distinction is that a user might turn off all bugmail and you might have all bugmail enabled which means you will get bugmail that the user didn't get even though you wouldn't have gotten it if you weren't watching the user. similarly if you turn off bugmail for certain cases that the user you watch didn't, then you won't get a copy of the bugmail sent to that user you were watching.
--

<quote>Users watching you</quote> shows the users who are watching you. 

+ This means the user has you listed in their <quote>Users to watch</quote> list and can get bugmail accordingly (<@see>Users to watch</@see>).

[no, i don't know the syntax for @see, please find one, if there isn't one, that's ok, you can use English]

Personally, I'd rather you add blank lines before your <para> and after your </para>. This file is slightly inconsistent, although it does look like it tries to use them in certain instances.
Attachment #207873 - Flags: review?(documentation) → review-
Created attachment 211920 [details] [diff] [review]
docs patch for 2.20 v2
Attachment #207873 - Attachment is obsolete: true
Attachment #211920 - Flags: review?(documentation)
Comment on attachment 211920 [details] [diff] [review]
docs patch for 2.20 v2

>Index: docs/xml/using.xml

>       <para>
>+        <quote>Users watching you</quote> shows the users who are watching you.
>+        This means the user has you listed in their <quote>Users to watch</quote>
>+        list and can get bugmail according to your relationship to the bug and
>+        their </quote>Field/recipient specific options</quote> setting.
>+      </para>

Stray </quote>

>+
>+      <para>
>         In general, users have almost complete control over how much (or
>         how little) email Bugzilla sends them. If you want to receive the
>         maximum amount of email possible, click the <quote>Enable All 
>@@ -957,6 +974,15 @@
> 
>     </section>
> 
>+    <section id="saved-searches">
>+      <title>saved-searches</title>

You didn't capatalise this.

>+
>+      <para>
>+        You can run, edit, delete and show/hide your saved searches on this page.
>+      </para>
>+
>+    </section>
>+
Attachment #211920 - Flags: review?(documentation) → review-
Created attachment 212596 [details] [diff] [review]
docs patch for 2.20 v3
Attachment #211920 - Attachment is obsolete: true
Attachment #212596 - Flags: review?(documentation)

Comment 16

13 years ago
Comment on attachment 212596 [details] [diff] [review]
docs patch for 2.20 v3

Almost perfect.

> select CSV separator

Unfortunately this either needs an article (the, a), a possesive (your), or a rewrite (what separator to use for CSV lists)
Created attachment 213467 [details] [diff] [review]
docs patch for tip v4
Attachment #212596 - Attachment is obsolete: true
Attachment #213467 - Flags: review?(documentation)
Attachment #212596 - Flags: review?(documentation)
Comment on attachment 213467 [details] [diff] [review]
docs patch for tip v4

>Index: docs/xml/using.xml

>+      <title>Saved-searches</title>

This should be:

<title>Saved Searches</title>

 since it's never hyphenated in Bugzilla, and it's a title here.

Fix that, and I'll give it r+.
Attachment #213467 - Flags: review?(documentation) → review-
Created attachment 213929 [details] [diff] [review]
docs patch for tip v5
Attachment #213467 - Attachment is obsolete: true
Attachment #213929 - Flags: review?(documentation)
Created attachment 213933 [details] [diff] [review]
docs patch for tip v6

oops...
sorry for spam...
:-(
Attachment #213929 - Attachment is obsolete: true
Attachment #213933 - Flags: review?(documentation)
Attachment #213929 - Flags: review?(documentation)

Comment 21

13 years ago
Comment on attachment 213933 [details] [diff] [review]
docs patch for tip v6

+      <para>
+        On this page, you can select which separator to use for CSV
+        lists, the comment order, and can specify whether to show quips.
+      </para>

This shouldn't be commited on the tip, because on the tip you can do more (specify what to do after changing a bug, etc).

Maybe you uploaded by mistake the 2.20 patch?

However, I'd appreciate if you'd open new bugs for introducing documentation like this.

Let's keep this bug for documentation related to the "who's watching my account" thing, so that I don't have to take upon commit each paragraph, and see in what branches should it go.
Attachment #213933 - Flags: review?(documentation) → review-
Attachment #213933 - Attachment is obsolete: true
Created attachment 213975 [details] [diff] [review]
docs patch for tip
Attachment #213975 - Flags: review?(documentation)

Comment 23

13 years ago
Comment on attachment 213975 [details] [diff] [review]
docs patch for tip

Cool!

+        <quote>Users watching you</quote> shows the users who are watching you.

This one sounds a little bit obvious.

We can probably get away by skipping it:

"Each user listed in the <quote>Users watching you</quote> field has you listed in their ... "

I'll skip it on checkin. This should go on 2.20, 2.22 and tip.
Attachment #213975 - Flags: review?(documentation) → review+

Comment 24

13 years ago
Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.47; previous revision: 1.46
done

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.37.2.8; previous revision: 1.37.2.7
done

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.33.2.11; previous revision: 1.33.2.10
done
Flags: documentation?
Flags: documentation2.22+
Flags: documentation2.20+
Flags: documentation+
You need to log in before you can comment on or make changes to this bug.