Bug 256135: Parameter 'movers' is handled differently in Bug.pm and buglist.cgi ...

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index beb64cbf5016608c1a5a7159ed1674b714b3fb58..3a9a64ddc021b0ed0dfc0295920da33d94c33966 100755 (executable)
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -588,9 +588,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
@@ -598,17 +597,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,

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 55daccefcce923339b8bd482df3412d681e8d8c5..639da0c371821a5d24dc67ae6b138cd137d595f2 100644 (file)
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1105,7 +1105,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;
 
@@ -1554,6 +1565,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

diff --git a/buglist.cgi b/buglist.cgi
index 45c0db5524a4f5e4261b652abb76d758ffb48585..2e59006697a127b1543968bca3950a1686fec101 100755 (executable)
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -940,18 +940,8 @@ $vars->{'closedstates'} = ['CLOSED', 'VERIFIED', 'RESOLVED'];
 $vars->{'urlquerypart'} = $::buffer;
 $vars->{'urlquerypart'} =~ s/(order|cmdtype)=[^&]*&?//g;
 $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');

diff --git a/move.pl b/move.pl
index 4dbae4c547a6ee35cc108dc78e2a1aa3b2815442..96e964b55e3744ee29ad6a0abc78dc21efb16238 100755 (executable)
--- a/move.pl
+++ b/move.pl
@@ -36,15 +36,18 @@ use Bugzilla::Bug;
 use Bugzilla::Config qw(:DEFAULT $datadir);
 use Bugzilla::BugMail;
 
+my $cgi = Bugzilla->cgi;
+
 unless ( Param("move-enabled") ) {
+  print $cgi->header();
+  PutHeader("Move Bugs");
   print "\n<P>Sorry. Bug moving is not enabled here. ";
   print "If you need to move a bug, contact " . Param("maintainer");
+  PutFooter();
   exit;
 }
 
-Bugzilla->login(LOGIN_REQUIRED);
-
-my $cgi = Bugzilla->cgi;
+my $user = Bugzilla->login(LOGIN_REQUIRED);
 
 if (!defined $cgi->param('buglist')) {
   print $cgi->header();
@@ -57,11 +60,7 @@ if (!defined $cgi->param('buglist')) {
   exit;
 }
 
-my $exporter = Bugzilla->user->login;
-my $movers = Param("movers");
-$movers =~ s/\s?,\s?/|/g;
-$movers =~ s/@/\@/g;
-unless ($exporter =~ /($movers)/) {
+unless ($user->is_mover) {
   print $cgi->header();
   PutHeader("Move Bugs");
   print "<P>You do not have permission to move bugs<P>\n";
@@ -70,14 +69,15 @@ unless ($exporter =~ /($movers)/) {
 }
 
 my @bugs;
+my $exporterid = $user->id;
+
+print $cgi->header();
+PutHeader("Move Bugs");
 
-print "<P>\n";
 foreach my $id (split(/:/, scalar($cgi->param('buglist')))) {
-  my $bug = new Bugzilla::Bug($id, $::userid);
+  my $bug = new Bugzilla::Bug($id, $exporterid);
   push @bugs, $bug;
   if (!$bug->error) {
-    my $exporterid = DBNameToIdAndCheck($exporter);
-
     my $fieldid = GetFieldID("bug_status");
     my $cur_status= $bug->bug_status;
     SendSQL("INSERT INTO bugs_activity " .
@@ -99,7 +99,7 @@ foreach my $id (split(/:/, scalar($cgi->param('buglist')))) {
         $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";
@@ -107,10 +107,9 @@ foreach my $id (split(/:/, scalar($cgi->param('buglist')))) {
         "($id,  $exporterid, now(), " . SqlQuote($comment) . ")");
 
     print "<P>Bug $id moved to " . Param("move-to-url") . ".<BR>\n";
-    Bugzilla::BugMail::Send($id, { 'changer' => $exporter });
+    Bugzilla::BugMail::Send($id, { 'changer' => $user->login });
   }
 }
-print "<P>\n";
 
 my $buglist = $cgi->param('buglist');
 $buglist =~ s/:/,/g;
@@ -138,3 +137,4 @@ $template->process("bug/show.xml.tmpl", { bugs => \@bugs,
 $msg .= "\n";
 
 Bugzilla::BugMail::MessageToMTA($msg);
+PutFooter();

diff --git a/process_bug.cgi b/process_bug.cgi
index 7cebab2bbcc05d62cea01fe4df56eddc9e5f6c00..cf10e0c6ffd086f40ddf11e4a915c151bf3cc903 100755 (executable)
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -595,14 +595,11 @@ if (defined $cgi->param('id')) {
     }
 }
 
-my $action = '';
-if (defined $cgi->param('action')) {
-  $action = trim($cgi->param('action'));
-}
-if (Param("move-enabled") && $action eq Param("move-button-text")) {
+my $action = trim($cgi->param('action') || '');
+
+if ($action eq Param('move-button-text')) {
   $cgi->param('buglist', join (":", @idlist));
   do "move.pl" || die "Error executing move.cgi: $!";
-  PutFooter();
   exit;
 }
 

diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl
index ea52af3310106b9d5e8bd53f40307bada38d6dec..5f8b1ee41122573276d4205f0515b3fe9ea4a84f 100644 (file)
--- a/template/en/default/list/edit-multiple.html.tmpl
+++ b/template/en/default/list/edit-multiple.html.tmpl
@@ -322,7 +322,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 %]