Skip to content

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

Bug 287330: Multi-Select Custom Fields

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
parent 16336299
Show whitespace changes
Inline Side-by-side
Showing with 207 additions and 42 deletions
Bugzilla/Bug.pm
View file @ aa888f22
......@@ -70,6 +70,9 @@ use constant LIST_ORDER => ID_FIELD;
# This is a sub because it needs to call other subroutines.
sub DB_COLUMNS {
my $dbh = Bugzilla->dbh;
my @custom = Bugzilla->get_fields({ custom => 1, obsolete => 0});
@custom = grep {$_->type != FIELD_TYPE_MULTI_SELECT} @custom;
my @custom_names = map {$_->name} @custom;
return qw(
alias
bug_file_loc
......@@ -98,7 +101,7 @@ sub DB_COLUMNS {
'qa_contact AS qa_contact_id',
$dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts',
$dbh->sql_date_format('deadline', '%Y-%m-%d') . ' AS deadline',
Bugzilla->custom_field_names;
@custom_names;
}
use constant REQUIRED_CREATE_FIELDS => qw(
......@@ -140,6 +143,9 @@ sub VALIDATORS {
if ($field->type == FIELD_TYPE_SINGLE_SELECT) {
$validator = \&_check_select_field;
}
elsif ($field->type == FIELD_TYPE_MULTI_SELECT) {
$validator = \&_check_multi_select_field;
}
else {
$validator = \&_check_freetext_field;
}
......@@ -157,6 +163,9 @@ use constant UPDATE_VALIDATORS => {
};
sub UPDATE_COLUMNS {
my @custom = Bugzilla->get_fields({ custom => 1, obsolete => 0});
@custom = grep {$_->type != FIELD_TYPE_MULTI_SELECT} @custom;
my @custom_names = map {$_->name} @custom;
my @columns = qw(
alias
cclist_accessible
......@@ -175,7 +184,7 @@ sub UPDATE_COLUMNS {
short_desc
status_whiteboard
);
push(@columns, Bugzilla->custom_field_names);
push(@columns, @custom_names);
return @columns;
};
......@@ -303,14 +312,10 @@ sub create {
# These are not a fields in the bugs table, so we don't pass them to
# insert_create_data.
my $cc_ids = $params->{cc};
delete $params->{cc};
my $groups = $params->{groups};
delete $params->{groups};
my $depends_on = $params->{dependson};
delete $params->{dependson};
my $blocked = $params->{blocked};
delete $params->{blocked};
my $cc_ids = delete $params->{cc};
my $groups = delete $params->{groups};
my $depends_on = delete $params->{dependson};
my $blocked = delete $params->{blocked};
my ($comment, $privacy) = ($params->{comment}, $params->{commentprivacy});
delete $params->{comment};
delete $params->{commentprivacy};
......@@ -322,9 +327,9 @@ sub create {
# We don't want the bug to appear in the system until it's correctly
# protected by groups.
my $timestamp = $params->{creation_ts};
delete $params->{creation_ts};
my $timestamp = delete $params->{creation_ts};
my $ms_values = $class->_extract_multi_selects($params);
my $bug = $class->insert_create_data($params);
# Add the group restrictions
......@@ -372,6 +377,16 @@ sub create {
$sth_bug_time->execute($timestamp, $blocked_id);
}
# Insert the values into the multiselect value tables
foreach my $field (keys %$ms_values) {
$dbh->do("DELETE FROM bug_$field where bug_id = ?",
undef, $bug->bug_id);
foreach my $value ( @{$ms_values->{$field}} ) {
$dbh->do("INSERT INTO bug_$field (bug_id, value) VALUES (?,?)",
undef, $bug->bug_id, $value);
}
}
$dbh->bz_commit_transaction();
# Because MySQL doesn't support transactions on the longdescs table,
......@@ -469,6 +484,7 @@ sub update {
my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()");
$self->{delta_ts} = $delta_ts;
my $old_bug = $self->new($self->id);
my $changes = $self->SUPER::update(@_);
foreach my $comment (@{$self->{added_comments} || []}) {
......@@ -480,6 +496,24 @@ sub update {
$self->bug_id, Bugzilla->user->id, $delta_ts, @values);
}
# Insert the values into the multiselect value tables
my @multi_selects = Bugzilla->get_fields(
{ custom => 1, type => FIELD_TYPE_MULTI_SELECT, obsolete => 0 });
foreach my $field (@multi_selects) {
my $name = $field->name;
my ($removed, $added) = diff_arrays($old_bug->$name, $self->$name);
if (scalar @$removed || scalar @$added) {
$changes->{$name} = [join(', ', @$removed), join(', ', @$added)];
$dbh->do("DELETE FROM bug_$name where bug_id = ?",
undef, $self->id);
foreach my $value (@{$self->$name}) {
$dbh->do("INSERT INTO bug_$name (bug_id, value) VALUES (?,?)",
undef, $self->id, $value);
}
}
}
# Log bugs_activity items
# XXX Eventually, when bugs_activity is able to track the dupe_id,
# this code should go below the duplicates-table-updating code below.
......@@ -504,6 +538,25 @@ sub update {
return $changes;
}
# Used by create().
# We need to handle multi-select fields differently than normal fields,
# because they're arrays and don't go into the bugs table.
sub _extract_multi_selects {
my ($invocant, $params) = @_;
my @multi_selects = Bugzilla->get_fields(
{ custom => 1, type => FIELD_TYPE_MULTI_SELECT, obsolete => 0 });
my %ms_values;
foreach my $field (@multi_selects) {
my $name = $field->name;
if (exists $params->{$name}) {
my $array = delete($params->{$name}) || [];
$ms_values{$name} = $array;
}
}
return \%ms_values;
}
# XXX Temporary hack until all of process_bug uses update().
sub update_cc {
my $self = shift;
......@@ -1171,6 +1224,19 @@ sub _check_work_time {
return $_[0]->_check_time($_[1], 'work_time');
}
# Custom Field Validators
sub _check_multi_select_field {
my ($invocant, $values, $field) = @_;
return [] if !$values;
foreach my $value (@$values) {
$value = trim($value);
check_field($field, $value);
trick_taint($value);
}
return $values;
}
sub _check_select_field {
my ($invocant, $value, $field) = @_;
$value = trim($value);
......@@ -1233,6 +1299,9 @@ sub set_alias { $_[0]->set('alias', $_[1]); }
sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); }
sub set_custom_field {
my ($self, $field, $value) = @_;
if (ref $value eq 'ARRAY' && !$field->type == FIELD_TYPE_MULTI_SELECT) {
$value = $value->[0];
}
ThrowCodeError('field_not_custom', { field => $field }) if !$field->custom;
$self->set($field->name, $value);
}
......@@ -2670,6 +2739,9 @@ sub check_can_change_field {
# Return true if they haven't changed this field at all.
if ($oldvalue eq $newvalue) {
return 1;
} elsif (ref($newvalue) eq 'ARRAY' && ref($oldvalue) eq 'ARRAY') {
my ($removed, $added) = diff_arrays($oldvalue, $newvalue);
return 1 if !scalar(@$removed) && !scalar(@$added);
} elsif (trim($oldvalue) eq trim($newvalue)) {
return 1;
# numeric fields need to be compared using ==
......@@ -2941,11 +3013,19 @@ sub AUTOLOAD {
no strict 'refs';
*$AUTOLOAD = sub {
my $self = shift;
if (defined $self->{$attr}) {
return $self->{$attr} if defined $self->{$attr};
$self->{_multi_selects} ||= [Bugzilla->get_fields(
{custom => 1, type => FIELD_TYPE_MULTI_SELECT })];
if ( grep($_->name eq $attr, @{$self->{_multi_selects}}) ) {
$self->{$attr} ||= Bugzilla->dbh->selectcol_arrayref(
"SELECT value FROM bug_$attr WHERE bug_id = ? ORDER BY value",
undef, $self->id);
return $self->{$attr};
} else {
return '';
}
return '';
};
goto &$AUTOLOAD;
......
Bugzilla/Constants.pm
View file @ aa888f22
......@@ -119,6 +119,7 @@ use File::Basename;
FIELD_TYPE_UNKNOWN
FIELD_TYPE_FREETEXT
FIELD_TYPE_SINGLE_SELECT
FIELD_TYPE_MULTI_SELECT
USAGE_MODE_BROWSER
USAGE_MODE_CMDLINE
......@@ -340,6 +341,7 @@ use constant SENDMAIL_PATH => '/usr/lib:/usr/sbin:/usr/ucblib';
use constant FIELD_TYPE_UNKNOWN => 0;
use constant FIELD_TYPE_FREETEXT => 1;
use constant FIELD_TYPE_SINGLE_SELECT => 2;
use constant FIELD_TYPE_MULTI_SELECT => 3;
# The maximum number of days a token will remain valid.
use constant MAX_TOKEN_AGE => 3;
......
Bugzilla/DB.pm
View file @ aa888f22
......@@ -642,9 +642,8 @@ sub _bz_add_table_raw {
$self->do($_) foreach (@statements);
}
sub bz_add_field_table {
my ($self, $name) = @_;
my $table_schema = $self->_bz_schema->FIELD_TABLE_SCHEMA;
sub _bz_add_field_table {
my ($self, $name, $table_schema) = @_;
# We do nothing if the table already exists.
return if $self->bz_table_info($name);
my $indexes = $table_schema->{INDEXES};
......@@ -659,6 +658,19 @@ sub bz_add_field_table {
$self->bz_add_table($name);
}
sub bz_add_field_tables {
my ($self, $field) = @_;
$self->_bz_add_field_table($field->name,
$self->_bz_schema->FIELD_TABLE_SCHEMA);
if ( $field->type == FIELD_TYPE_MULTI_SELECT ) {
$self->_bz_add_field_table('bug_' . $field->name,
$self->_bz_schema->MULTI_SELECT_VALUE_TABLE);
}
}
sub bz_drop_column {
my ($self, $table, $column) = @_;
......
Bugzilla/DB/Schema.pm
View file @ aa888f22
......@@ -1254,6 +1254,18 @@ use constant FIELD_TABLE_SCHEMA => {
sortkey_idx => ['sortkey', 'value'],
],
};
use constant MULTI_SELECT_VALUE_TABLE => {
FIELDS => [
bug_id => {TYPE => 'INT3', NOTNULL => 1},
value => {TYPE => 'varchar(64)', NOTNULL => 1},
],
INDEXES => [
bug_id_idx => {FIELDS => [qw( bug_id value)], TYPE => 'UNIQUE'},
],
};
#--------------------------------------------------------------------------
=head1 METHODS
......
Bugzilla/Field.pm
View file @ aa888f22
......@@ -252,9 +252,9 @@ sub _check_sortkey {
sub _check_type {
my ($invocant, $type) = @_;
my $saved_type = $type;
# FIELD_TYPE_SINGLE_SELECT here should be updated every time a new,
# The constant here should be updated every time a new,
# higher field type is added.
(detaint_natural($type) && $type <= FIELD_TYPE_SINGLE_SELECT)
(detaint_natural($type) && $type <= FIELD_TYPE_MULTI_SELECT)
|| ThrowCodeError('invalid_customfield_type', { type => $saved_type });
return $type;
}
......@@ -454,13 +454,20 @@ sub create {
if ($field->custom) {
my $name = $field->name;
my $type = $field->type;
if (SQL_DEFINITIONS->{$type}) {
# Create the database column that stores the data for this field.
$dbh->bz_add_column('bugs', $name, SQL_DEFINITIONS->{$type});
}
if ($type == FIELD_TYPE_SINGLE_SELECT) {
if ($type == FIELD_TYPE_SINGLE_SELECT
|| $type == FIELD_TYPE_MULTI_SELECT)
{
# Create the table that holds the legal values for this field.
$dbh->bz_add_field_table($name);
# And insert a default value of "---" into it.
$dbh->bz_add_field_tables($field);
}
if ($type == FIELD_TYPE_SINGLE_SELECT) {
# Insert a default value of "---" into the legal values table.
$dbh->do("INSERT INTO $name (value) VALUES ('---')");
}
}
......
colchange.cgi
View file @ aa888f22
......@@ -80,7 +80,9 @@ if (Bugzilla->user->in_group(Bugzilla->params->{"timetrackinggroup"})) {
push(@masterlist, ("short_desc", "short_short_desc"));
push(@masterlist, Bugzilla->custom_field_names);
my @custom_fields = grep { $_->type != FIELD_TYPE_MULTI_SELECT }
Bugzilla->get_fields({ custom => 1, obsolete => 0 });
push(@masterlist, map { $_->name } @custom_fields);
$vars->{'masterlist'} = \@masterlist;
......
editvalues.cgi
View file @ aa888f22
......@@ -41,6 +41,8 @@ our @valid_fields = ('op_sys', 'rep_platform', 'priority', 'bug_severity',
# Add custom select fields.
my @custom_fields = Bugzilla->get_fields({custom => 1,
type => FIELD_TYPE_SINGLE_SELECT});
push(@custom_fields, Bugzilla->get_fields({custom => 1,
type => FIELD_TYPE_MULTI_SELECT}));
push(@valid_fields, map { $_->name } @custom_fields);
......
post_bug.cgi
View file @ aa888f22
......@@ -120,8 +120,8 @@ $template->process($format->{'template'}, $vars, \$comment)
|| ThrowTemplateError($template->error());
# Include custom fields editable on bug creation.
my @custom_bug_fields = Bugzilla->get_fields(
{ custom => 1, obsolete => 0, enter_bug => 1 });
my @custom_bug_fields = grep {$_->type != FIELD_TYPE_MULTI_SELECT}
Bugzilla->get_fields({ custom => 1, obsolete => 0, enter_bug => 1 });
# Undefined custom fields are ignored to ensure they will get their default
# value (e.g. "---" for custom single select fields).
......@@ -167,6 +167,13 @@ $bug_params{'cc'} = [$cgi->param('cc')];
$bug_params{'groups'} = \@selected_groups;
$bug_params{'comment'} = $comment;
my @multi_selects = Bugzilla->get_fields(
{ type => FIELD_TYPE_MULTI_SELECT, custom => 1, obsolete => 0,
enter_bug => 1 });
foreach my $field (@multi_selects) {
$bug_params{$field->name} = [$cgi->param($field->name)];
}
my $bug = Bugzilla::Bug->create(\%bug_params);
# Get the bug ID back.
......
process_bug.cgi
View file @ aa888f22
......@@ -571,11 +571,11 @@ foreach my $field ("version", "target_milestone") {
# Add custom fields data to the query that will update the database.
foreach my $field (Bugzilla->get_fields({custom => 1, obsolete => 0})) {
my $fname = $field->name;
if (defined $cgi->param($fname)
if ( (defined $cgi->param($fname) || defined $cgi->param("defined_$fname"))
&& (!$cgi->param('dontchange')
|| $cgi->param($fname) ne $cgi->param('dontchange')))
{
$_->set_custom_field($field, $cgi->param($fname)) foreach @bug_objects;
$_->set_custom_field($field, [$cgi->param($fname)]) foreach @bug_objects;
}
}
......@@ -1010,6 +1010,11 @@ foreach my $id (@idlist) {
my $bug_changed = 0;
my $write = "WRITE"; # Might want to make a param to control
# whether we do LOW_PRIORITY ...
my @multi_select_locks = map {'bug_' . $_->name . " $write"}
Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT,
obsolete => 0 });
$dbh->bz_lock_tables("bugs $write", "bugs_activity $write", "cc $write",
"profiles READ", "dependencies $write", "votes $write",
"products READ", "components READ", "milestones READ",
......@@ -1020,7 +1025,8 @@ foreach my $id (@idlist) {
"keyworddefs READ", "groups READ", "attachments READ",
"bug_status READ", "group_control_map AS oldcontrolmap READ",
"group_control_map AS newcontrolmap READ",
"group_control_map READ", "email_setting READ", "classifications READ");
"group_control_map READ", "email_setting READ",
"classifications READ", @multi_select_locks);
# It may sound crazy to set %formhash for each bug as $cgi->param()
# will not change, but %formhash is modified below and we prefer
......
query.cgi
View file @ aa888f22
......@@ -253,10 +253,21 @@ $vars->{'priority'} = get_legal_field_values('priority');
$vars->{'bug_severity'} = get_legal_field_values('bug_severity');
# Boolean charts
my @fields;
push(@fields, { name => "noop", description => "---" });
push(@fields, $dbh->bz_get_field_defs());
@fields = sort {lc($a->{'description'}) cmp lc($b->{'description'})} @fields;
my @fields = Bugzilla->get_fields({ obsolete => 0 });
# Multi-selects aren't searchable, currently.
@fields = grep($_->type != FIELD_TYPE_MULTI_SELECT, @fields);
# If we're not in the time-tracking group, exclude time-tracking fields.
if (!Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
foreach my $tt_field (qw(estimated_time remaining_time work_time
percentage_complete deadline))
{
@fields = grep($_->name ne $tt_field, @fields);
}
}
@fields = sort {lc($a->description) cmp lc($b->description)} @fields;
unshift(@fields, { name => "noop", description => "---" });
$vars->{'fields'} = \@fields;
# Creating new charts - if the cmd-add value is there, we define the field
......
template/en/default/admin/custom_fields/edit.html.tmpl
View file @ aa888f22
......@@ -84,7 +84,8 @@
<th>&nbsp;</th>
<td>&nbsp;</td>
</tr>
[% IF field.type == constants.FIELD_TYPE_SINGLE_SELECT %]
[% IF field.type == constants.FIELD_TYPE_SINGLE_SELECT
|| field.type == constants.FIELD_TYPE_MULTI_SELECT %]
<tr>
<th>&nbsp;</th>
<td colspan="3">
......
template/en/default/bug/field.html.tmpl
View file @ aa888f22
......@@ -42,21 +42,42 @@
[% SWITCH field.type %]
[% CASE constants.FIELD_TYPE_FREETEXT %]
<input name="[% field.name FILTER html %]" value="[% value FILTER html %]" size="60">
[% CASE constants.FIELD_TYPE_SINGLE_SELECT %]
<select id="[% field.name FILTER html %]" name="[% field.name FILTER html %]">
[% CASE [ constants.FIELD_TYPE_SINGLE_SELECT
constants.FIELD_TYPE_MULTI_SELECT ] %]
<select id="[% field.name FILTER html %]"
name="[% field.name FILTER html %]"
[% IF field.type == constants.FIELD_TYPE_MULTI_SELECT %]
[% SET field_size = 5 %]
[% IF field.legal_values.size < 5 %]
[% SET field_size = field.legal_values.size %]
[% END %]
size="[% field_size FILTER html %]" multiple="multiple"
[% END %]
>
[% IF allow_dont_change %]
<option value="[% dontchange FILTER html %]">
<option value="[% dontchange FILTER html %]"
[% ' selected="selected"' IF value == dontchange %]>
[% dontchange FILTER html %]
</option>
[% END %]
[% FOREACH legal_value = field.legal_values %]
<option value="[% legal_value FILTER html %]"
[%- " selected=\"selected\"" IF value == legal_value %]>
[%- " selected=\"selected\"" IF value.contains(legal_value).size %]>
[%- legal_value FILTER html %]</option>
[% END %]
</select>
[%# When you pass an empty multi-select in the web interface,
# it doesn't appear at all in the CGI object. Instead of
# forcing all users of process_bug to always specify every
# multi-select, we have this field defined if the multi-select
# field is defined, and then if this is passed but the multi-select
# isn't, we know that the multi-select was emptied.
%]
[% IF field.type == constants.FIELD_TYPE_MULTI_SELECT %]
<input type="hidden" name="defined_[% field.name FILTER html %]">
[% END %]
[% END %]
[% ELSE %]
[% value FILTER html %]
[% value.join(', ') FILTER html %]
[% END %]
</td>
template/en/default/global/field-descs.none.tmpl
View file @ aa888f22
......@@ -80,7 +80,9 @@
[% field_types = { ${constants.FIELD_TYPE_UNKNOWN} => "Unknown Type",
${constants.FIELD_TYPE_FREETEXT} => "Free Text",
${constants.FIELD_TYPE_SINGLE_SELECT} => "Drop Down" } %]
${constants.FIELD_TYPE_SINGLE_SELECT} => "Drop Down",
${constants.FIELD_TYPE_MULTI_SELECT} => "Multiple-Selection Box",
} %]
[% status_descs = { "UNCONFIRMED" => "UNCONFIRMED",
"NEW" => "NEW",
......
template/en/default/global/user-error.html.tmpl
View file @ aa888f22
......@@ -661,10 +661,10 @@
You tried to change the
<strong>[% field_descs.$field FILTER html %]</strong> field
[% IF oldvalue.defined %]
from <em>[% oldvalue FILTER html %]</em>
from <em>[% oldvalue.join(', ') FILTER html %]</em>
[% END %]
[% IF newvalue.defined %]
to <em>[% newvalue FILTER html %]</em>
to <em>[% newvalue.join(', ') FILTER html %]</em>
[% END %]
, but only
[% IF privs < 3 %]
......
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