859550 - Create a user profile page for bugzilla users

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

Create a user profile page for bugzilla users that will show summaries of some of their basic activities. Rather than a feed of activity like the user activity report, it will sum up counts of how many bugs a user has filed, commented on, and so on.  

Described at greater length here: https://wiki.mozilla.org/BMO/UserProfiles

Comment 1

[Colby Russell :crussell (no longer Mozilla-ing)]

(In reply to David Lawrence [:dkl] from bug 680215, comment #8)
> One thing of interest that is being worked on currently where this would be
> a better fit is the user profiles page. We can carry on this discussion
> there instead.
> 
> https://wiki.mozilla.org/BMO/UserProfiles

Creating a b.m.o feature and just deferring to it seems antithetical to mozillians.org's raison d'être.

Here's my other comment from that bug:
(Colby Russell :crussell wrote in bug 680215, comment #7)
> Ah, yeah, and the thing that I *really* came in here looking for:
> variable-length AKA fields on mozillians.org that b.m.o can use to
> supplement its autocomplete.
> 
> (Joshua Cranmer [:jcranmer] wrote in bug 842632, comment #13)
> > Note to self: :NeilAway does not match Neil when requesting review.
> > 
> > However much I want it to. :-P

Comment 2

[Colby Russell :crussell (no longer Mozilla-ing)]

(In reply to Colby Russell :crussell from comment #1)
> > variable-length AKA fields on mozillians.org that b.m.o can use

If that's confusing, what I meant is a variable-length list of aliases, which is biting this bug, too.  From the wiki page:

> right now automatically linking from bmo to mozillians isn't viable as
> mozillians only allows for a single email address, and many people use
> different accounts on bmo vs mozillians.

Comment 3

[:glob ✱]

Attachment: patch v1

here's an initial patch for this. the core is all there, however there's some cosmetic work to be done such as linking to the page which contains the descriptions (i need to chat with liz first).

after running checksetup, you need to run bin/migrate.pl and then bin/update.pl.  bin/update.pl is the script which will be execute periodically to update the statistics.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

glob suggested leaving out tooltip descriptions of each field and instead having a single "What do these fields mean?" link to a wiki page. That sounds much more sensible and easier to update than tooltips. Here is the page:

https://wiki.mozilla.org/User_profile_fields

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Wait, actually this is a better url for that page so that it fits into the rest of the BMO-related pages.  

https://wiki.mozilla.org/BMO/User_profile_fields

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Note, we need to add a link from the main page to the user profile.

Comment 7

[:glob ✱]

Attachment: patch v2

this revision:
  - adds last_activity field to User.get webservice
  - adds "what do these fields mean?" link to profile page
  - adds "Profile" link to user context menu
  - adds "User Profile" link to "Account Information" page

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 760912 [details] [diff] [review]
patch v2

Review of attachment 760912 [details] [diff] [review]:
-----------------------------------------------------------------

Overall looks good and works well. Just a few minor things.

::: contrib/reorg-tools/movebugs.pl
@@ +14,2 @@
>  use Bugzilla::Util;
> +use Bugzilla::Config qw(SetParam write_params);

Don't see these used anywhere in the file.

::: contrib/reorg-tools/movecomponent.pl
@@ +39,2 @@
>  use Bugzilla::Util;
> +use Bugzilla::Config qw(SetParam write_params);

I don't see these used anywhere in the file.

::: extensions/UserProfile/Extension.pm
@@ +14,5 @@
> +
> +use Bugzilla::Constants;
> +use Bugzilla::Extension::UserProfile::Util;
> +use Bugzilla::Install::Filesystem;
> +use Bugzilla::WebService::Util qw(filter validate);

Don't see these used anywhere here.

@@ +56,5 @@
> +
> +    my $user = Bugzilla->user;
> +    my ($assigned_to, $qa_contact);
> +    if (exists $args->{changes}) {
> +        return unless scalar(keys %{ $args->{changes} });

when making a comment only, there is a 'changes' key but it is always empty so last_activity_ts will not get updated for only a comment.
You would need to also check $bug->{added_comments}.

@@ +77,5 @@
> +        if (exists $args->{changes}->{product}) {
> +            tag_for_recount_from_bug($bug->id);
> +        }
> +
> +    } else {

Nit:

}
else {

@@ +127,5 @@
> +        my $timestamp = Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
> +        $user->set_last_activity_ts($timestamp);
> +        $user->update();
> +
> +    } elsif ($object->isa('Bugzilla::Product') && exists $args->{changes}->{name}) {

Nit:

}
elsif (

@@ +326,5 @@
> +                }
> +            },
> +            product => {
> +                TYPE    => 'VARCHAR(64)',
> +                NOTNULL => 1,

Should this be a foreign key to products table instead in case a product is deleted?

@@ +355,5 @@
> +    my $extensions_dir = bz_locations()->{'extensionsdir'};
> +    my $script_name = $extensions_dir . "/" . __PACKAGE__->NAME . "/bin/update.pl";
> +    $files->{$script_name} = {
> +        perms => Bugzilla::Install::Filesystem::WS_EXECUTE
> +    };

$script_name = $extensions_dir . "/" . __PACKAGE__->NAME . "/bin/migrate.pl";
   $files->{$script_name} = {
       perms => Bugzilla::Install::Filesystem::OWNER_EXECUTE
   };

::: extensions/UserProfile/lib/Util.pm
@@ +15,5 @@
> +                  tag_for_recount_from_bug
> +                  last_user_activity );
> +
> +use Bugzilla;
> +use Bugzilla::Config qw(SetParam write_params);

I don't see these used anywhere in the file.

@@ +317,5 @@
> +        { Slice => {} },
> +        $user_id,
> +    );
> +    foreach my $row (@$rows) {
> +        unless(defined $row->{$name_field}) {

nit: space after unless

::: extensions/UserProfile/template/en/default/pages/user_profile.html.tmpl
@@ +40,5 @@
> +</tr>
> +<tr>
> +  <th>Email</th>
> +  <td colspan="2">
> +    <a href="mailto:[% target.email FILTER uri %]">[% target.email FILTER html %]</a>

FILTER email for non-logged in users

Comment 10

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #9)
> > +            product => {
> > +                TYPE    => 'VARCHAR(64)',
> > +                NOTNULL => 1,
> 
> Should this be a foreign key to products table instead in case a product is
> deleted?

i had that initially, however due to the delays between actions taken and the statistics being updated, that became problematic. i also need somewhere to store the count for 'other', and didn't see the point of creating a new table just for one value.

Comment 11

[:glob ✱]

Attachment: patch v3

(In reply to David Lawrence [:dkl] from comment #9)
> > +    <a href="mailto:[% target.email FILTER uri %]">[% target.email FILTER html %]</a>
> 
> FILTER email for non-logged in users

as you need to know the user's email address in order to view their profile, there's little point hiding it at this point (it's also present in most of the bug search urls).


other issues addressed, along with a few other minor tweaks.

Comment 12

[David Lawrence [:dkl]]

Comment on attachment 776223 [details] [diff] [review]
patch v3

Review of attachment 776223 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/UserProfile/Extension.pm
@@ +219,5 @@
> +        }
> +        else {
> +            $product->{product} = Bugzilla::Product->new({ name => $product->{product} });
> +        }
> +    }

Sorry I didn't catch this before. The product names need to be filtered based on the current viewers permissions to see the product names.

Comment 13

[David Lawrence [:dkl]]

Comment on attachment 776223 [details] [diff] [review]
patch v3

Oops accidently cleared the review flag.

Comment 14

[David Lawrence [:dkl]]

Comment on attachment 776223 [details] [diff] [review]
patch v3

(In reply to David Lawrence [:dkl] from comment #12)
> Comment on attachment 776223 [details] [diff] [review]
> patch v3
> 
> Review of attachment 776223 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: extensions/UserProfile/Extension.pm
> @@ +219,5 @@
> > +        }
> > +        else {
> > +            $product->{product} = Bugzilla::Product->new({ name => $product->{product} });
> > +        }
> > +    }
> 
> Sorry I didn't catch this before. The product names need to be filtered
> based on the current viewers permissions to see the product names.

Everything else is looking good so once you revise for this, should be a quick r+ once verified.

dkl

Comment 15

[:glob ✱]

Attachment: 859550_4.patch

changes:
- fixed product visibility (count added to 'other')
- don't crash if a webservice User.get call filters out the user id
- filter out duplicate bug ids from activity/comment query
- add ui to profile page to load other user's profiles

Comment 16

[:glob ✱]

Attachment: 859550_5.patch

last revision threw an uninit warning when hitting the /user_profile page without providing a login parameter.

Comment 17

[David Lawrence [:dkl]]

Comment on attachment 782639 [details] [diff] [review]
859550_5.patch

Review of attachment 782639 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl with webservice fix

::: extensions/UserProfile/Extension.pm
@@ +159,5 @@
> +    my ($self, $args) = @_;
> +    my ($service, $users) = @$args{qw(webservice users)};
> +
> +    my $dbh = Bugzilla->dbh;
> +    my $ids = [ map { $_->{id}->value } grep { exists $_->{id} } @$users ];

Can't call method \"value\" without a package or object reference at ./extensions/UserProfile/Extension.pm line 163.

Need to just use $_->{id} and that is it. Same for line 171. Once I fixed those the webservice worked fine.

Comment 18

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #17)
> > +    my $ids = [ map { $_->{id}->value } grep { exists $_->{id} } @$users ];
> 
> Can't call method \"value\" without a package or object reference at
> ./extensions/UserProfile/Extension.pm line 163.

ah, that's true for REST, however via xmlrpc it's a blessed XMLRPC::Data object, so we'll need to call value there.  nothing that blessed() can't fix :)

schema only changes:

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
added extensions/UserProfile
added extensions/UserProfile/Config.pm
added extensions/UserProfile/Extension.pm
Committed revision 8910.

Comment 19

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
modified .htaccess
modified contrib/reorg-tools/movebugs.pl
modified contrib/reorg-tools/movecomponent.pl
modified extensions/BMO/web/js/edituser_menu.js
modified extensions/UserProfile/Extension.pm
added extensions/UserProfile/bin
added extensions/UserProfile/lib
added extensions/UserProfile/template
added extensions/UserProfile/bin/migrate.pl
added extensions/UserProfile/bin/update.pl
added extensions/UserProfile/lib/Util.pm
added extensions/UserProfile/template/en
added extensions/UserProfile/template/en/default
added extensions/UserProfile/template/en/default/hook
added extensions/UserProfile/template/en/default/pages
added extensions/UserProfile/template/en/default/hook/account
added extensions/UserProfile/template/en/default/hook/account/prefs
added extensions/UserProfile/template/en/default/hook/account/prefs/account-field.html.tmpl
added extensions/UserProfile/template/en/default/pages/user_profile.html.tmpl
Committed revision 8916.

Comment 20

[:glob ✱]

unfortunately we hit issues while pushing this to production.
we'll have to deploy this during scheduled downtime.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

glob, is that something we should ask IT for? How do we make that happen?

Comment 22

[:glob ✱]

(In reply to Liz Henry :lizzard from comment #21)
> glob, is that something we should ask IT for? How do we make that happen?

the ball's already rolling on that, via bug 900375.

Comment 23

[:glob ✱]

this is now live.

Comment 24

[Dave Miller [:justdave]]

I don't see it in the user context menu, only my own in my preferences.

Comment 25

[:glob ✱]

(In reply to Dave Miller [:justdave] from comment #24)
> I don't see it in the user context menu, only my own in my preferences.

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
modified extensions/BMO/web/js/edituser_menu.js
Committed revision 8969.

Comment 26

[Asa Dotzler [:asa]]

(In reply to Byron Jones ‹:glob› from comment #7)

>   - adds "Profile" link to user context menu

What is this user context menu you refer to here?

Comment 27

[:glob ✱]

(In reply to Asa Dotzler [:asa] from comment #26)
> What is this user context menu you refer to here?

left-click on a user's name (eg. your name above your comment).

Comment 28

[Asa Dotzler [:asa]]

Attachment: bmo-profile.jpg

(In reply to Byron Jones ‹:glob› from comment #27)
> (In reply to Asa Dotzler [:asa] from comment #26)
> > What is this user context menu you refer to here?
> 
> left-click on a user's name (eg. your name above your comment).

OK. This is pretty awful UI.  When a user mouses over that link, they see a mailto in the status bar. A primary click is supposed to perform that load -- 20 years of Web UI tells us that. For some probably significant number of users, myself included, that's a "don't left-click here unless you want to pop open an email compose window" signal. I suspect many people are completely missing this feature because the UI is miscommunicating the feature. 

If you want to hang a left-click menu on something, the gravatar would be a much better choice, especially if you overlayed or added a little menu indicator of some sort. Also, when you left-click, it's a menu, not a context menu. 

Attached is a super-quick mock-up of how this could look. Some new affordance is definitely needed here.

Comment 29

[:glob ✱]

(In reply to Asa Dotzler [:asa] from comment #28)
> OK. This is pretty awful UI.

i've filed bug 911812 for that discussion.