From 63be194996849202878c4a87e4c68a25d1976d3e Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Fri, 7 Nov 2008 17:34:39 +0000 Subject: Bug 308253: Ability to add select (enum) fields to a bug whose list of values depends on the value of another field Patch By Max Kanat-Alexander r=bbaetz, a=mkanat --- Bugzilla/DB/Schema.pm | 100 ++++++++++++++++++----------------------------- Bugzilla/Field.pm | 82 ++++++++++++++++++++++++++++++++++++-- Bugzilla/Field/Choice.pm | 48 ++++++++++++++++++++++- Bugzilla/Install/DB.pm | 39 +++++++++++++++++- Bugzilla/Status.pm | 11 ++---- 5 files changed, 204 insertions(+), 76 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 778db72c5..ed1245d98 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -210,6 +210,26 @@ use constant SCHEMA_VERSION => '2.00'; use constant ADD_COLUMN => 'ADD COLUMN'; # This is a reasonable default that's true for both PostgreSQL and MySQL. use constant MAX_IDENTIFIER_LEN => 63; + +use constant FIELD_TABLE_SCHEMA => { + FIELDS => [ + id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, + PRIMARYKEY => 1}, + value => {TYPE => 'varchar(64)', NOTNULL => 1}, + sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, + isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, + DEFAULT => 'TRUE'}, + visibility_value_id => {TYPE => 'INT2'}, + ], + # Note that bz_add_field_table should prepend the table name + # to these index names. + INDEXES => [ + value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, + sortkey_idx => ['sortkey', 'value'], + visibility_value_id_idx => ['visibility_value_id'], + ], +}; + use constant ABSTRACT_SCHEMA => { # BUG-RELATED TABLES @@ -638,11 +658,15 @@ use constant ABSTRACT_SCHEMA => { REFERENCES => {TABLE => 'fielddefs', COLUMN => 'id'}}, visibility_value_id => {TYPE => 'INT2'}, + value_field_id => {TYPE => 'INT3', + REFERENCES => {TABLE => 'fielddefs', + COLUMN => 'id'}}, ], INDEXES => [ fielddefs_name_idx => {FIELDS => ['name'], TYPE => 'UNIQUE'}, fielddefs_sortkey_idx => ['sortkey'], + fielddefs_value_field_id_idx => ['value_field_id'], ], }, @@ -688,98 +712,65 @@ use constant ABSTRACT_SCHEMA => { bug_status => { FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, + @{ dclone(FIELD_TABLE_SCHEMA->{FIELDS}) }, is_open => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'}, + ], INDEXES => [ bug_status_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, bug_status_sortkey_idx => ['sortkey', 'value'], + bug_status_visibility_value_id_idx => ['visibility_value_id'], ], }, resolution => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], + FIELDS => dclone(FIELD_TABLE_SCHEMA->{FIELDS}), INDEXES => [ resolution_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, resolution_sortkey_idx => ['sortkey', 'value'], + resolution_visibility_value_id_idx => ['visibility_value_id'], ], }, bug_severity => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], + FIELDS => dclone(FIELD_TABLE_SCHEMA->{FIELDS}), INDEXES => [ bug_severity_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, bug_severity_sortkey_idx => ['sortkey', 'value'], + bug_severity_visibility_value_id_idx => ['visibility_value_id'], ], }, priority => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], + FIELDS => dclone(FIELD_TABLE_SCHEMA->{FIELDS}), INDEXES => [ priority_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, priority_sortkey_idx => ['sortkey', 'value'], + priority_visibility_value_id_idx => ['visibility_value_id'], ], }, rep_platform => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], + FIELDS => dclone(FIELD_TABLE_SCHEMA->{FIELDS}), INDEXES => [ rep_platform_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, rep_platform_sortkey_idx => ['sortkey', 'value'], + rep_platform_visibility_value_id_idx => ['visibility_value_id'], ], }, op_sys => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], + FIELDS => dclone(FIELD_TABLE_SCHEMA->{FIELDS}), INDEXES => [ op_sys_value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, op_sys_sortkey_idx => ['sortkey', 'value'], + op_sys_visibility_value_id_idx => ['visibility_value_id'], ], }, @@ -1409,23 +1400,6 @@ use constant ABSTRACT_SCHEMA => { }; -use constant FIELD_TABLE_SCHEMA => { - FIELDS => [ - id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, - PRIMARYKEY => 1}, - value => {TYPE => 'varchar(64)', NOTNULL => 1}, - sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}, - isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, - DEFAULT => 'TRUE'}, - ], - # Note that bz_add_field_table should prepend the table name - # to these index names. - INDEXES => [ - value_idx => {FIELDS => ['value'], TYPE => 'UNIQUE'}, - sortkey_idx => ['sortkey', 'value'], - ], -}; - # Foreign Keys are added in Bugzilla::DB::bz_add_field_tables use constant MULTI_SELECT_VALUE_TABLE => { FIELDS => [ diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index a5e380a11..17459cb2c 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -98,6 +98,7 @@ use constant DB_COLUMNS => qw( enter_bug visibility_field_id visibility_value_id + value_field_id ); use constant REQUIRED_CREATE_FIELDS => qw(name description); @@ -110,10 +111,11 @@ use constant VALIDATORS => { obsolete => \&_check_obsolete, sortkey => \&_check_sortkey, type => \&_check_type, - visibility_field_id => \&_check_control_field, + visibility_field_id => \&_check_visibility_field_id, }; use constant UPDATE_VALIDATORS => { + value_field_id => \&_check_value_field_id, visibility_value_id => \&_check_control_value, }; @@ -125,6 +127,7 @@ use constant UPDATE_COLUMNS => qw( enter_bug visibility_field_id visibility_value_id + value_field_id type ); @@ -292,7 +295,16 @@ sub _check_type { return $type; } -sub _check_control_field { +sub _check_value_field_id { + my ($invocant, $field_id, $is_select) = @_; + $is_select = $invocant->is_select if !defined $is_select; + if ($field_id && !$is_select) { + ThrowUserError('field_value_control_select_only'); + } + return $invocant->_check_visibility_field_id($field_id); +} + +sub _check_visibility_field_id { my ($invocant, $field_id) = @_; $field_id = trim($field_id); return undef if !$field_id; @@ -310,7 +322,7 @@ sub _check_control_field { sub _check_control_value { my ($invocant, $value_id, $field_id) = @_; my $field; - if (blessed($invocant)) { + if (blessed $invocant) { $field = $invocant->visibility_field; } elsif ($field_id) { @@ -528,6 +540,48 @@ sub controls_visibility_of { =pod +=over + +=item C + +The Bugzilla::Field that controls the list of values for this field. + +Returns undef if there is no field that controls this field's visibility. + +=back + +=cut + +sub value_field { + my $self = shift; + if ($self->{value_field_id}) { + $self->{value_field} ||= $self->new($self->{value_field_id}); + } + return $self->{value_field}; +} + +=pod + +=over + +=item C + +An arrayref of C objects, representing fields that this +field controls the values of. + +=back + +=cut + +sub controls_values_of { + my $self = shift; + $self->{controls_values_of} ||= + Bugzilla::Field->match({ value_field_id => $self->id }); + return $self->{controls_values_of}; +} + +=pod + =head2 Instance Mutators These set the particular field that they are named after. @@ -552,6 +606,8 @@ They will throw an error if you try to set the values to something invalid. =item C +=item C + =back =cut @@ -572,6 +628,11 @@ sub set_visibility_value { $self->set('visibility_value_id', $value); delete $self->{visibility_value}; } +sub set_value_field { + my ($self, $value) = @_; + $self->set('value_field_id', $value); + delete $self->{value_field}; +} # This is only used internally by upgrade code in Bugzilla::Field. sub _set_type { $_[0]->set('type', $_[1]); } @@ -734,9 +795,24 @@ sub run_create_validators { $class->_check_control_value($params->{visibility_value_id}, $params->{visibility_field_id}); + my $type = $params->{type}; + $params->{value_field_id} = + $class->_check_value_field_id($params->{value_field_id}, + ($type == FIELD_TYPE_SINGLE_SELECT + || $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0); return $params; } +sub update { + my $self = shift; + my $changes = $self->SUPER::update(@_); + my $dbh = Bugzilla->dbh; + if ($changes->{value_field_id} && $self->is_select) { + $dbh->do("UPDATE " . $self->name . " SET visibility_value_id = NULL"); + } + return $changes; +} + =pod diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm index 4da644f1d..9e8fb1235 100644 --- a/Bugzilla/Field/Choice.pm +++ b/Bugzilla/Field/Choice.pm @@ -40,11 +40,13 @@ use constant DB_COLUMNS => qw( id value sortkey + visibility_value_id ); use constant UPDATE_COLUMNS => qw( value sortkey + visibility_value_id ); use constant NAME_FIELD => 'value'; @@ -55,6 +57,7 @@ use constant REQUIRED_CREATE_FIELDS => qw(value); use constant VALIDATORS => { value => \&_check_value, sortkey => \&_check_sortkey, + visibility_value_id => \&_check_visibility_value_id, }; use constant CLASS_MAP => { @@ -186,9 +189,12 @@ sub remove_from_db { ThrowUserError("fieldvalue_still_has_bugs", { field => $self->field, value => $self }); } - if (my @vis_fields = @{ $self->controls_visibility_of_fields }) { + my $vis_fields = $self->controls_visibility_of_fields; + my $values = $self->controlled_values; + if (@$vis_fields || @$values) { ThrowUserError('fieldvalue_is_controller', - { value => $self, fields => [map($_->name, @vis_fields)] }); + { value => $self, fields => [map($_->name, @$vis_fields)], + vals => $values }); } $self->SUPER::remove_from_db(); } @@ -260,12 +266,41 @@ sub controls_visibility_of_fields { return $self->{controls_visibility_of_fields}; } +sub visibility_value { + my $self = shift; + if ($self->{visibility_value_id}) { + $self->{visibility_value} ||= + Bugzilla::Field::Choice->type($self->field->value_field)->new( + $self->{visibility_value_id}); + } + return $self->{visibility_value}; +} + +sub controlled_values { + my $self = shift; + return $self->{controlled_values} if defined $self->{controlled_values}; + my $fields = $self->field->controls_values_of; + my @controlled_values; + foreach my $field (@$fields) { + my $controlled = Bugzilla::Field::Choice->type($field) + ->match({ visibility_value_id => $self->id }); + push(@controlled_values, @$controlled); + } + $self->{controlled_values} = \@controlled_values; + return $self->{controlled_values}; +} + ############ # Mutators # ############ sub set_name { $_[0]->set('value', $_[1]); } sub set_sortkey { $_[0]->set('sortkey', $_[1]); } +sub set_visibility_value { + my ($self, $value) = @_; + $self->set('visibility_value_id', $value); + delete $self->{visibility_value}; +} ############## # Validators # @@ -312,6 +347,15 @@ sub _check_sortkey { return $value; } +sub _check_visibility_value_id { + my ($invocant, $value_id) = @_; + $value_id = trim($value_id); + my $field = $invocant->field->value_field; + return undef if !$field || !$value_id; + my $value_obj = Bugzilla::Field::Choice->type($field) + ->check({ id => $value_id }); + return $value_obj->id; +} 1; diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 7655b7619..b6bda167e 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -88,6 +88,9 @@ sub update_fielddefs_definition { $dbh->bz_add_column('fielddefs', 'visibility_field_id', {TYPE => 'INT3'}); $dbh->bz_add_column('fielddefs', 'visibility_value_id', {TYPE => 'INT2'}); + $dbh->bz_add_column('fielddefs', 'value_field_id', {TYPE => 'INT3'}); + $dbh->bz_add_index('fielddefs', 'fielddefs_value_field_id_idx', + ['value_field_id']); # Remember, this is not the function for adding general table changes. # That is below. Add new changes to the fielddefs table above this @@ -539,6 +542,8 @@ sub update_table_definitions { # 2008-09-07 LpSolit@gmail.com - Bug 452893 _fix_illegal_flag_modification_dates(); + _add_visiblity_value_to_value_tables(); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -2907,7 +2912,25 @@ sub _initialize_workflow { # Make sure the bug status used by the 'duplicate_or_move_bug_status' # parameter has all the required transitions set. - Bugzilla::Status::add_missing_bug_status_transitions(); + my $dup_status = Bugzilla->params->{'duplicate_or_move_bug_status'}; + my $status_id = $dbh->selectrow_array( + 'SELECT id FROM bug_status WHERE value = ?', undef, $dup_status); + # There's a minor chance that this status isn't in the DB. + $status_id || return; + + my $missing_statuses = $dbh->selectcol_arrayref( + 'SELECT id FROM bug_status + LEFT JOIN status_workflow ON old_status = id + AND new_status = ? + WHERE old_status IS NULL', undef, $status_id); + + my $sth = $dbh->prepare('INSERT INTO status_workflow + (old_status, new_status) VALUES (?, ?)'); + + foreach my $old_status_id (@$missing_statuses) { + next if ($old_status_id == $status_id); + $sth->execute($old_status_id, $status_id); + } } sub _make_lang_setting_dynamic { @@ -3098,6 +3121,20 @@ sub _fix_illegal_flag_modification_dates { print "$rows flags had an illegal modification date. Fixed!\n" if ($rows =~ /^\d+$/); } +sub _add_visiblity_value_to_value_tables { + my $dbh = Bugzilla->dbh; + my @standard_fields = + qw(bug_status resolution priority bug_severity op_sys rep_platform); + my $custom_fields = $dbh->selectcol_arrayref( + 'SELECT name FROM fielddefs WHERE custom = 1 AND type IN(?,?)', + undef, FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT); + foreach my $field (@standard_fields, @$custom_fields) { + $dbh->bz_add_column($field, 'visibility_value_id', {TYPE => 'INT2'}); + $dbh->bz_add_index($field, "${field}_visibility_value_id_idx", + ['visibility_value_id']); + } +} + 1; __END__ diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index d32b6c354..289e17260 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -46,13 +46,10 @@ use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw( use constant DB_TABLE => 'bug_status'; -use constant DB_COLUMNS => qw( - id - value - sortkey - isactive - is_open -); +# This has all the standard Bugzilla::Field::Choice columns plus "is_open" +sub DB_COLUMNS { + return ($_[0]->SUPER::DB_COLUMNS, 'is_open'); +} sub VALIDATORS { my $invocant = shift; -- cgit v1.2.3-24-g4f1b