From aa888f2218179d59b4f0b8e51e43b863f1da3e43 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Sat, 8 Sep 2007 05:14:25 +0000 Subject: Bug 287330: Multi-Select Custom Fields Patch By Max Kanat-Alexander r=LpSolit, a=LpSolit --- Bugzilla/Bug.pm | 110 +++++++++++++++++++++++++++++++++++++++++++------- Bugzilla/Constants.pm | 2 + Bugzilla/DB.pm | 18 +++++++-- Bugzilla/DB/Schema.pm | 12 ++++++ Bugzilla/Field.pm | 21 ++++++---- 5 files changed, 138 insertions(+), 25 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 7ed76311f..631c77caf 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -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; diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index a5fbba61d..4406d7415 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -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; diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 7384e7b5f..4aad803c6 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -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) = @_; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index f8c1588e4..1c6ee352e 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -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 diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 1830784a9..6555bba96 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -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; - # Create the database column that stores the data for this field. - $dbh->bz_add_column('bugs', $name, SQL_DEFINITIONS->{$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 ('---')"); } } -- cgit v1.2.3-24-g4f1b