Bug 256135: Parameter 'movers' is handled differently in Bug.pm and buglist.cgi - Patch by Fr�d�ric Buclin <LpSolit@gmail.com> r/a=myk

webtools/bugzilla/Bugzilla/Bug.pm

@@ -599,9 +599,8 @@ sub user {
     return $self->{'user'} if exists $self->{'user'};
     return {} if $self->{'error'};
 
-    my @movers = map { trim $_ } split(",", Param("movers"));
-    my $canmove = Param("move-enabled") && Bugzilla->user->id && 
-                  (lsearch(\@movers, Bugzilla->user->login) != -1);
+    my $user = Bugzilla->user;
+    my $canmove = Param('move-enabled') && $user->is_mover;
 
     # In the below, if the person hasn't logged in, then we treat them
     # as if they can do anything.  That's because we don't know why they
@@ -609,17 +608,17 @@ sub user {
     # Display everything as if they have all the permissions in the
     # world; their permissions will get checked when they log in and
     # actually try to make the change.
-    my $unknown_privileges = !Bugzilla->user->id
-                             || Bugzilla->user->in_group("editbugs");
+    my $unknown_privileges = !$user->id
+                             || $user->in_group("editbugs");
     my $canedit = $unknown_privileges
-                  || Bugzilla->user->id == $self->{assigned_to_id}
+                  || $user->id == $self->{assigned_to_id}
                   || (Param('useqacontact')
                       && $self->{'qa_contact_id'}
-                      && Bugzilla->user->id == $self->{qa_contact_id});
+                      && $user->id == $self->{qa_contact_id});
     my $canconfirm = $unknown_privileges
-                     || Bugzilla->user->in_group("canconfirm");
-    my $isreporter = Bugzilla->user->id
-                     && Bugzilla->user->id == $self->{reporter_id};
+                     || $user->in_group("canconfirm");
+    my $isreporter = $user->id
+                     && $user->id == $self->{reporter_id};
 
     $self->{'user'} = {canmove    => $canmove,
                        canconfirm => $canconfirm,

webtools/bugzilla/Bugzilla/User.pm

@@ -1102,7 +1102,18 @@ sub wants_mail {
                               
     return defined($wants_mail) ? 1 : 0;
 }
-  
+
+sub is_mover {
+    my $self = shift;
+
+    if (!defined $self->{'is_mover'}) {
+        my @movers = map { trim($_) } split(',', Param('movers'));
+        $self->{'is_mover'} = ($self->id
+                               && lsearch(\@movers, $self->login) != -1);
+    }
+    return $self->{'is_mover'};
+}
+
 sub get_userlist {
     my $self = shift;
 
@@ -1565,6 +1576,12 @@ Returns true if the user wants mail for a given set of events. This method is
 more general than C<wants_bug_mail>, allowing you to check e.g. permissions
 for flag mail.
 
+=item C<is_mover>
+
+Returns true if the user is in the list of users allowed to move bugs
+to another database. Note that this method doesn't check whether bug
+moving is enabled.
+
 =back
 
 =head1 CLASS FUNCTIONS

webtools/bugzilla/buglist.cgi

@@ -945,18 +945,8 @@ $vars->{'urlquerypart'} = $params->canonicalise_query('order',
                                                       'cmdtype',
                                                       'query_based_on');
 $vars->{'order'} = $order;
-
-# The user's login account name (i.e. email address).
-my $login = Bugzilla->user->login;
-
 $vars->{'caneditbugs'} = UserInGroup('editbugs');
 
-# Whether or not this user is authorized to move bugs to another installation.
-$vars->{'ismover'} = 1
-  if Param('move-enabled')
-    && defined($login)
-      && Param('movers') =~ /^(\Q$login\E[,\s])|([,\s]\Q$login\E[,\s]+)/;
-
 my @bugowners = keys %$bugowners;
 if (scalar(@bugowners) > 1 && UserInGroup('editbugs')) {
     my $suffix = Param('emailsuffix');

webtools/bugzilla/process_bug.cgi

@@ -592,14 +592,8 @@ my $action = trim($cgi->param('action') || '');
 if ($action eq Param('move-button-text')) {
     Param('move-enabled') || ThrowUserError("move_bugs_disabled");
 
-    my $exporter = $user->login;
-    my $movers = Param('movers');
-    $movers =~ s/\s?,\s?/|/g;
-    $movers =~ s/@/\@/g;
-    if ($exporter !~ /($movers)/) {
-        ThrowUserError("auth_failure", {action => 'move',
-                                        object => 'bugs'});
-    }
+    $user->is_mover || ThrowUserError("auth_failure", {action => 'move',
+                                                       object => 'bugs'});
 
     # Moved bugs are marked as RESOLVED MOVED.
     my $sth = $dbh->prepare("UPDATE bugs
@@ -615,7 +609,7 @@ if ($action eq Param('move-button-text')) {
         $comment = $cgi->param('comment') . "\n\n";
     }
     $comment .= "Bug moved to " . Param('move-to-url') . ".\n\n";
-    $comment .= "If the move succeeded, $exporter will receive a mail\n";
+    $comment .= "If the move succeeded, " . $user->login . " will receive a mail\n";
     $comment .= "containing the number of the new bug in the other database.\n";
     $comment .= "If all went well,  please mark this bug verified, and paste\n";
     $comment .= "in a link to the new bug. Otherwise, reopen this bug.\n";
@@ -652,7 +646,7 @@ if ($action eq Param('move-button-text')) {
 
     # Now send emails.
     foreach my $id (@idlist) {
-        $vars->{'mailrecipients'} = { 'changer' => $exporter };
+        $vars->{'mailrecipients'} = { 'changer' => $user->login };
         $vars->{'id'} = $id;
         $vars->{'type'} = "move";
 

webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl

@@ -344,7 +344,7 @@
 
 <input type="submit" value="Commit">
 
-[% IF ismover %]
+[% IF Param('move-enabled') && user.is_mover %]
   <input type="submit" name="action" value="[% Param('move-button-text') %]">
 [% END %]