Bug 942029: review suggestions only shows the first mentor

4 files changed, 53 insertions, 28 deletions

diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm
index b00fc4e69..0aa21ad5b 100644
--- a/extensions/Review/Extension.pm
+++ b/extensions/Review/Extension.pm
@@ -32,7 +32,7 @@ BEGIN {
     *Bugzilla::Product::reviewer_required = \&_product_reviewer_required;
     *Bugzilla::Component::reviewers       = \&_component_reviewers;
     *Bugzilla::Component::reviewers_objs  = \&_component_reviewers_objs;
-    *Bugzilla::Bug::mentor                = \&_bug_mentor;
+    *Bugzilla::Bug::mentors               = \&_bug_mentors;
     *Bugzilla::User::review_count         = \&_user_review_count;
 }
 
@@ -72,29 +72,35 @@ sub _reviewers_objs {
     return $object->{reviewers};
 }
 
-sub _bug_mentor {
+sub _bug_mentors {
     my ($self) = @_;
-    # extract the mentor from the status_whiteboard
-    # when the mentor gets its own field, this will be easier
-    if (!exists $self->{mentor}) {
-        my $mentor;
-        if ($self->status_whiteboard =~ /\[mentor=([^\]]+)\]/) {
+    # extract the mentors from the status_whiteboard
+    # when the mentors gets its own field, this will be easier
+    if (!exists $self->{mentors}) {
+        my @mentors;
+        my $whiteboard = $self->status_whiteboard;
+        while ($whiteboard =~ /\[mentor=([^\]]+)\]/g) {
             my $mentor_string = $1;
+            my $user;
             if ($mentor_string =~ /\@/) {
                 # assume it's a full username if it contains an @
-                $mentor = Bugzilla::User->new({ name => $mentor_string });
+                $user = Bugzilla::User->new({ name => $mentor_string });
             } else {
                 # otherwise assume it's a : prefixed nick.  only works if a
                 # single user matches.
-                my $matches = Bugzilla::User::match("*:$mentor_string*", 2);
-                if ($matches && scalar(@$matches) == 1) {
-                    $mentor = $matches->[0];
+                foreach my $query ("*:$mentor_string*", "*$mentor_string*") {
+                    my $matches = Bugzilla::User::match($query, 2);
+                    if ($matches && scalar(@$matches) == 1) {
+                        $user = $matches->[0];
+                        last;
+                    }
                 }
             }
+            push @mentors, $user if $user;
         }
-        $self->{mentor} = $mentor;
+        $self->{mentors} = \@mentors;
     }
-    return $self->{mentor};
+    return $self->{mentors};
 }
 
 sub _user_review_count {
diff --git a/extensions/Review/lib/WebService.pm b/extensions/Review/lib/WebService.pm
index acce32d2b..4cb3d48d8 100644
--- a/extensions/Review/lib/WebService.pm
+++ b/extensions/Review/lib/WebService.pm
@@ -20,9 +20,9 @@ sub suggestions {
     my ($self, $params) = @_;
     my $dbh = Bugzilla->switch_to_shadow_db();
 
-    my ($product, $component);
+    my ($bug, $product, $component);
     if (exists $params->{bug_id}) {
-        my $bug = Bugzilla::Bug->check($params->{bug_id});
+        $bug = Bugzilla::Bug->check($params->{bug_id});
         $product = $bug->product_obj;
         $component = $bug->component_obj;
     }
@@ -38,16 +38,27 @@ sub suggestions {
         ThrowUserError("reviewer_suggestions_param_required");
     }
 
-    my $reviewers = [];
+    my @reviewers;
+    if ($bug) {
+        # we always need to be authentiated to perform user matching
+        my $user = Bugzilla->user;
+        if (!$user->id) {
+            Bugzilla->set_user(Bugzilla::User->check({ name => 'nobody@mozilla.org' }));
+            push @reviewers, @{ $bug->mentors };
+            Bugzilla->set_user($user);
+        } else {
+            push @reviewers, @{ $bug->mentors };
+        }
+    }
     if ($component) {
-        $reviewers = $component->reviewers_objs;
+        push @reviewers, @{ $component->reviewers_objs };
     }
-    if (!@$reviewers) {
-        $reviewers = $product->reviewers_objs;
+    if (!@{ $component->reviewers_objs }) {
+        push @reviewers, @{ $product->reviewers_objs };
     }
 
     my @result;
-    foreach my $reviewer (@$reviewers) {
+    foreach my $reviewer (@reviewers) {
         push @result, {
             id    => $self->type('int', $reviewer->id),
             email => $self->type('email', $reviewer->login),
diff --git a/extensions/Review/template/en/default/hook/global/header-start.html.tmpl b/extensions/Review/template/en/default/hook/global/header-start.html.tmpl
index 585041e99..0c276ad8c 100644
--- a/extensions/Review/template/en/default/hook/global/header-start.html.tmpl
+++ b/extensions/Review/template/en/default/hook/global/header-start.html.tmpl
@@ -23,26 +23,28 @@
 
 [% IF bug %]
   [%# create attachment %]
-  [% mentor = bug.mentor %]
+  [% mentors = bug.mentors %]
   [% product_obj = bug.product_obj %]
   [% component_obj = bug.component_obj %]
 [% ELSIF attachment.bug %]
   [%# edit attachment %]
-  [% mentor = attachment.bug.mentor %]
+  [% mentors = attachment.bug.mentors %]
   [% product_obj = attachment.bug.product_obj %]
   [% component_obj = attachment.bug.component_obj %]
 [% ELSE %]
   [%# create bug %]
-  [% mentor = '' %]
+  [% mentors = [] %]
   [% product_obj = product %]
   [% component_obj = 0 %]
 [% END %]
 
 [% review_js = BLOCK %]
   review_suggestions = {
-    [% IF mentor %]
-      _mentor: [% PROCESS reviewer u = mentor %],
-    [% END %]
+    _mentors: [
+      [% FOREACH u = mentors %]
+        [% PROCESS reviewer %][% "," UNLESS loop.last %]
+      [% END %]
+    ],
 
     [% IF product_obj.reviewers %]
       _product: [
diff --git a/extensions/Review/web/js/review.js b/extensions/Review/web/js/review.js
index 6d0e24342..08ae29547 100644
--- a/extensions/Review/web/js/review.js
+++ b/extensions/Review/web/js/review.js
@@ -107,8 +107,8 @@ var REVIEW = {
         // build review list
         if (field.component != component) {
             field.menu.clearContent();
-            if (review_suggestions._mentor) {
-                REVIEW.add_menu_item(field_idx, review_suggestions._mentor, true);
+            for (var i = 0, il = review_suggestions._mentors.length; i < il; i++) {
+                REVIEW.add_menu_item(field_idx, review_suggestions._mentors[i], true);
             }
             if (review_suggestions[component] && review_suggestions[component].length) {
                 REVIEW.add_menu_items(field_idx, review_suggestions[component]);
@@ -129,6 +129,12 @@ var REVIEW = {
 
     add_menu_item: function(field_idx, user, is_mentor) {
         var menu = REVIEW.fields[field_idx].menu;
+        var items = menu.getItems();
+        for (var i = 0, il = items.length; i < il; i++) {
+            if (items[i].cfg.config.url.value == '#' + user.login) {
+                return;
+            }
+        }
         var queue = '';
         if (user.review_count == 0) {
             queue = 'empty queue';