From 0844fe9fc6d2d5293fad1384d2955dedf30fc8c4 Mon Sep 17 00:00:00 2001 From: Tiago Mello Date: Fri, 27 Aug 2010 00:27:08 -0300 Subject: Bug 479400: Add the ability to show or hide particular custom fields based on multiple values of another field (visibility controllers) r/a=mkanat --- Bugzilla/DB/Schema.pm | 20 +++- Bugzilla/Field.pm | 116 +++++++++++++++------ Bugzilla/Install/DB.pm | 47 ++++++--- editfields.cgi | 4 +- js/field.js | 18 +++- .../en/default/admin/custom_fields/cf-js.js.tmpl | 2 +- .../default/admin/custom_fields/create.html.tmpl | 7 +- .../en/default/admin/custom_fields/edit.html.tmpl | 10 +- template/en/default/bug/field-events.js.tmpl | 7 +- template/en/default/bug/field.html.tmpl | 4 +- template/en/default/global/user-error.html.tmpl | 5 + 11 files changed, 171 insertions(+), 69 deletions(-) diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index a4d44d191..2efdbefc4 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -671,7 +671,6 @@ use constant ABSTRACT_SCHEMA => { visibility_field_id => {TYPE => 'INT3', REFERENCES => {TABLE => 'fielddefs', COLUMN => 'id'}}, - visibility_value_id => {TYPE => 'INT2'}, value_field_id => {TYPE => 'INT3', REFERENCES => {TABLE => 'fielddefs', COLUMN => 'id'}}, @@ -688,6 +687,25 @@ use constant ABSTRACT_SCHEMA => { ], }, + # Field Visibility Information + # ------------------------- + + field_visibility => { + FIELDS => [ + field_id => {TYPE => 'INT3', + REFERENCES => {TABLE => 'fielddefs', + COLUMN => 'id', + DELETE => 'CASCADE'}}, + value_id => {TYPE => 'INT2', NOTNULL => 1} + ], + INDEXES => [ + field_visibility_field_id_idx => { + FIELDS => [qw(field_id value_id)], + TYPE => 'UNIQUE' + }, + ], + }, + # Per-product Field Values # ------------------------ diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index daf708179..41b90a644 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -77,6 +77,7 @@ use base qw(Exporter Bugzilla::Object); use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Util; +use List::MoreUtils qw(any); use Scalar::Util qw(blessed); @@ -99,7 +100,6 @@ use constant DB_COLUMNS => qw( enter_bug buglist visibility_field_id - visibility_value_id value_field_id reverse_desc is_mandatory @@ -118,7 +118,7 @@ use constant VALIDATORS => { type => \&_check_type, value_field_id => \&_check_value_field_id, visibility_field_id => \&_check_visibility_field_id, - visibility_value_id => \&_check_control_value, + visibility_values => \&_check_visibility_values, is_mandatory => \&Bugzilla::Object::check_boolean, }; @@ -127,7 +127,7 @@ use constant VALIDATOR_DEPENDENCIES => { type => ['custom'], reverse_desc => ['type'], value_field_id => ['type'], - visibility_value_id => ['visibility_field_id'], + visibility_values => ['visibility_field_id'], }; use constant UPDATE_COLUMNS => qw( @@ -138,7 +138,6 @@ use constant UPDATE_COLUMNS => qw( enter_bug buglist visibility_field_id - visibility_value_id value_field_id reverse_desc is_mandatory @@ -357,8 +356,8 @@ sub _check_visibility_field_id { return $field->id; } -sub _check_control_value { - my ($invocant, $value_id, undef, $params) = @_; +sub _check_visibility_values { + my ($invocant, $values, undef, $params) = @_; my $field; if (blessed $invocant) { $field = $invocant->visibility_field; @@ -366,11 +365,24 @@ sub _check_control_value { elsif ($params->{visibility_field_id}) { $field = $invocant->new($params->{visibility_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; + # When no field is set, no values are set. + return [] if !$field; + + if (!scalar @$values) { + ThrowUserError('field_visibility_values_must_be_selected', + { field => $field }); + } + + my @visibility_values; + my $choice = Bugzilla::Field::Choice->type($field); + foreach my $value (@$values) { + if (!blessed $value) { + $value = $choice->check({ id => $value }); + } + push(@visibility_values, $value); + } + + return \@visibility_values; } sub _check_reverse_desc { @@ -603,9 +615,9 @@ sub visibility_field { =over -=item C +=item C -If we have a L, then what value does that field have to +If we have a L, then what values 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. @@ -613,16 +625,23 @@ or undef if there is no C set. =cut - -sub visibility_value { +sub visibility_values { 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}); + my $dbh = Bugzilla->dbh; + + return [] if !$self->{visibility_field_id}; + + if (!defined $self->{visibility_values}) { + my $visibility_value_ids = + $dbh->selectcol_arrayref("SELECT value_id FROM field_visibility + WHERE field_id = ?", undef, $self->id); + + $self->{visibility_values} = + Bugzilla::Field::Choice->type($self->visibility_field) + ->new_from_list($visibility_value_ids); } - return $self->{visibility_value}; + + return $self->{visibility_values}; } =pod @@ -700,10 +719,13 @@ See L. sub is_visible_on_bug { my ($self, $bug) = @_; - my $visibility_value = $self->visibility_value; - return 1 if !$visibility_value; + # Always return visible, if this field is not + # visibility controlled. + return 1 if !$self->{visibility_field_id}; - return $visibility_value->is_set_on_bug($bug); + my $visibility_values = $self->visibility_values; + + return (any { $_->is_set_on_bug($bug) } @$visibility_values) ? 1 : 0; } =over @@ -784,7 +806,7 @@ They will throw an error if you try to set the values to something invalid. =item C -=item C +=item C =item C @@ -806,12 +828,11 @@ sub set_visibility_field { my ($self, $value) = @_; $self->set('visibility_field_id', $value); delete $self->{visibility_field}; - delete $self->{visibility_value}; + delete $self->{visibility_values}; } -sub set_visibility_value { - my ($self, $value) = @_; - $self->set('visibility_value_id', $value); - delete $self->{visibility_value}; +sub set_visibility_values { + my ($self, $value_ids) = @_; + $self->set('visibility_values', $value_ids); } sub set_value_field { my ($self, $value) = @_; @@ -945,13 +966,24 @@ C - boolean - Whether this field is mandatory. Defaults to 0. sub create { my $class = shift; my ($params) = @_; + my $dbh = Bugzilla->dbh; + # This makes sure the "sortkey" validator runs, even if # the parameter isn't sent to create(). $params->{sortkey} = undef if !exists $params->{sortkey}; $params->{type} ||= 0; - my $field = $class->SUPER::create(@_); + + $dbh->bz_start_transaction(); + $class->check_required_create_fields(@_); + my $field_values = $class->run_create_validators($params); + my $visibility_values = delete $field_values->{visibility_values}; + my $field = $class->insert_create_data($field_values); + + $field->set_visibility_values($visibility_values); + $field->_update_visibility_values(); + + $dbh->bz_commit_transaction(); - my $dbh = Bugzilla->dbh; if ($field->custom) { my $name = $field->name; my $type = $field->type; @@ -981,9 +1013,29 @@ sub update { if ($changes->{value_field_id} && $self->is_select) { $dbh->do("UPDATE " . $self->name . " SET visibility_value_id = NULL"); } + $self->_update_visibility_values(); return $changes; } +sub _update_visibility_values { + my $self = shift; + my $dbh = Bugzilla->dbh; + + my @visibility_value_ids = map($_->id, @{$self->visibility_values}); + $self->_delete_visibility_values(); + for my $value_id (@visibility_value_ids) { + $dbh->do("INSERT INTO field_visibility (field_id, value_id) + VALUES (?, ?)", undef, $self->id, $value_id); + } +} + +sub _delete_visibility_values { + my ($self) = @_; + my $dbh = Bugzilla->dbh; + $dbh->do("DELETE FROM field_visibility WHERE field_id = ?", + undef, $self->id); + delete $self->{visibility_values}; +} =pod diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index bfce7779f..b786e0624 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -88,7 +88,6 @@ 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']); @@ -113,6 +112,9 @@ sub update_fielddefs_definition { $dbh->bz_add_index('fielddefs', 'fielddefs_is_mandatory_idx', ['is_mandatory']); + # 2010-04-05 dkl@redhat.com - Bug 479400 + _migrate_field_visibility_value(); + # Remember, this is not the function for adding general table changes. # That is below. Add new changes to the fielddefs table above this # comment. @@ -561,8 +563,6 @@ sub update_table_definitions { # 2008-09-07 LpSolit@gmail.com - Bug 452893 _fix_illegal_flag_modification_dates(); - _add_visiblity_value_to_value_tables(); - # 2009-03-02 arbingersys@gmail.com - Bug 423613 _add_extern_id_index(); @@ -3208,20 +3208,6 @@ 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']); - } -} - sub _add_extern_id_index { my $dbh = Bugzilla->dbh; if (!$dbh->bz_index_info('profiles', 'profiles_extern_id_idx')) { @@ -3395,6 +3381,33 @@ sub _remove_attachment_isurl { } } +sub _migrate_field_visibility_value { + my $dbh = Bugzilla->dbh; + + if ($dbh->bz_column_info('fielddefs', 'visibility_value_id')) { + print "Populating new field_visibility table...\n"; + + $dbh->bz_start_transaction(); + + my %results = + @{ $dbh->selectcol_arrayref( + "SELECT id, visibility_value_id FROM fielddefs + WHERE visibility_value_id IS NOT NULL", + { Columns => [1,2] }) }; + + my $insert_sth = + $dbh->prepare("INSERT INTO field_visibility (field_id, value_id) + VALUES (?, ?)"); + + foreach my $id (keys %results) { + $insert_sth->execute($id, $results{$id}); + } + + $dbh->bz_commit_transaction(); + $dbh->bz_drop_column('fielddefs', 'visibility_value_id'); + } +} + 1; __END__ diff --git a/editfields.cgi b/editfields.cgi index e207a2ee4..cdf1008f7 100755 --- a/editfields.cgi +++ b/editfields.cgi @@ -66,7 +66,7 @@ elsif ($action eq 'new') { custom => 1, buglist => 1, visibility_field_id => scalar $cgi->param('visibility_field_id'), - visibility_value_id => scalar $cgi->param('visibility_value_id'), + visibility_values => [ $cgi->param('visibility_values') ], value_field_id => scalar $cgi->param('value_field_id'), reverse_desc => scalar $cgi->param('reverse_desc'), is_mandatory => scalar $cgi->param('is_mandatory'), @@ -114,7 +114,7 @@ elsif ($action eq 'update') { $field->set_obsolete($cgi->param('obsolete')); $field->set_is_mandatory($cgi->param('is_mandatory')); $field->set_visibility_field($cgi->param('visibility_field_id')); - $field->set_visibility_value($cgi->param('visibility_value_id')); + $field->set_visibility_values([ $cgi->param('visibility_values') ]); $field->set_value_field($cgi->param('value_field_id')); $field->set_reverse_desc($cgi->param('reverse_desc')); $field->update(); diff --git a/js/field.js b/js/field.js index a64369d33..8e1f53991 100644 --- a/js/field.js +++ b/js/field.js @@ -465,12 +465,12 @@ function setClassification() { * a certain value. May only be called after the controller has already * been added to the DOM. */ -function showFieldWhen(controlled_id, controller_id, value) { +function showFieldWhen(controlled_id, controller_id, values) { var controller = document.getElementById(controller_id); // Note that we don't get an object for "controlled" here, because it // might not yet exist in the DOM. We just pass along its id. - YAHOO.util.Event.addListener(controller, 'change', - handleVisControllerValueChange, [controlled_id, controller, value]); + YAHOO.util.Event.addListener(controller, 'change', + handleVisControllerValueChange, [controlled_id, controller, values]); } /** @@ -480,13 +480,21 @@ function showFieldWhen(controlled_id, controller_id, value) { function handleVisControllerValueChange(e, args) { var controlled_id = args[0]; var controller = args[1]; - var value = args[2]; + var values = args[2]; var label_container = document.getElementById('field_label_' + controlled_id); var field_container = document.getElementById('field_container_' + controlled_id); - if (bz_valueSelected(controller, value)) { + var selected = false; + for (var i = 0; i < values.length; i++) { + if (bz_valueSelected(controller, values[i])) { + selected = true; + break; + } + } + + if (selected) { YAHOO.util.Dom.removeClass(label_container, 'bz_hidden_field'); YAHOO.util.Dom.removeClass(field_container, 'bz_hidden_field'); } diff --git a/template/en/default/admin/custom_fields/cf-js.js.tmpl b/template/en/default/admin/custom_fields/cf-js.js.tmpl index f9c31b5c7..cc1a4e4aa 100644 --- a/template/en/default/admin/custom_fields/cf-js.js.tmpl +++ b/template/en/default/admin/custom_fields/cf-js.js.tmpl @@ -65,7 +65,7 @@ function onChangeType(type_field) { function onChangeVisibilityField() { var vis_field = document.getElementById('visibility_field_id'); - var vis_value = document.getElementById('visibility_value_id'); + var vis_value = document.getElementById('visibility_values'); if (vis_field.value) { var values = select_values[vis_field.value]; diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl index b96738473..f89f979fd 100644 --- a/template/en/default/admin/custom_fields/create.html.tmpl +++ b/template/en/default/admin/custom_fields/create.html.tmpl @@ -130,8 +130,11 @@ YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('ty [% END %] - - diff --git a/template/en/default/admin/custom_fields/edit.html.tmpl b/template/en/default/admin/custom_fields/edit.html.tmpl index 48c3396f3..5ce2b7fd5 100644 --- a/template/en/default/admin/custom_fields/edit.html.tmpl +++ b/template/en/default/admin/custom_fields/edit.html.tmpl @@ -118,12 +118,14 @@ [% END %] - - [% FOREACH value = field.visibility_field.legal_values %]