Skip to content

bugzilla
bugzilla
Commit fab5d3a3 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

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
parent 931bd3b6
Hide whitespace changes
Inline Side-by-side
Showing with 136 additions and 106 deletions
Bugzilla/Bug.pm
View file @ fab5d3a3
......@@ -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.
......
Bugzilla/Field/Choice.pm
View file @ fab5d3a3
......@@ -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 });
}
......
Bugzilla/Object.pm
View file @ fab5d3a3
......@@ -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
......
Bugzilla/Product.pm
View file @ fab5d3a3
......@@ -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()');
......
Bugzilla/Status.pm
View file @ fab5d3a3
......@@ -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 #
##############
......
editusers.cgi
View file @ fab5d3a3
......@@ -238,7 +238,7 @@ if ($action eq 'search') {
# Update profiles table entry; silently skip doing this if the user
# is not authorized.
my %changes;
my $changes = {};
if ($editusers) {
$otherUser->set_login($cgi->param('login'));
$otherUser->set_name($cgi->param('name'));
......@@ -246,7 +246,7 @@ if ($action eq 'search') {
if $cgi->param('password');
$otherUser->set_disabledtext($cgi->param('disabledtext'));
$otherUser->set_disable_mail($cgi->param('disable_mail'));
%changes = %{$otherUser->update()};
$changes = $otherUser->update();
}
# Update group settings.
......@@ -334,7 +334,7 @@ if ($action eq 'search') {
delete_token($token);
$vars->{'message'} = 'account_updated';
$vars->{'changed_fields'} = [keys %changes];
$vars->{'changed_fields'} = [keys %$changes];
$vars->{'groups_added_to'} = \@groupsAddedTo;
$vars->{'groups_removed_from'} = \@groupsRemovedFrom;
$vars->{'groups_granted_rights_to_bless'} = \@groupsGrantedRightsToBless;
......
editvalues.cgi
View file @ fab5d3a3
......@@ -356,91 +356,15 @@ if ($action eq 'edit') {
#
if ($action eq 'update') {
check_token_data($token, 'edit_field_value');
my $valueold = trim($cgi->param('valueold') || '');
my $sortkeyold = trim($cgi->param('sortkeyold') || '0');
ValueMustExist($field, $valueold);
trick_taint($valueold);
$vars->{'value'} = $value;
# If the value cannot be renamed, throw an error.
if (lsearch($static{$field}, $valueold) >= 0 && $value ne $valueold) {
$vars->{'old_value'} = $valueold;
ThrowUserError('fieldvalue_not_editable', $vars);
}
if (length($value) > 60) {
ThrowUserError('fieldvalue_name_too_long', $vars);
}
$dbh->bz_start_transaction();
# Need to store because detaint_natural() will delete this if
# invalid
my $stored_sortkey = $sortkey;
if ($sortkey != $sortkeyold) {
if (!detaint_natural($sortkey)) {
ThrowUserError('fieldvalue_sortkey_invalid',
{'name' => $field,
'sortkey' => $stored_sortkey});
}
$dbh->do("UPDATE $field SET sortkey = ? WHERE value = ?",
undef, $sortkey, $valueold);
$vars->{'updated_sortkey'} = 1;
$vars->{'sortkey'} = $sortkey;
}
if ($value ne $valueold) {
unless ($value) {
ThrowUserError('fieldvalue_undefined');
}
if (ValueExists($field, $value)) {
ThrowUserError('fieldvalue_already_exists', $vars);
}
if ($field eq 'bug_status'
&& (grep { lc($value) eq $_ } SPECIAL_STATUS_WORKFLOW_ACTIONS))
{
$vars->{'value'} = $value;
ThrowUserError('fieldvalue_reserved_word', $vars);
}
trick_taint($value);
if ($field_obj->type != FIELD_TYPE_MULTI_SELECT) {
$dbh->do("UPDATE bugs SET $field = ? WHERE $field = ?",
undef, $value, $valueold);
}
else {
$dbh->do("UPDATE bug_$field SET value = ? WHERE value = ?",
undef, $value, $valueold);
}
$dbh->do("UPDATE $field SET value = ? WHERE value = ?",
undef, $value, $valueold);
$vars->{'updated_value'} = 1;
}
$dbh->bz_commit_transaction();
# If the old value was the default value for the field,
# update data/params accordingly.
# This update is done while tables are unlocked due to the
# annoying calls in Bugzilla/Config/Common.pm.
if (defined $defaults{$field}
&& $value ne $valueold
&& $valueold eq Bugzilla->params->{$defaults{$field}})
{
SetParam($defaults{$field}, $value);
write_params();
$vars->{'default_value_updated'} = 1;
}
$vars->{'value'} = $cgi->param('valueold');
my $value_obj = Bugzilla::Field::Choice->type($field_obj)
->check($cgi->param('valueold'));
$value_obj->set_name($value);
$value_obj->set_sortkey($sortkey);
$vars->{'changes'} = $value_obj->update();
delete_token($token);
$vars->{'value_obj'} = $value_obj;
$vars->{'message'} = 'field_value_updated';
display_field_values();
}
......
template/en/default/global/messages.html.tmpl
View file @ fab5d3a3
......@@ -314,20 +314,23 @@
[% ELSIF message_tag == "field_value_updated" %]
[% title = "Field Value Updated" %]
[% IF updated_value || updated_sortkey %]
Changes to the <em>[% value FILTER html %]</em> value of the
[% IF changes.keys.size %]
The <em>[% value FILTER html %]</em> value of the
<em>[% field.description FILTER html %]</em>
(<em>[% field.name FILTER html %]</em>) field have been changed:
(<em>[% field.name FILTER html %]</em>) field has been changed:
<ul>
[% IF updated_value %]
<li>Field value updated to <em>[% value FILTER html %]</em></li>
[% IF default_value_updated %]
(note that this value is the default for this field. All
references to the default value will now point to this new value)
[% END %]
[% IF changes.value %]
<li>Field value updated to
<em>[% changes.value.1 FILTER html %]</em>.
[% IF value_obj.is_default %]
(Note that this value is the default for this field. All
references to the default value will now point to this new value.)
[% END %]
</li>
[% END %]
[% IF updated_sortkey %]
<li>Field value sortkey updated to <em>[% sortkey FILTER html %]</em></li>
[% IF changes.sortkey %]
<li>Sortkey updated to
<em>[% changes.sortkey.1 FILTER html %]</em>.</li>
[% END %]
</ul>
[% ELSE %]
......
template/en/default/global/user-error.html.tmpl
View file @ fab5d3a3
......@@ -457,7 +457,8 @@
[% ELSIF error == "fieldvalue_not_editable" %]
[% title = "Field Value Not Editable" %]
The value '[% old_value FILTER html %]' cannot be renamed because
it plays some special role for the '[% field.description FILTER html %]' field.
it plays some special role for the '[% field.description FILTER html %]'
field.
[% ELSIF error == "fieldvalue_not_deletable" %]
[% title = "Field Value Not Deletable" %]
......@@ -470,7 +471,7 @@
[% ELSIF error == "fieldvalue_reserved_word" %]
[% title = "Reserved Word Not Allowed" %]
You cannot use the '[% value FILTER html %]' value for the
You cannot use the value '[% value FILTER html %]' for the
'[% field.description FILTER html %]' field. This value is used internally.
Please choose another one.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment