Commit 44bc791e authored by Tiago Mello's avatar Tiago Mello Committed by Max Kanat-Alexander

Bug 494395: Implement the ability to mark custom fields as mandatory when

creating/changing bugs r=mkanat, a=mkanat
parent d614dd98
......@@ -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);
}
......
......@@ -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(
......
......@@ -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'],
],
},
......
......@@ -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<is_visible_on_bug>
See L<Bugzilla::Field::ChoiceInterface>.
=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<is_relationship>
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<is_mandatory>
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<set_value_field>
=item C<set_is_mandatory>
=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<obsolete> - boolean - Whether this field is obsolete. Defaults to 0.
C<is_mandatory> - 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 {
......
......@@ -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.
......
......@@ -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'));
......
......@@ -98,6 +98,24 @@ YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('ty
<input type="text" id="sortkey" name="sortkey" size="6" maxlength="6">
</td>
<th align="right"><label for="is_mandatory">Is mandatory:</label></th>
<td><input type="checkbox" id="is_mandatory" name="is_mandatory" value="1"></td>
</tr>
<tr>
<th class="narrow_label">
<label for="reverse_desc">Reverse Relationship Description:</label>
</th>
<td>
<input type="text" id="reverse_desc" name="reverse_desc" value="" size="40" disabled="disabled">
<br/>
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.
</td>
<th>
<label for="visibility_field_id">Field only appears when:</label>
</th>
......@@ -120,19 +138,7 @@ YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('ty
</tr>
<tr>
<th class="narrow_label">
<label for="reverse_desc">Reverse Relationship Description:</label>
</th>
<td>
<input type="text" id="reverse_desc" name="reverse_desc" value="" size="40" disabled="disabled">
<br/>
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.
</td>
<td colspan="2">&nbsp;</td>
<th>
<label for="value_field_id">
Field that controls the values<br>
......
......@@ -78,6 +78,29 @@
<input type="text" id="sortkey" name="sortkey" size="6" maxlength="6"
value="[% field.sortkey FILTER html %]">
</td>
<th align="right"><label for="is_mandatory">Is mandatory:</label></th>
<td><input type="checkbox" id="is_mandatory" name="is_mandatory" value="1"
[%- ' checked="checked"' IF field.is_mandatory %]></td>
</tr>
<tr>
[% IF field.type == constants.FIELD_TYPE_BUG_ID %]
<th class="narrow_label">
<label for="reverse_desc">Reverse Relationship Description:</label>
</th>
<td>
<input type="text" id="reverse_desc" name="reverse_desc" size="40"
value="[% field.reverse_desc FILTER html %]">
<br/>
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.
</td>
[% ELSE %]
<td colspan="2">&nbsp;</td>
[% END %]
<th>
<label for="visibility_field_id">Field only appears when:</label>
</th>
......@@ -141,24 +164,6 @@
</td>
</tr>
[% END %]
[% IF field.type == constants.FIELD_TYPE_BUG_ID %]
<tr>
<th class="narrow_label">
<label for="reverse_desc">Reverse Relationship Description:</label>
</th>
<td>
<input type="text" id="reverse_desc" name="reverse_desc" size="40"
value="[% field.reverse_desc FILTER html %]">
<br/>
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.
</td>
</tr>
[% END %]
</table>
<br>
<input type="hidden" name="action" value="update">
......
......@@ -57,6 +57,10 @@
heading => "Is Obsolete"
},
{
name => "is_mandatory"
heading => "Is Mandatory"
},
{
name => "action"
heading => "Action"
content => ""
......
......@@ -218,7 +218,7 @@ TUI_hide_default('expert_fields');
describecomponents.cgi?product=[% product.name FILTER url_quote %]
[% END %]
[% INCLUDE "bug/field-label.html.tmpl"
field = bug_fields.component editable = 1 required = 1
field = bug_fields.component editable = 1
desc_url = component_desc_url
%]
<td id="field_container_component">
......@@ -487,13 +487,13 @@ TUI_hide_default('expert_fields');
</tr>
</tbody>
<tbody class="expert_fields">
<tbody>
[% USE Bugzilla %]
[% FOREACH field = Bugzilla.active_custom_fields %]
[% NEXT UNLESS field.enter_bug %]
[% SET value = ${field.name}.defined ? ${field.name} : "" %]
<tr>
<tr [% 'class="expert_fields"' IF !field.is_mandatory %]>
[% INCLUDE bug/field.html.tmpl
bug = default, field = field, value = value, editable = 1,
value_span = 3 %]
......@@ -505,7 +505,7 @@ TUI_hide_default('expert_fields');
<tr>
[% INCLUDE "bug/field-label.html.tmpl"
field = bug_fields.short_desc editable = 1 required = 1
field = bug_fields.short_desc editable = 1
%]
<td colspan="3">
<input name="short_desc" size="70" value="[% short_desc FILTER html %]"
......
......@@ -22,14 +22,13 @@
# field: a Bugzilla::Field object
# desc_url: An alternate link to help for the field.
# hidden: True if the field label should start hidden.
# required: True if this field must have a value.
# rowspan: a "rowspan" value for the label's <th>.
#%]
[% PROCESS "bug/field-help.none.tmpl" %]
<th class="field_label [% ' bz_hidden_field' IF hidden %]
[%- ' required' IF required %]"
[%- ' required' IF field.is_mandatory %]"
id="field_label_[% field.name FILTER html %]"
[% IF rowspan %] rowspan="[% rowspan FILTER html %]"[% END %]>
......
......@@ -48,12 +48,9 @@
[% IF NOT no_tds %]
[% PROCESS "bug/field-label.html.tmpl" %]
[% END %]
[% IF NOT no_tds %]
<td class="field_value [% ' bz_hidden_field' IF hidden %]"
id="field_container_[% field.name FILTER html %]"
[% " colspan=\"$value_span\"" FILTER none IF value_span %]>
<td class="field_value [% ' bz_hidden_field' IF hidden %]"
id="field_container_[% field.name FILTER html %]"
[% " colspan=\"$value_span\"" FILTER none IF value_span %]>
[% END %]
[% Hook.process('start_field_column') %]
[% IF editable %]
......@@ -62,11 +59,13 @@
<input id="[% field.name FILTER html %]" class="text_input"
name="[% field.name FILTER html %]"
value="[% value FILTER html %]" size="40"
maxlength="[% constants.MAX_FREETEXT_LENGTH FILTER none %]">
maxlength="[% constants.MAX_FREETEXT_LENGTH FILTER none %]"
[% ' aria-required="true"' IF field.is_mandatory %]>
[% CASE constants.FIELD_TYPE_DATETIME %]
<input name="[% field.name FILTER html %]" size="20"
id="[% field.name FILTER html %]"
value="[% value FILTER html %]"
[% ' aria-required="true"' IF field.is_mandatory %]
onchange="updateCalendarFromField(this)">
<button type="button" class="calendar_button"
id="button_calendar_[% field.name FILTER html %]"
......@@ -82,7 +81,9 @@
[% CASE constants.FIELD_TYPE_BUG_ID %]
<span id="[% field.name FILTER html %]_input_area">
<input name="[% field.name FILTER html %]" id="[% field.name FILTER html %]"
value="[% value FILTER html %]" size="7">
value="[% value FILTER html %]" size="7"
[% ' aria-required="true"' IF field.is_mandatory %]>
</span>
[% IF value %]
......@@ -108,6 +109,7 @@
[% SET field_size = field.legal_values.size %]
[% END %]
size="[% field_size FILTER html %]" multiple="multiple"
[% ' aria-required="true"' IF field.is_mandatory %]
[% END %]
>
[% IF allow_dont_change %]
......@@ -157,7 +159,7 @@
[% CASE constants.FIELD_TYPE_TEXTAREA %]
[% INCLUDE global/textarea.html.tmpl
id = field.name name = field.name minrows = 4 maxrows = 8
cols = 60 defaultcontent = value %]
cols = 60 defaultcontent = value mandatory = field.is_mandatory %]
[% CASE constants.FIELD_TYPE_BUG_URLS %]
[% '<ul class="bug_urls">' IF value.size %]
[% FOREACH url = value %]
......
......@@ -31,6 +31,8 @@
# minrows will be used.
# cols: (required) Number of columns the textarea shall have.
# defaultcontent: (optional) Default content for the textarea.
# mandatory: (optional) Boolean specifying whether or not the textarea
# is mandatory.
#%]
<textarea [% IF name %]name="[% name FILTER html %]"[% END %]
......@@ -47,4 +49,7 @@
cols="[% cols FILTER html %]"
[% IF maxrows && user.settings.zoom_textareas.value == 'on' %]
onFocus="this.rows=[% maxrows FILTER html %]"
[% END %]
[% IF mandatory %]
aria-required="true"
[% END %]>[% defaultcontent FILTER html %]</textarea>
......@@ -1473,6 +1473,11 @@
[% title = "New Password Needed" %]
You cannot change your password without choosing a new one.
[% ELSIF error == "required_field" %]
[% title = "Field Must Be Set" %]
A value must be set for the '[% field_descs.${field.name} FILTER html %]'
field.
[% ELSIF error == "require_summary" %]
[% title = "Summary Needed" %]
You must enter a summary for this [% terms.bug %].
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment