From 44bc791eb3aa690940d540a7154d93dc8b10f186 Mon Sep 17 00:00:00 2001 From: Tiago Mello Date: Wed, 19 May 2010 09:28:37 -0700 Subject: Bug 494395: Implement the ability to mark custom fields as mandatory when creating/changing bugs r=mkanat, a=mkanat --- Bugzilla/Bug.pm | 36 +++++++++++++++++ Bugzilla/Constants.pm | 4 ++ Bugzilla/DB/Schema.pm | 3 ++ Bugzilla/Field.pm | 46 +++++++++++++++++++++- Bugzilla/Install/DB.pm | 5 +++ editfields.cgi | 2 + .../default/admin/custom_fields/create.html.tmpl | 32 +++++++++------ .../en/default/admin/custom_fields/edit.html.tmpl | 41 ++++++++++--------- .../en/default/admin/custom_fields/list.html.tmpl | 4 ++ template/en/default/bug/create/create.html.tmpl | 8 ++-- template/en/default/bug/field-label.html.tmpl | 3 +- template/en/default/bug/field.html.tmpl | 20 +++++----- template/en/default/global/textarea.html.tmpl | 5 +++ template/en/default/global/user-error.html.tmpl | 5 +++ 14 files changed, 166 insertions(+), 48 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 6f2c2b602..dcf3f96da 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -54,6 +54,7 @@ use List::Util qw(min first); use Storable qw(dclone); use URI; use URI::QueryParam; +use Scalar::Util qw(blessed); use base qw(Bugzilla::Object Exporter); @Bugzilla::Bug::EXPORT = qw( @@ -632,6 +633,14 @@ sub run_create_validators { Bugzilla::Hook::process('bug_end_of_create_validators', { params => $params }); + my @mandatory_fields = Bugzilla->get_fields({ is_mandatory => 1, + enter_bug => 1, + obsolete => 0 }); + foreach my $field (@mandatory_fields) { + $class->_check_field_is_mandatory($params->{$field->name}, $field, + $params); + } + return $params; } @@ -1680,6 +1689,32 @@ sub _check_work_time { # Custom Field Validators +sub _check_field_is_mandatory { + my ($invocant, $value, $field, $params) = @_; + + if (!blessed($field)) { + $field = Bugzilla::Field->check({ name => $field }); + } + + return if !$field->is_mandatory; + + return if !$field->is_visible_on_bug($params || $invocant); + + if (ref($value) eq 'ARRAY') { + $value = join('', @$value); + } + + $value = trim($value); + if (!defined($value) + or $value eq "" + or ($value eq '---' and $field->type == FIELD_TYPE_SINGLE_SELECT) + or ($value =~ EMPTY_DATETIME_REGEX + and $field->type == FIELD_TYPE_DATETIME)) + { + ThrowUserError('required_field', { field => $field }); + } +} + sub _check_datetime_field { my ($invocant, $date_time) = @_; @@ -1843,6 +1878,7 @@ sub _set_global_validator { newvalue => $value, privs => $privs }); } + $self->_check_field_is_mandatory($value, $field); } diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 47aeb8a9d..054fc879a 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -128,6 +128,8 @@ use File::Basename; FIELD_TYPE_BUG_ID FIELD_TYPE_BUG_URLS + EMPTY_DATETIME_REGEX + ABNORMAL_SELECTS TIMETRACKING_FIELDS @@ -388,6 +390,8 @@ use constant FIELD_TYPE_DATETIME => 5; use constant FIELD_TYPE_BUG_ID => 6; use constant FIELD_TYPE_BUG_URLS => 7; +use constant EMPTY_DATETIME_REGEX => qr/^[0\-:\sA-Za-z]+$/; + # See the POD for Bugzilla::Field/is_abnormal to see why these are listed # here. use constant ABNORMAL_SELECTS => qw( diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index c66f401de..5da55cf26 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -674,12 +674,15 @@ use constant ABSTRACT_SCHEMA => { REFERENCES => {TABLE => 'fielddefs', COLUMN => 'id'}}, reverse_desc => {TYPE => 'TINYTEXT'}, + is_mandatory => {TYPE => 'BOOLEAN', NOTNULL => 1, + DEFAULT => 'FALSE'}, ], INDEXES => [ fielddefs_name_idx => {FIELDS => ['name'], TYPE => 'UNIQUE'}, fielddefs_sortkey_idx => ['sortkey'], fielddefs_value_field_id_idx => ['value_field_id'], + fielddefs_is_mandatory_idx => ['is_mandatory'], ], }, diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index b53abfb61..1ab28c50f 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -102,6 +102,7 @@ use constant DB_COLUMNS => qw( visibility_value_id value_field_id reverse_desc + is_mandatory ); use constant REQUIRED_CREATE_FIELDS => qw(name description); @@ -116,6 +117,7 @@ use constant VALIDATORS => { sortkey => \&_check_sortkey, type => \&_check_type, visibility_field_id => \&_check_visibility_field_id, + is_mandatory => \&Bugzilla::Object::check_boolean, }; use constant UPDATE_VALIDATORS => { @@ -135,7 +137,7 @@ use constant UPDATE_COLUMNS => qw( visibility_value_id value_field_id reverse_desc - + is_mandatory type ); @@ -158,7 +160,7 @@ use constant DEFAULT_FIELDS => ( {name => 'bug_id', desc => 'Bug #', in_new_bugmail => 1, buglist => 1}, {name => 'short_desc', desc => 'Summary', in_new_bugmail => 1, - buglist => 1}, + is_mandatory => 1, buglist => 1}, {name => 'classification', desc => 'Classification', in_new_bugmail => 1, buglist => 1}, {name => 'product', desc => 'Product', in_new_bugmail => 1, @@ -184,6 +186,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, + is_mandatory => 1, type => FIELD_TYPE_SINGLE_SELECT, buglist => 1}, {name => 'assigned_to', desc => 'AssignedTo', in_new_bugmail => 1, buglist => 1}, @@ -375,6 +378,7 @@ sub _check_reverse_desc { return $reverse_desc; } +sub _check_is_mandatory { return $_[1] ? 1 : 0; } =pod @@ -676,6 +680,25 @@ sub controls_values_of { =over +=item C + +See L. + +=back + +=cut + +sub is_visible_on_bug { + my ($self, $bug) = @_; + + my $visibility_value = $self->visibility_value; + return 1 if !$visibility_value; + + return $visibility_value->is_set_on_bug($bug); +} + +=over + =item C Applies only to fields of type FIELD_TYPE_BUG_ID. @@ -711,6 +734,18 @@ the reverse description would be "Duplicates of this bug". sub reverse_desc { return $_[0]->{reverse_desc} } +=over + +=item C + +a boolean specifying whether or not the field is mandatory; + +=back + +=cut + +sub is_mandatory { return $_[0]->{is_mandatory} } + =pod @@ -744,6 +779,9 @@ They will throw an error if you try to set the values to something invalid. =item C +=item C + + =back =cut @@ -771,6 +809,7 @@ sub set_value_field { $self->set('value_field_id', $value); delete $self->{value_field}; } +sub set_is_mandatory { $_[0]->set('is_mandatory', $_[1]); } # This is only used internally by upgrade code in Bugzilla::Field. sub _set_type { $_[0]->set('type', $_[1]); } @@ -886,6 +925,8 @@ selectable as a display or order column in bug lists. Defaults to 0. C - boolean - Whether this field is obsolete. Defaults to 0. +C - boolean - Whether this field is mandatory. Defaults to 0. + =back =back @@ -1019,6 +1060,7 @@ sub populate_field_definitions { $field->set_in_new_bugmail($def->{in_new_bugmail}); $field->set_buglist($def->{buglist}); $field->_set_type($def->{type}) if $def->{type}; + $field->set_is_mandatory($def->{is_mandatory}); $field->update(); } else { diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index fb199b2f0..0d9513ccf 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -108,6 +108,11 @@ sub update_fielddefs_definition { $dbh->do('UPDATE fielddefs SET buglist = 1 WHERE custom = 1 AND type = ' . FIELD_TYPE_MULTI_SELECT); + $dbh->bz_add_column('fielddefs', 'is_mandatory', + {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}); + $dbh->bz_add_index('fielddefs', 'fielddefs_is_mandatory_idx', + ['is_mandatory']); + # 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/editfields.cgi b/editfields.cgi index 35f67ae3f..0f7a45fb7 100755 --- a/editfields.cgi +++ b/editfields.cgi @@ -69,6 +69,7 @@ elsif ($action eq 'new') { visibility_value_id => scalar $cgi->param('visibility_value_id'), value_field_id => scalar $cgi->param('value_field_id'), reverse_desc => scalar $cgi->param('reverse_desc'), + is_mandatory => scalar $cgi->param('is_mandatory'), }); delete_token($token); @@ -111,6 +112,7 @@ elsif ($action eq 'update') { $field->set_in_new_bugmail($cgi->param('new_bugmail')); $field->set_enter_bug($cgi->param('enter_bug')); $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_value_field($cgi->param('value_field_id')); diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl index f8a3eafa2..fcdf73bc7 100644 --- a/template/en/default/admin/custom_fields/create.html.tmpl +++ b/template/en/default/admin/custom_fields/create.html.tmpl @@ -98,6 +98,24 @@ YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('ty + + + + + + + + + + +
+ Use this label for the list of [% terms.bugs %] that link to + [%+ terms.abug %] with this + [%+ field_types.${constants.FIELD_TYPE_BUG_ID} FILTER html %] + field. For example, if the description is "Is a duplicate of", + the reverse description would be "Duplicates of this [% terms.bug %]". + Leave blank to disable the list for this field. + @@ -120,19 +138,7 @@ YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('ty - - - - - -
- Use this label for the list of [% terms.bugs %] that link to - [%+ terms.abug %] with this - [%+ field_types.${constants.FIELD_TYPE_BUG_ID} FILTER html %] - field. For example, if the description is "Is a duplicate of", - the reverse description would be "Duplicates of this [% terms.bug %]". - Leave blank to disable the list for this field. - +