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

creating/changing bugs
r=mkanat, a=mkanat

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<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 {
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
         <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>
diff --git a/template/en/default/admin/custom_fields/edit.html.tmpl b/template/en/default/admin/custom_fields/edit.html.tmpl
index ebf0891c8..4c1bbbeb0 100644
--- a/template/en/default/admin/custom_fields/edit.html.tmpl
+++ b/template/en/default/admin/custom_fields/edit.html.tmpl
@@ -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">
diff --git a/template/en/default/admin/custom_fields/list.html.tmpl b/template/en/default/admin/custom_fields/list.html.tmpl
index dd266c759..385650a63 100644
--- a/template/en/default/admin/custom_fields/list.html.tmpl
+++ b/template/en/default/admin/custom_fields/list.html.tmpl
@@ -57,6 +57,10 @@
        heading => "Is Obsolete"
      },
      {
+       name => "is_mandatory"
+       heading => "Is Mandatory"
+     },
+     {
        name => "action"
        heading => "Action"
        content => ""
diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl
index 983f12bb9..39f14e8de 100644
--- a/template/en/default/bug/create/create.html.tmpl
+++ b/template/en/default/bug/create/create.html.tmpl
@@ -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 %]"
diff --git a/template/en/default/bug/field-label.html.tmpl b/template/en/default/bug/field-label.html.tmpl
index c3a282701..7b63f7b8c 100644
--- a/template/en/default/bug/field-label.html.tmpl
+++ b/template/en/default/bug/field-label.html.tmpl
@@ -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 %]>
 
diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl
index 211f16b8e..97d38661c 100644
--- a/template/en/default/bug/field.html.tmpl
+++ b/template/en/default/bug/field.html.tmpl
@@ -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 %]
diff --git a/template/en/default/global/textarea.html.tmpl b/template/en/default/global/textarea.html.tmpl
index b762f1c4f..1d8cacafb 100644
--- a/template/en/default/global/textarea.html.tmpl
+++ b/template/en/default/global/textarea.html.tmpl
@@ -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>
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 279f29c71..c5667bd27 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -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 %].