From 3cea91884b28b52df4e38f2ba88c00b65071a81f Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Sat, 25 Oct 2008 04:11:30 +0000 Subject: Bug 291433: Ability to have custom fields whose visibility depends on the values of other fields Patch By Max Kanat-Alexander r=bbaetz, a=mkanat --- Bugzilla/DB.pm | 14 ++-- Bugzilla/DB/Schema.pm | 4 + Bugzilla/Field.pm | 169 +++++++++++++++++++++++++++++++++++---- Bugzilla/Field/Choice.pm | 12 +++ Bugzilla/Install/DB.pm | 3 + Bugzilla/Object.pm | 10 +-- Bugzilla/WebService/Constants.pm | 2 +- 7 files changed, 186 insertions(+), 28 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index e264dc443..cc4ddb9aa 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -1209,12 +1209,16 @@ sub _check_references { my $foreign_table = $fk->{TABLE}; my $foreign_column = $fk->{COLUMN}; + # We use table aliases because sometimes we join a table to itself, + # and we can't use the same table name on both sides of the join. + # We also can't use the words "table" or "foreign" because those are + # reserved words. my $bad_values = $self->selectcol_arrayref( - "SELECT DISTINCT $table.$column - FROM $table LEFT JOIN $foreign_table - ON $table.$column = $foreign_table.$foreign_column - WHERE $foreign_table.$foreign_column IS NULL - AND $table.$column IS NOT NULL"); + "SELECT DISTINCT tabl.$column + FROM $table AS tabl LEFT JOIN $foreign_table AS forn + ON tabl.$column = forn.$foreign_column + WHERE forn.$foreign_column IS NULL + AND tabl.$column IS NOT NULL"); if (@$bad_values) { my $delete_action = $fk->{DELETE} || ''; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 850e48a1b..80a964446 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -631,6 +631,10 @@ use constant ABSTRACT_SCHEMA => { DEFAULT => 'FALSE'}, enter_bug => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}, + visibility_field_id => {TYPE => 'INT3', + REFERENCES => {TABLE => 'fielddefs', + COLUMN => 'id'}}, + visibility_value_id => {TYPE => 'INT2'}, ], INDEXES => [ fielddefs_name_idx => {FIELDS => ['name'], diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 5d8e5adc0..7da8a8bba 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -77,6 +77,8 @@ use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Util; +use Scalar::Util qw(blessed); + ############################### #### Initialization #### ############################### @@ -84,16 +86,18 @@ use Bugzilla::Util; use constant DB_TABLE => 'fielddefs'; use constant LIST_ORDER => 'sortkey, name'; -use constant DB_COLUMNS => ( - 'id', - 'name', - 'description', - 'type', - 'custom', - 'mailhead', - 'sortkey', - 'obsolete', - 'enter_bug', +use constant DB_COLUMNS => qw( + id + name + description + type + custom + mailhead + sortkey + obsolete + enter_bug + visibility_field_id + visibility_value_id ); use constant REQUIRED_CREATE_FIELDS => qw(name description); @@ -106,6 +110,11 @@ use constant VALIDATORS => { obsolete => \&_check_obsolete, sortkey => \&_check_sortkey, type => \&_check_type, + visibility_field_id => \&_check_control_field, +}; + +use constant UPDATE_VALIDATORS => { + visibility_value_id => \&_check_control_value, }; use constant UPDATE_COLUMNS => qw( @@ -114,6 +123,8 @@ use constant UPDATE_COLUMNS => qw( sortkey obsolete enter_bug + visibility_field_id + visibility_value_id ); # How various field types translate into SQL data definitions. @@ -259,6 +270,37 @@ sub _check_type { return $type; } +sub _check_control_field { + my ($invocant, $field_id) = @_; + $field_id = trim($field_id); + return undef if !$field_id; + my $field = Bugzilla::Field->check({ id => $field_id }); + if (blessed($invocant) && $field->id == $invocant->id) { + ThrowUserError('field_cant_control_self', { field => $field }); + } + if (!$field->is_select) { + ThrowUserError('field_control_must_be_select', + { field => $field }); + } + return $field->id; +} + +sub _check_control_value { + my ($invocant, $value_id, $field_id) = @_; + my $field; + if (blessed($invocant)) { + $field = $invocant->visibility_field; + } + elsif ($field_id) { + $field = $invocant->new($field_id); + } + # When no field is set, no value is set. + return undef if !$field; + my $value_obj = Bugzilla::Field::Choice->type($field) + ->check({ id => $value_id }); + return $value_obj->id; +} + =pod =head2 Instance Properties @@ -362,6 +404,11 @@ sub enter_bug { return $_[0]->{enter_bug} } =over +=item C + +True if this is a C or C +field. It is only safe to call L if this is true. + =item C Valid values for this field, as an array of L @@ -371,6 +418,11 @@ objects. =cut +sub is_select { + return ($_[0]->type == FIELD_TYPE_SINGLE_SELECT + || $_[0]->type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0 +} + sub legal_values { my $self = shift; @@ -384,6 +436,76 @@ sub legal_values { =pod +=over + +=item C + +What field controls this field's visibility? Returns a C +object representing the field that controls this field's visibility. + +Returns undef if there is no field that controls this field's visibility. + +=back + +=cut + +sub visibility_field { + my $self = shift; + if ($self->{visibility_field_id}) { + $self->{visibility_field} ||= + $self->new($self->{visibility_field_id}); + } + return $self->{visibility_field}; +} + +=pod + +=over + +=item C + +If we have a L, then what value does that field have to +be set to in order to show this field? Returns a L +or undef if there is no C set. + +=back + +=cut + + +sub visibility_value { + my $self = shift; + if ($self->{visibility_field_id}) { + require Bugzilla::Field::Choice; + $self->{visibility_value} ||= + Bugzilla::Field::Choice->type($self->visibility_field)->new( + $self->{visibility_value_id}); + } + return $self->{visibility_value}; +} + +=pod + +=over + +=item C + +An arrayref of C objects, representing fields that this +field controls the visibility of. + +=back + +=cut + +sub controls_visibility_of { + my $self = shift; + $self->{controls_visibility_of} ||= + Bugzilla::Field->match({ visibility_field_id => $self->id }); + return $self->{controls_visibility_of}; +} + +=pod + =head2 Instance Mutators These set the particular field that they are named after. @@ -404,6 +526,10 @@ They will throw an error if you try to set the values to something invalid. =item C +=item C + +=item C + =back =cut @@ -413,6 +539,17 @@ sub set_enter_bug { $_[0]->set('enter_bug', $_[1]); } sub set_obsolete { $_[0]->set('obsolete', $_[1]); } sub set_sortkey { $_[0]->set('sortkey', $_[1]); } sub set_in_new_bugmail { $_[0]->set('mailhead', $_[1]); } +sub set_visibility_field { + my ($self, $value) = @_; + $self->set('visibility_field_id', $value); + delete $self->{visibility_field}; + delete $self->{visibility_value}; +} +sub set_visibility_value { + my ($self, $value) = @_; + $self->set('visibility_value_id', $value); + delete $self->{visibility_value}; +} =pod @@ -487,9 +624,7 @@ sub remove_from_db { $dbh->bz_drop_column('bugs', $name); } - if ($type == FIELD_TYPE_SINGLE_SELECT - || $type == FIELD_TYPE_MULTI_SELECT) - { + if ($self->is_select) { # Delete the table that holds the legal values for this field. $dbh->bz_drop_field_tables($self); } @@ -545,9 +680,7 @@ sub create { $dbh->bz_add_column('bugs', $name, SQL_DEFINITIONS->{$type}); } - if ($type == FIELD_TYPE_SINGLE_SELECT - || $type == FIELD_TYPE_MULTI_SELECT) - { + if ($field->is_select) { # Create the table that holds the legal values for this field. $dbh->bz_add_field_tables($field); } @@ -572,6 +705,10 @@ sub run_create_validators { "SELECT MAX(sortkey) + 100 FROM fielddefs") || 100; } + $params->{visibility_value_id} = + $class->_check_control_value($params->{visibility_value_id}, + $params->{visibility_field_id}); + return $params; } diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm index dbdfea1a3..4da644f1d 100644 --- a/Bugzilla/Field/Choice.pm +++ b/Bugzilla/Field/Choice.pm @@ -186,6 +186,10 @@ sub remove_from_db { ThrowUserError("fieldvalue_still_has_bugs", { field => $self->field, value => $self }); } + if (my @vis_fields = @{ $self->controls_visibility_of_fields }) { + ThrowUserError('fieldvalue_is_controller', + { value => $self, fields => [map($_->name, @vis_fields)] }); + } $self->SUPER::remove_from_db(); } @@ -248,6 +252,14 @@ sub is_static { return 0; } +sub controls_visibility_of_fields { + my $self = shift; + $self->{controls_visibility_of_fields} ||= Bugzilla::Field->match( + { visibility_field_id => $self->field->id, + visibility_value_id => $self->id }); + return $self->{controls_visibility_of_fields}; +} + ############ # Mutators # ############ diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 213fe3099..7655b7619 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -86,6 +86,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'}); + # Remember, this is not the function for adding general table changes. # That is below. Add new changes to the fielddefs table above this # comment. diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 1e624e5f7..53720327b 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -117,12 +117,10 @@ sub check { if (!ref $param) { $param = { name => $param }; } - # Don't allow empty names. - if (exists $param->{name}) { - $param->{name} = trim($param->{name}); - $param->{name} || ThrowUserError('object_name_not_specified', - { class => $class }); - } + # Don't allow empty names or ids. + my $check_param = exists $param->{id} ? $param->{id} : $param->{name}; + $check_param = trim($check_param); + $check_param || ThrowUserError('object_not_specified', { class => $class }); my $obj = $class->new($param) || ThrowUserError('object_does_not_exist', {%$param, class => $class}); return $obj; diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index a43e1f590..593801a6f 100755 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -49,7 +49,7 @@ use base qw(Exporter); # have to fix it here. use constant WS_ERROR_CODE => { # Generic Bugzilla::Object errors are 50-99. - object_name_not_specified => 50, + object_not_specified => 50, param_required => 50, object_does_not_exist => 51, # Bug errors usually occupy the 100-200 range. -- cgit v1.2.3-24-g4f1b