diff options
author | Max Kanat-Alexander <mkanat@bugzilla.org> | 2010-02-02 02:34:26 +0100 |
---|---|---|
committer | Max Kanat-Alexander <mkanat@bugzilla.org> | 2010-02-02 02:34:26 +0100 |
commit | b0642d67ae6a9a7e7bbb8b8dc7a832c26bb211af (patch) | |
tree | c08cb54facdfa7f21833b6519fd0f468f5022e29 /Bugzilla | |
parent | 52ca02ea108a6c1d4c1ec735d3907782c2000586 (diff) | |
download | bugzilla-b0642d67ae6a9a7e7bbb8b8dc7a832c26bb211af.tar.gz bugzilla-b0642d67ae6a9a7e7bbb8b8dc7a832c26bb211af.tar.xz |
Bug 487508: Allow restricting the visibility of custom fields and values by component
r=dkl, a=mkanat
Diffstat (limited to 'Bugzilla')
-rw-r--r-- | Bugzilla/Component.pm | 30 | ||||
-rw-r--r-- | Bugzilla/Constants.pm | 9 | ||||
-rw-r--r-- | Bugzilla/Field.pm | 24 | ||||
-rw-r--r-- | Bugzilla/Field/Choice.pm | 140 | ||||
-rw-r--r-- | Bugzilla/Migrate.pm | 4 | ||||
-rw-r--r-- | Bugzilla/Product.pm | 17 | ||||
-rw-r--r-- | Bugzilla/Search.pm | 2 | ||||
-rw-r--r-- | Bugzilla/Status.pm | 5 | ||||
-rw-r--r-- | Bugzilla/WebService/Bug.pm | 4 |
9 files changed, 75 insertions, 160 deletions
diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm index 194a3957c..5fb911031 100644 --- a/Bugzilla/Component.pm +++ b/Bugzilla/Component.pm @@ -17,11 +17,9 @@ # Max Kanat-Alexander <mkanat@bugzilla.org> # Akamai Technologies <bugzilla-dev@akamai.com> -use strict; - package Bugzilla::Component; - -use base qw(Bugzilla::Object); +use strict; +use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object); use Bugzilla::Constants; use Bugzilla::Util; @@ -35,6 +33,8 @@ use Bugzilla::Series; ############################### use constant DB_TABLE => 'components'; +# This is mostly for the editfields.cgi case where ->get_all is called. +use constant LIST_ORDER => 'product_id, name'; use constant DB_COLUMNS => qw( id @@ -80,7 +80,7 @@ sub new { my $dbh = Bugzilla->dbh; my $product; - if (ref $param) { + if (ref $param and !defined $param->{id}) { $product = $param->{product}; my $name = $param->{name}; if (!defined $product) { @@ -156,6 +156,8 @@ sub remove_from_db { my $self = shift; my $dbh = Bugzilla->dbh; + $self->_check_if_controller(); # From ChoiceInterface + $dbh->bz_start_transaction(); if ($self->bug_count) { @@ -418,11 +420,25 @@ sub product { #### Accessors #### ############################### -sub id { return $_[0]->{'id'}; } -sub name { return $_[0]->{'name'}; } sub description { return $_[0]->{'description'}; } sub product_id { return $_[0]->{'product_id'}; } +############################################## +# Implement Bugzilla::Field::ChoiceInterface # +############################################## + +use constant FIELD_NAME => 'component'; +use constant is_default => 0; +use constant is_active => 1; + +sub is_set_on_bug { + my ($self, $bug) = @_; + # We treat it like a hash always, so that we don't have to check if it's + # a hash or an object. + return 0 if !defined $bug->{component_id}; + $bug->{component_id} == $self->id ? 1 : 0; +} + ############################### #### Subroutines #### ############################### diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 94d9f8bed..948ff5337 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -129,6 +129,8 @@ use File::Basename; FIELD_TYPE_BUG_ID FIELD_TYPE_BUG_URLS + ABNORMAL_SELECTS + TIMETRACKING_FIELDS USAGE_MODE_BROWSER @@ -368,6 +370,13 @@ use constant FIELD_TYPE_DATETIME => 5; use constant FIELD_TYPE_BUG_ID => 6; use constant FIELD_TYPE_BUG_URLS => 7; +# See the POD for Bugzilla::Field/is_abnormal to see why these are listed +# here. +use constant ABNORMAL_SELECTS => qw( + product + component +); + # The fields from fielddefs that are blocked from non-timetracking users. # work_time is sometimes called actual_time. use constant TIMETRACKING_FIELDS => diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 7b1569c52..2f14037ab 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -180,7 +180,7 @@ use constant DEFAULT_FIELDS => ( {name => 'priority', desc => 'Priority', in_new_bugmail => 1, type => FIELD_TYPE_SINGLE_SELECT, buglist => 1}, {name => 'component', desc => 'Component', in_new_bugmail => 1, - buglist => 1}, + type => FIELD_TYPE_SINGLE_SELECT, buglist => 1}, {name => 'assigned_to', desc => 'AssignedTo', in_new_bugmail => 1, buglist => 1}, {name => 'reporter', desc => 'ReportedBy', in_new_bugmail => 1, @@ -492,6 +492,28 @@ sub is_select { || $_[0]->type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0 } +=over + +=item C<is_abnormal> + +Most fields that have a C<SELECT> L</type> have a certain schema for +the table that stores their values, the table has the same name as the field, +and the field's legal values can be edited via F<editvalues.cgi>. + +However, some fields do not follow that pattern. Those fields are +considered "abnormal". + +This method returns C<1> if the field is "abnormal", C<0> otherwise. + +=back + +=cut + +sub is_abnormal { + my $self = shift; + return grep($_ eq $self->name, ABNORMAL_SELECTS) ? 1 : 0; +} + sub legal_values { my $self = shift; diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm index 7e07ca1e2..9c2fbdb38 100644 --- a/Bugzilla/Field/Choice.pm +++ b/Bugzilla/Field/Choice.pm @@ -23,7 +23,7 @@ use strict; package Bugzilla::Field::Choice; -use base qw(Bugzilla::Object); +use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object); use Bugzilla::Config qw(SetParam write_params); use Bugzilla::Constants; @@ -66,6 +66,7 @@ use constant VALIDATORS => { use constant CLASS_MAP => { bug_status => 'Bugzilla::Status', + component => 'Bugzilla::Component', product => 'Bugzilla::Product', }; @@ -194,115 +195,10 @@ sub remove_from_db { ThrowUserError("fieldvalue_still_has_bugs", { field => $self->field, value => $self }); } - $self->_check_if_controller(); + $self->_check_if_controller(); # From ChoiceInterface. $self->SUPER::remove_from_db(); } -# Factored out to make life easier for subclasses. -sub _check_if_controller { - my $self = shift; - my $vis_fields = $self->controls_visibility_of_fields; - my $values = $self->controlled_values; - if (@$vis_fields || scalar(keys %$values)) { - ThrowUserError('fieldvalue_is_controller', - { value => $self, fields => [map($_->name, @$vis_fields)], - vals => $values }); - } -} - - -############# -# Accessors # -############# - -sub is_active { return $_[0]->{'isactive'}; } -sub sortkey { return $_[0]->{'sortkey'}; } - -sub bug_count { - my $self = shift; - return $self->{bug_count} if defined $self->{bug_count}; - my $dbh = Bugzilla->dbh; - my $fname = $self->field->name; - my $count; - if ($self->field->type == FIELD_TYPE_MULTI_SELECT) { - $count = $dbh->selectrow_array("SELECT COUNT(*) FROM bug_$fname - WHERE value = ?", undef, $self->name); - } - else { - $count = $dbh->selectrow_array("SELECT COUNT(*) FROM bugs - WHERE $fname = ?", - undef, $self->name); - } - $self->{bug_count} = $count; - return $count; -} - -sub field { - my $invocant = shift; - my $class = ref $invocant || $invocant; - my $cache = Bugzilla->request_cache; - # This is just to make life easier for subclasses. Our auto-generated - # subclasses from type() already have this set. - $cache->{"field_$class"} ||= - new Bugzilla::Field({ name => $class->DB_TABLE }); - return $cache->{"field_$class"}; -} - -sub is_default { - my $self = shift; - my $name = $self->DEFAULT_MAP->{$self->field->name}; - # If it doesn't exist in DEFAULT_MAP, then there is no parameter - # related to this field. - return 0 unless $name; - return ($self->name eq Bugzilla->params->{$name}) ? 1 : 0; -} - -sub is_static { - my $self = shift; - # If we need to special-case Resolution for *anything* else, it should - # get its own subclass. - if ($self->field->name eq 'resolution') { - return grep($_ eq $self->name, ('', 'FIXED', 'MOVED', 'DUPLICATE')) - ? 1 : 0; - } - elsif ($self->field->custom) { - return $self->name eq '---' ? 1 : 0; - } - 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}; -} - -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) { - $controlled_values{$field->name} = - Bugzilla::Field::Choice->type($field) - ->match({ visibility_value_id => $self->id }); - } - $self->{controlled_values} = \%controlled_values; - return $self->{controlled_values}; -} - ############ # Mutators # ############ @@ -402,6 +298,9 @@ each value type needs its own class. See the L</SYNOPSIS> for examples of how this works. +This class implements L<Bugzilla::Field::ChoiceInterface>, and so all +methods of that class are also available here. + =head1 METHODS =head2 Class Factory @@ -424,28 +323,7 @@ must call C<type> to get a class you can call methods on. =back -=head2 Accessors +=head2 Mutators -These are in addition to the standard L<Bugzilla::Object> accessors. - -=over - -=item C<sortkey> - -The key that determines the sort order of this item. - -=item C<field> - -The L<Bugzilla::Field> object that this field value belongs to. - -=item C<controlled_values> - -Tells you which values in B<other> fields appear (become visible) when this -value is set in its field. - -Returns a hashref of arrayrefs. The hash keys are the names of fields, -and the values are arrays of C<Bugzilla::Field::Choice> objects, -representing values that this value controls the visibility of, for -that field. - -=back +This class implements mutators for all of the settable accessors in +L<Bugzilla::Field::ChoiceInterface>. diff --git a/Bugzilla/Migrate.pm b/Bugzilla/Migrate.pm index 282279e75..6c353357d 100644 --- a/Bugzilla/Migrate.pm +++ b/Bugzilla/Migrate.pm @@ -323,7 +323,7 @@ sub reset_serial_values { ); my @select_fields = grep { $_->is_select } (values %{ $self->bug_fields }); foreach my $field (@select_fields) { - next if $field->name eq 'product'; + next if $field->is_abnormal; $reset{$field->name} = 'id'; } @@ -709,8 +709,8 @@ sub insert_bugs { $self->debug($bug, 3); foreach my $field (@standard_drop_downs) { + next if $field->is_abnormal; my $field_name = $field->name; - next if $field_name eq 'product'; if (!defined $bug->{$field_name}) { # If there's a default value for this, then just let create() # pick it. diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index c993905db..6b00fcbf6 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -32,9 +32,7 @@ use Bugzilla::Mailer; use Bugzilla::Series; use Bugzilla::Hook; -# Currently, we only implement enough of the Bugzilla::Field::Choice -# interface to control the visibility of other fields. -use base qw(Bugzilla::Field::Choice); +use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object); use constant DEFAULT_CLASSIFICATION_ID => 1; @@ -43,10 +41,6 @@ use constant DEFAULT_CLASSIFICATION_ID => 1; ############################### use constant DB_TABLE => 'products'; -# Reset these back to the Bugzilla::Object defaults, instead of the -# Bugzilla::Field::Choice defaults. -use constant NAME_FIELD => 'name'; -use constant LIST_ORDER => 'name'; use constant DB_COLUMNS => qw( id @@ -565,14 +559,7 @@ sub _check_votes { # Implement Bugzilla::Field::Choice # ##################################### -sub field { - my $invocant = shift; - my $class = ref $invocant || $invocant; - my $cache = Bugzilla->request_cache; - $cache->{"field_$class"} ||= new Bugzilla::Field({ name => 'product' }); - return $cache->{"field_$class"}; -} - +use constant FIELD_NAME => 'product'; use constant is_default => 0; ############################### diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 43b95f293..88e44ac1d 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -219,8 +219,8 @@ sub init { type => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS], obsolete => 0 }); foreach my $field (@select_fields) { + next if $field->is_abnormal; my $name = $field->name; - next if $name eq 'product'; # products don't have sortkeys. $special_order{$name} = [ "$name.sortkey", "$name.value" ], $special_order_join{$name} = "LEFT JOIN $name ON $name.value = bugs.$name"; diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index 4d1281e7e..0dd17ae39 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -23,7 +23,10 @@ use strict; package Bugzilla::Status; use Bugzilla::Error; - +# This subclasses Bugzilla::Field::Choice instead of implementing +# ChoiceInterface, because a bug status literally is a special type +# of Field::Choice, not just an object that happens to have the same +# methods. use base qw(Bugzilla::Field::Choice Exporter); @Bugzilla::Status::EXPORT = qw( BUG_STATE_OPEN diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 53f3255d1..16217bb63 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -422,8 +422,8 @@ sub legal_values { my $field = Bugzilla::Bug::FIELD_MAP->{$params->{field}} || $params->{field}; - my @global_selects = Bugzilla->get_fields( - {type => [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT]}); + my @global_selects = grep { !$_->is_abnormal } + Bugzilla->get_fields({ is_select => 1 }); my $values; if (grep($_->name eq $field, @global_selects)) { |