Bug 455641: Implement Bugzilla::Field::Choice->update and have editvalues.cgi use it

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

5 files changed, 109 insertions, 7 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index d620f222c..d3aa1eeec 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -581,8 +581,7 @@ sub update {
     # inside this function.
     my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()");
 
-    my $old_bug = $self->new($self->id);
-    my $changes = $self->SUPER::update(@_);
+    my ($changes, $old_bug) = $self->SUPER::update(@_);
 
     # Certain items in $changes have to be fixed so that they hold
     # a name instead of an ID.
diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm
index 7705258eb..9db5c5efd 100644
--- a/Bugzilla/Field/Choice.pm
+++ b/Bugzilla/Field/Choice.pm
@@ -24,6 +24,7 @@ package Bugzilla::Field::Choice;
 
 use base qw(Bugzilla::Object);
 
+use Bugzilla::Config qw(SetParam write_params);
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Field;
@@ -41,6 +42,11 @@ use constant DB_COLUMNS => qw(
     sortkey
 );
 
+use constant UPDATE_COLUMNS => qw(
+    value
+    sortkey
+);
+
 use constant NAME_FIELD => 'value';
 use constant LIST_ORDER => 'sortkey, value';
 
@@ -55,6 +61,13 @@ use constant CLASS_MAP => {
     bug_status => 'Bugzilla::Status',
 };
 
+use constant DEFAULT_MAP => {
+    op_sys       => 'defaultopsys',
+    rep_platform => 'defaultplatform',
+    priority     => 'defaultpriority',
+    bug_severity => 'defaultseverity',
+};
+
 #################
 # Class Factory #
 #################
@@ -127,6 +140,37 @@ sub create {
     return $class->SUPER::create(@_);
 }
 
+sub update {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+    my $fname = $self->field->name;
+
+    $dbh->bz_start_transaction();
+
+    my ($changes, $old_self) = $self->SUPER::update(@_);
+    if (exists $changes->{value}) {
+        my ($old, $new) = @{ $changes->{value} };
+        if ($self->field->type == FIELD_TYPE_MULTI_SELECT) {
+            $dbh->do("UPDATE bug_$fname SET value = ? WHERE value = ?",
+                     undef, $new, $old);
+        }
+        else {
+            $dbh->do("UPDATE bugs SET $fname = ? WHERE $fname = ?",
+                     undef, $new, $old);
+        }
+
+        if ($old_self->is_default) {
+            my $param = $self->DEFAULT_MAP->{$self->field->name};
+            SetParam($param, $self->name);
+            write_params();
+        }
+    }
+
+    $dbh->bz_commit_transaction();
+    return $changes;
+}
+
+
 #############
 # Accessors #
 #############
@@ -143,6 +187,35 @@ sub field {
     return $cache->{"field_$class"};
 }
 
+sub is_default {
+    my $self = shift;
+    my $param_value = 
+        Bugzilla->params->{ $self->DEFAULT_MAP->{$self->field->name} };
+    return 0 if !defined $param_value;
+    return $self->name eq $param_value ? 1 : 0;
+}
+
+sub is_static {
+    my $self = shift;
+    # If we need to special-case Resolution for *anything* else, it should
+    # get its own subclass.
+    if ($self->field->name eq 'resolution') {
+        return grep($_ eq $self->name, ('', 'FIXED', 'MOVED', 'DUPLICATE'))
+               ? 1 : 0;
+    }
+    elsif ($self->field->custom) {
+        return $self->name eq '---' ? 1 : 0;
+    }
+    return 0;
+}
+
+############
+# Mutators #
+############
+
+sub set_name    { $_[0]->set('value', $_[1]);   }
+sub set_sortkey { $_[0]->set('sortkey', $_[1]); }
+
 ##############
 # Validators #
 ##############
@@ -153,12 +226,21 @@ sub _check_value {
     my $field = $invocant->field;
 
     $value = trim($value);
+
+    # Make sure people don't rename static values
+    if (blessed($invocant) && $value ne $invocant->name 
+        && $invocant->is_static) 
+    {
+        ThrowUserError('fieldvalue_not_editable',
+                       { field => $field, old_value => $invocant->name });
+    }
+
     ThrowUserError('fieldvalue_undefined') if !defined $value || $value eq "";
     ThrowUserError('fieldvalue_name_too_long', { value => $value })
         if length($value) > MAX_FIELD_VALUE_SIZE;
 
     my $exists = $invocant->type($field)->new({ name => $value });
-    if ($exists) {
+    if ($exists && (!blessed($invocant) || $invocant->id != $exists->id)) {
         ThrowUserError('fieldvalue_already_exists', 
                        { field => $field, value => $value });
     }
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index bcd436484..cde440b95 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -283,6 +283,10 @@ sub update {
 
     $dbh->bz_commit_transaction();
 
+    if (wantarray) {
+        return (\%changes, $old_self);
+    }
+
     return \%changes;
 }
 
@@ -703,6 +707,8 @@ updated, and they will only be updated if their values have changed.
 
 =item B<Returns>
 
+B<In scalar context:>
+
 A hashref showing what changed during the update. The keys are the column
 names from L</UPDATE_COLUMNS>. If a field was not changed, it will not be
 in the hash at all. If the field was changed, the key will point to an arrayref.
@@ -711,6 +717,13 @@ will be the new value.
 
 If there were no changes, we return a reference to an empty hash.
 
+B<In array context:>
+
+Returns a list, where the first item is the above hashref. The second item
+is the object as it was in the database before update() was called. (This
+is mostly useful to subclasses of C<Bugzilla::Object> that are implementing
+C<update>.)
+
 =back
 
 =back
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index 235a06926..8fb7b7ca9 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -147,7 +147,7 @@ sub update {
 
     # Don't update the DB if something goes wrong below -> transaction.
     $dbh->bz_start_transaction();
-    my $changes = $self->SUPER::update(@_);
+    my ($changes, $old_self) = $self->SUPER::update(@_);
 
     # We also have to fix votes.
     my @msgs; # Will store emails to send to voters.
@@ -247,7 +247,7 @@ sub update {
         require Bugzilla::Bug;
         import Bugzilla::Bug qw(LogActivityEntry);
 
-        my $old_settings = $self->new($self->id)->group_controls;
+        my $old_settings = $old_self->group_controls;
         my $new_settings = $self->group_controls;
         my $timestamp = $dbh->selectrow_array('SELECT NOW()');
 
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index 5f37974e7..565433850 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -77,11 +77,19 @@ sub create {
 #####     Accessors        ####
 ###############################
 
-sub name      { return $_[0]->{'value'};    }
-sub sortkey   { return $_[0]->{'sortkey'};  }
 sub is_active { return $_[0]->{'isactive'}; }
 sub is_open   { return $_[0]->{'is_open'};  }
 
+sub is_static {
+    my $self = shift;
+    if ($self->name eq 'UNCONFIRMED'
+        || $self->name eq Bugzilla->params->{'duplicate_or_move_bug_status'}) 
+    {
+        return 1;
+    }
+    return 0;
+}
+
 ##############
 # Validators #
 ##############