Bug 388036: Make Bugzilla::Bug do updating for alias in process_bug

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

4 files changed, 37 insertions, 79 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 60fd18bb6..cf1051a74 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -50,7 +50,7 @@ use Storable qw(dclone);
 use base qw(Bugzilla::Object Exporter);
 @Bugzilla::Bug::EXPORT = qw(
     AppendComment ValidateComment
-    bug_alias_to_id ValidateBugAlias ValidateBugID
+    bug_alias_to_id ValidateBugID
     RemoveVotes CheckIfVotedConfirmed
     LogActivityEntry
     BUG_STATE_OPEN is_open_state
@@ -156,6 +156,7 @@ use constant UPDATE_VALIDATORS => {
 
 sub UPDATE_COLUMNS {
     my @columns = qw(
+        alias
         everconfirmed
         bug_file_loc
         bug_severity
@@ -472,8 +473,10 @@ sub update {
     # this code should go below the duplicates-table-updating code below.
     foreach my $field (keys %$changes) {
         my $change = $changes->{$field};
-        LogActivityEntry($self->id, $field, $change->[0], $change->[1], 
-                         Bugzilla->user->id, $delta_ts);
+        my $from = defined $change->[0] ? $change->[0] : '';
+        my $to   = defined $change->[1] ? $change->[1] : '';
+        LogActivityEntry($self->id, $field, $from, $to, Bugzilla->user->id,
+                         $delta_ts);
     }
 
     # If this bug is no longer a duplicate, it no longer belongs in the
@@ -691,7 +694,28 @@ sub _check_alias {
    my ($invocant, $alias) = @_;
    $alias = trim($alias);
    return undef if (!Bugzilla->params->{'usebugaliases'} || !$alias);
-   ValidateBugAlias($alias);
+
+    # Make sure the alias isn't too long.
+    if (length($alias) > 20) {
+        ThrowUserError("alias_too_long");
+    }
+    # Make sure the alias isn't just a number.
+    if ($alias =~ /^\d+$/) {
+        ThrowUserError("alias_is_numeric", { alias => $alias });
+    }
+    # Make sure the alias has no commas or spaces.
+    if ($alias =~ /[, ]/) {
+        ThrowUserError("alias_has_comma_or_space", { alias => $alias });
+    }
+    # Make sure the alias is unique, or that it's already our alias.
+    my $other_bug = new Bugzilla::Bug($alias);
+    if (!$other_bug->{error}
+        && (!ref $invocant || $other_bug->id != $invocant->id))
+    {
+        ThrowUserError("alias_in_use", { alias => $alias,
+                                         bug_id => $other_bug->id });
+    }
+
    return $alias;
 }
 
@@ -1182,6 +1206,7 @@ sub _set_global_validator {
 # "Set" Methods #
 #################
 
+sub set_alias { $_[0]->set('alias', $_[1]); }
 sub set_custom_field {
     my ($self, $field, $value) = @_;
     ThrowCodeError('field_not_custom', { field => $field }) if !$field->custom;
@@ -2752,53 +2777,6 @@ sub ValidateBugID {
     }
 }
 
-# ValidateBugAlias:
-#   Check that the bug alias is valid and not used by another bug.  If 
-#   curr_id is specified, verify the alias is not used for any other
-#   bug id.  
-sub ValidateBugAlias {
-    my ($alias, $curr_id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    $alias = trim($alias || "");
-    trick_taint($alias);
-
-    if ($alias eq "") {
-        ThrowUserError("alias_not_defined");
-    }
-
-    # Make sure the alias isn't too long.
-    if (length($alias) > 20) {
-        ThrowUserError("alias_too_long");
-    }
-
-    # Make sure the alias is unique.
-    my $query = "SELECT bug_id FROM bugs WHERE alias = ?";
-    if ($curr_id && detaint_natural($curr_id)) {
-        $query .= " AND bug_id != $curr_id";
-    }
-    my $id = $dbh->selectrow_array($query, undef, $alias); 
-
-    my $vars = {};
-    $vars->{'alias'} = $alias;
-    if ($id) {
-        $vars->{'bug_id'} = $id;
-        ThrowUserError("alias_in_use", $vars);
-    }
-
-    # Make sure the alias isn't just a number.
-    if ($alias =~ /^\d+$/) {
-        ThrowUserError("alias_is_numeric", $vars);
-    }
-
-    # Make sure the alias has no commas or spaces.
-    if ($alias =~ /[, ]/) {
-        ThrowUserError("alias_has_comma_or_space", $vars);
-    }
-
-    $_[0] = $alias;
-}
-
 # Validate and return a hash of dependencies
 sub ValidateDependencies {
     my $fields = {};
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 39d25298d..01359868a 100755
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -56,7 +56,6 @@ use constant WS_ERROR_CODE => {
     bug_access_denied           => 102,
     invalid_field_name          => 108,
     # These all mean "invalid alias"
-    alias_not_defined        => 103,
     alias_too_long           => 103,
     alias_in_use             => 103,
     alias_is_numeric         => 103,
diff --git a/process_bug.cgi b/process_bug.cgi
index 267613f67..7773d15e7 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -651,28 +651,13 @@ if ($cgi->param('component') ne $cgi->param('dontchange')) {
     }
 }
 
-# If this installation uses bug aliases, and the user is changing the alias,
-# add this change to the query.
-if (Bugzilla->params->{"usebugaliases"} && defined $cgi->param('alias')) {
-    my $alias = trim($cgi->param('alias'));
-    
-    # Since aliases are unique (like bug numbers), they can only be changed
-    # for one bug at a time, so ignore the alias change unless only a single
-    # bug is being changed.
-    if (scalar(@idlist) == 1) {
-        # Add the alias change to the query.  If the field contains the blank 
-        # value, make the field be NULL to indicate that the bug has no alias.
-        # Otherwise, if the field contains a value, update the record 
-        # with that value.
-        DoComma();
-        if ($alias ne "") {
-            ValidateBugAlias($alias, $idlist[0]);
-            $::query .= "alias = ?";
-            push(@values, $alias);
-        } else {
-            $::query .= "alias = NULL";
-        }
-    }
+# Since aliases are unique (like bug numbers), they can only be changed
+# for one bug at a time. So if we're doing a mass-change, we ignore
+# the alias field.
+if (Bugzilla->params->{"usebugaliases"} && defined $cgi->param('alias')
+    && scalar(@bug_objects) == 1)
+{
+    $bug_objects[0]->set_alias($cgi->param('alias'));
 }
 
 # If the user is submitting changes from show_bug.cgi for a single bug,
@@ -1391,7 +1376,7 @@ foreach my $id (@idlist) {
 
             # Bugzilla::Bug does these for us already.
             next if grep($_ eq $col, qw(keywords op_sys rep_platform priority
-                                        bug_severity short_desc
+                                        bug_severity short_desc alias
                                         status_whiteboard bug_file_loc),
                                      Bugzilla->custom_field_names);
 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 05aa0e4b3..7eac20cf2 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -86,10 +86,6 @@
     has already taken the alias <em>[% alias FILTER html %]</em>.
     Please choose another one.
 
-  [% ELSIF error == "alias_not_defined" %]
-    [% title = "Alias Is Not Defined" %]
-    You did not supply an alias to this [% terms.bug %].
-
   [% ELSIF error == "alias_is_numeric" %]
     [% title = "Alias Is Numeric" %]
     You tried to give this [% terms.bug %] the alias <em>[% alias FILTER html %]</em>,