Bug 251556: Allow "Bug ID" fields to have one-way mutual relationships (like blocks/dependson)

r=mkanat, a=mkanat

14 files changed, 233 insertions, 26 deletions

diff --git a/.bzrignore b/.bzrignore
index e42398ecc..8415e356b 100644
--- a/.bzrignore
+++ b/.bzrignore
@@ -29,3 +29,4 @@
 /skins/contrib/Dusk/summarize-time.css
 /skins/contrib/Dusk/voting.css
 /skins/contrib/Dusk/yui
+.DS_Store
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index fd28b5b82..1e418aa18 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -25,6 +25,7 @@
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
 #                 Frédéric Buclin <LpSolit@gmail.com>
 #                 Lance Larsh <lance.larsh@oracle.com>
+#                 Elliotte Martin <elliotte_martin@yahoo.com>
 
 package Bugzilla::Bug;
 
@@ -1761,7 +1762,42 @@ sub _check_select_field {
 sub _check_bugid_field {
     my ($invocant, $value, $field) = @_;
     return undef if !$value;
-    return $invocant->check($value, $field)->id;
+    
+    # check that the value is a valid, visible bug id
+    my $checked_id = $invocant->check($value, $field)->id;
+    
+    # check for loop (can't have a loop if this is a new bug)
+    if (ref $invocant) {
+        _check_relationship_loop($field, $invocant->bug_id, $checked_id);
+    }
+
+    return $checked_id;
+}
+
+sub _check_relationship_loop {
+    # Generates a dependency tree for a given bug.  Calls itself recursively
+    # to generate sub-trees for the bug's dependencies.
+    my ($field, $bug_id, $dep_id, $ids) = @_;
+
+    # Don't do anything if this bug doesn't have any dependencies.
+    return unless defined($dep_id);
+
+    # Check whether we have seen this bug yet
+    $ids = {} unless defined $ids;
+    $ids->{$bug_id} = 1;
+    if ($ids->{$dep_id}) {
+        ThrowUserError("relationship_loop_single", {
+            'bug_id' => $bug_id,
+            'dep_id' => $dep_id,
+            'field_name' => $field});
+    }
+    
+    # Get this dependency's record from the database
+    my $dbh = Bugzilla->dbh;
+    my $next_dep_id = $dbh->selectrow_array(
+        "SELECT $field FROM bugs WHERE bug_id = ?", undef, $dep_id);
+
+    _check_relationship_loop($field, $dep_id, $next_dep_id, $ids);
 }
 
 #####################################################################
@@ -2553,6 +2589,15 @@ sub blocked {
 # Even bugs in an error state always have a bug_id.
 sub bug_id { $_[0]->{'bug_id'}; }
 
+sub related_bugs {
+    my ($self, $relationship) = @_;
+    return [] if $self->{'error'};
+
+    my $field_name = $relationship->name;
+    $self->{'related_bugs'}->{$field_name} ||= $self->match({$field_name => $self->id});
+    return $self->{'related_bugs'}->{$field_name}; 
+}
+
 sub cc {
     my ($self) = @_;
     return $self->{'cc'} if exists $self->{'cc'};
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 8bee5dfe1..44d224d57 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -693,6 +693,7 @@ use constant ABSTRACT_SCHEMA => {
             value_field_id => {TYPE => 'INT3',
                                REFERENCES => {TABLE  => 'fielddefs',
                                               COLUMN => 'id'}},
+            reverse_desc => {TYPE => 'TINYTEXT'},
         ],
         INDEXES => [
             fielddefs_name_idx    => {FIELDS => ['name'],
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 2f14037ab..17e4194c2 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -101,6 +101,7 @@ use constant DB_COLUMNS => qw(
     visibility_field_id
     visibility_value_id
     value_field_id
+    reverse_desc
 );
 
 use constant REQUIRED_CREATE_FIELDS => qw(name description);
@@ -120,6 +121,7 @@ use constant VALIDATORS => {
 use constant UPDATE_VALIDATORS => {
     value_field_id      => \&_check_value_field_id,
     visibility_value_id => \&_check_control_value,
+    reverse_desc        => \&_check_reverse_desc,
 };
 
 use constant UPDATE_COLUMNS => qw(
@@ -132,6 +134,7 @@ use constant UPDATE_COLUMNS => qw(
     visibility_field_id
     visibility_value_id
     value_field_id
+    reverse_desc
 
     type
 );
@@ -357,6 +360,22 @@ sub _check_control_value {
     return $value_obj->id;
 }
 
+sub _check_reverse_desc {
+    my ($invocant, $reverse_desc, $type) = @_;
+    
+    if (blessed $invocant) {
+        $type = $invocant->type;
+    }
+    
+    if ($type != FIELD_TYPE_BUG_ID) {
+        return undef; # store NULL for non-reversible field types
+    }
+    
+    $reverse_desc = clean_text($reverse_desc);
+    return $reverse_desc;
+}
+
+
 =pod
 
 =head2 Instance Properties
@@ -637,6 +656,44 @@ sub controls_values_of {
     return $self->{controls_values_of};
 }
 
+=over
+
+=item C<is_relationship>
+
+Applies only to fields of type FIELD_TYPE_BUG_ID.
+Checks to see if a reverse relationship description has been set.
+This is the canonical condition to enable reverse link display,
+dependency tree display, and similar functionality.
+
+=back
+
+=cut
+
+sub is_relationship  {     
+    my $self = shift;
+    my $desc = $self->reverse_desc;
+    if (defined $desc && $desc ne "") {
+        return 1;
+    }
+    return 0;
+}
+
+=over
+
+=item C<reverse_desc>
+
+Applies only to fields of type FIELD_TYPE_BUG_ID.
+Describes the reverse relationship of this field.
+For example, if a BUG_ID field is called "Is a duplicate of",
+the reverse description would be "Duplicates of this bug".
+
+=back
+
+=cut
+
+sub reverse_desc { return $_[0]->{reverse_desc} }
+
+
 =pod
 
 =head2 Instance Mutators
@@ -661,6 +718,8 @@ They will throw an error if you try to set the values to something invalid.
 
 =item C<set_buglist>
 
+=item C<set_reverse_desc>
+
 =item C<set_visibility_field>
 
 =item C<set_visibility_value>
@@ -677,6 +736,7 @@ sub set_obsolete       { $_[0]->set('obsolete',    $_[1]); }
 sub set_sortkey        { $_[0]->set('sortkey',     $_[1]); }
 sub set_in_new_bugmail { $_[0]->set('mailhead',    $_[1]); }
 sub set_buglist        { $_[0]->set('buglist',     $_[1]); }
+sub set_reverse_desc    { $_[0]->set('reverse_desc', $_[1]); }
 sub set_visibility_field {
     my ($self, $value) = @_;
     $self->set('visibility_field_id', $value);
@@ -868,6 +928,12 @@ sub run_create_validators {
         $class->_check_value_field_id($params->{value_field_id},
             ($type == FIELD_TYPE_SINGLE_SELECT 
              || $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0);
+
+    $params->{reverse_desc} = $class->_check_reverse_desc(
+        $params->{reverse_desc}, $type);
+
+
+
     return $params;
 }
 
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index f6d6edcb1..d150a4e9b 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -102,6 +102,9 @@ sub update_fielddefs_definition {
         $dbh->do('UPDATE fielddefs SET buglist = 1 WHERE custom = 1 AND type != ' . FIELD_TYPE_MULTI_SELECT);
     }
 
+    #2008-08-26 elliotte_martin@yahoo.com - Bug 251556
+    $dbh->bz_add_column('fielddefs', 'reverse_desc', {TYPE => 'TINYTEXT'});
+
 
     # Remember, this is not the function for adding general table changes.
     # That is below. Add new changes to the fielddefs table above this
diff --git a/editfields.cgi b/editfields.cgi
index 09147d69a..4e8733752 100755
--- a/editfields.cgi
+++ b/editfields.cgi
@@ -68,6 +68,7 @@ elsif ($action eq 'new') {
         visibility_field_id => scalar $cgi->param('visibility_field_id'),
         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'),
     });
 
     delete_token($token);
@@ -113,6 +114,7 @@ elsif ($action eq 'update') {
     $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'));
+    $field->set_reverse_desc($cgi->param('reverse_desc'));
     $field->update();
 
     delete_token($token);
diff --git a/skins/standard/admin.css b/skins/standard/admin.css
index 184c4961a..6b330ef6d 100644
--- a/skins/standard/admin.css
+++ b/skins/standard/admin.css
@@ -113,3 +113,15 @@ th.title {
     text-align: center;
     vertical-align: middle;
 }
+
+#edit_custom_field tr {
+    vertical-align: top;
+}
+
+#edit_custom_field th {
+    text-align: right;
+}
+
+#edit_custom_field th.narrow_label {
+    white-space: normal;
+}
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 778dd3373..4c95a1690 100644
--- a/template/en/default/admin/custom_fields/cf-js.js.tmpl
+++ b/template/en/default/admin/custom_fields/cf-js.js.tmpl
@@ -50,6 +50,16 @@ function onChangeType(type_field) {
     else {
         value_field.disabled = true;
     }
+
+   var reverse_desc = document.getElementById('reverse_desc');
+   if (type_field.value == [% constants.FIELD_TYPE_BUG_ID %])
+   {
+       reverse_desc.disabled = false;
+   }
+   else {
+       reverse_desc.disabled = true;
+       reverse_desc.value = '';
+   }
 }
 
 function onChangeVisibilityField() {
diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl
index a2db4708b..d23be0fe2 100644
--- a/template/en/default/admin/custom_fields/create.html.tmpl
+++ b/template/en/default/admin/custom_fields/create.html.tmpl
@@ -28,8 +28,14 @@
            onload = "document.getElementById('new_bugmail').disabled = true;"
            javascript_urls = [ 'js/util.js' ]
            doc_section = "custom-fields.html#add-custom-fields"
+           style_urls = ['skins/standard/admin.css']
 %]
 
+[%# set initial editability of fields such as Reverse Relationship Description %]
+<script type="text/javascript">
+YAHOO.util.Event.onDOMReady(function() {onChangeType(document.getElementById('type'))});
+</script>
+
 <p>
   Adding custom fields can make the interface of [% terms.Bugzilla %] very
   complicated. Many admins who are new to [% terms.Bugzilla %] start off
@@ -48,14 +54,14 @@
 </ul>
 
 <form id="add_field" action="editfields.cgi" method="GET">
-  <table border="0" cellspacing="0" cellpadding="5">
+  <table border="0" cellspacing="0" cellpadding="5" id="edit_custom_field">
     <tr>
-      <th align="right"><label for="name">Name:</label></th>
+      <th class="narrow_label"><label for="name">Name:</label></th>
       <td>
         <input type="text" id="name" name="name" value="cf_" size="40" maxlength="64">
       </td>
 
-      <th align="right">
+      <th>
         <label for="enter_bug">Can be set on [% terms.bug %] creation:</label>
       </th>
       <td>
@@ -64,16 +70,16 @@
       </td>
     </tr>
     <tr>
-      <th align="right"><label for="desc">Description:</label></th>
+      <th class="narrow_label"><label for="desc">Description:</label></th>
       <td><input type="text" id="desc" name="desc" value="" size="40"></td>
 
-      <th align="right">
+      <th>
         <label for="new_bugmail">Displayed in [% terms.bug %]mail for new [% terms.bugs %]:</label>
       </th>
       <td><input type="checkbox" id="new_bugmail" name="new_bugmail" value="1"></td>
     </tr>
     <tr>
-      <th align="right"><label for="type">Type:</label></th>
+      <th class="narrow_label"><label for="type">Type:</label></th>
       <td>
         <select id="type" name="type" onchange="onChangeType(this)">
           [% FOREACH type = field_types.keys %]
@@ -83,16 +89,16 @@
         </select>
       </td>
 
-      <th align="right"><label for="obsolete">Is obsolete:</label></th>
+      <th><label for="obsolete">Is obsolete:</label></th>
       <td><input type="checkbox" id="obsolete" name="obsolete" value="1"></td>
     </tr>
     <tr>
-      <th align="right"><label for="sortkey">Sortkey:</label></th>
+      <th class="narrow_label"><label for="sortkey">Sortkey:</label></th>
       <td>
         <input type="text" id="sortkey" name="sortkey" size="6" maxlength="6">
       </td>
 
-      <th align="right">
+      <th>
         <label for="visibility_field_id">Field only appears when:</label>
       </th>
       <td>
@@ -114,7 +120,18 @@
     </tr>
 
     <tr>
-      <td colspan="2">&nbsp;</td>
+      <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 a [% terms.bug %]
+        with this [% field_types.${constants.FIELD_TYPE_BUG_ID} %] 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="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 981668260..01a8dcccb 100644
--- a/template/en/default/admin/custom_fields/edit.html.tmpl
+++ b/template/en/default/admin/custom_fields/edit.html.tmpl
@@ -32,6 +32,7 @@
            onload = "toggleCheckbox(document.getElementById('enter_bug'), 'new_bugmail');"
            javascript_urls = [ 'js/util.js' ]
            doc_section = "custom-fields.html#edit-custom-fields"
+           style_urls = ['skins/standard/admin.css']
 %]
 
 <p>
@@ -40,12 +41,12 @@
 </p>
 
 <form id="edit_field" action="editfields.cgi" method="GET">
-  <table border="0" cellspacing="0" cellpadding="5">
+  <table border="0" cellspacing="0" cellpadding="5" id="edit_custom_field">
     <tr>
-      <th align="right">Name:</th>
+      <th class="narrow_label">Name:</th>
       <td>[% field.name FILTER html %]</td>
 
-      <th align="right">
+      <th>
         <label for="enter_bug">Can be set on [% terms.bug %] creation:</label>
       </th>
       <td><input type="checkbox" id="enter_bug" name="enter_bug" value="1"
@@ -53,31 +54,31 @@
                  onchange="toggleCheckbox(this, 'new_bugmail');"></td>
     </tr>
     <tr>
-      <th align="right"><label for="desc">Description:</label></th>
+      <th class="narrow_label"><label for="desc">Description:</label></th>
       <td><input type="text" id="desc" name="desc" size="40"
                  value="[% field.description FILTER html %]"></td>
 
-      <th align="right">
+      <th>
         <label for="new_bugmail">Displayed in [% terms.bug %]mail for new [% terms.bugs %]:</label>
       </th>
       <td><input type="checkbox" id="new_bugmail" name="new_bugmail" value="1"
                  [%- " checked" IF field.mailhead %]></td>
     </tr>
     <tr>
-      <th align="right">Type:</th>
+      <th class="narrow_label">Type:</th>
       <td>[% field_types.${field.type} FILTER html %]</td>
 
-      <th align="right"><label for="obsolete">Is obsolete:</label></th>
+      <th><label for="obsolete">Is obsolete:</label></th>
       <td><input type="checkbox" id="obsolete" name="obsolete" value="1"
                  [%- " checked" IF field.obsolete %]></td>
     </tr>
     <tr>
-      <th align="right"><label for="sortkey">Sortkey:</label></th>
+      <th class="narrow_label"><label for="sortkey">Sortkey:</label></th>
       <td>
         <input type="text" id="sortkey" name="sortkey" size="6" maxlength="6"
                value="[% field.sortkey FILTER html %]">
       </td>
-      <th align="right">
+      <th>
         <label for="visibility_field_id">Field only appears when:</label>
       </th>
       <td>
@@ -140,6 +141,23 @@
         </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 a [% terms.bug %]
+          with this [% field_types.${constants.FIELD_TYPE_BUG_ID} %] 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/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index 63b81d733..1a35df916 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -888,10 +888,16 @@
   [% USE Bugzilla %]
   [% FOREACH field = Bugzilla.active_custom_fields %]
     <tr>
-      [% PROCESS bug/field.html.tmpl value=bug.${field.name}
+      [% PROCESS bug/field.html.tmpl value = bug.${field.name}
                                      editable = bug.check_can_change_field(field.name, 0, 1)
                                      value_span = 2 %]
     </tr>
+    [% IF extra_field_item %]
+      <tr>
+        <th class="field_label">[% extra_field_item.header FILTER none %]</th>
+        <td>[% extra_field_item.data FILTER none %]</td>
+      </tr>
+    [% END %]
   [% END %]
 [% END %]
 
diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl
index ac62bf7ba..482f88219 100644
--- a/template/en/default/bug/field.html.tmpl
+++ b/template/en/default/bug/field.html.tmpl
@@ -99,8 +99,8 @@
                  value="[% value FILTER html %]" size="7">
         </span>
 
-        [% IF bug.${field.name} %]  
-          [% bug.${field.name} FILTER bug_link(bug.${field.name}) FILTER none %]
+        [% IF value %]  
+          [% value FILTER bug_link(value, use_alias => 1) FILTER none %]
         [% END %]
         <span id="[% field.name FILTER html %]_edit_container" class="edit_me bz_default_hidden">
           (<a href="#" id="[% field.name FILTER html %]_edit_action">edit</a>)
@@ -110,7 +110,7 @@
                           '[% field.name FILTER js %]_input_area',
                           '[% field.name FILTER js %]_edit_action',
                           '[% field.name FILTER js %]',
-                          "[% bug.${field.name} FILTER js %]");
+                          "[% value FILTER js %]");
         </script>
     [% CASE [ constants.FIELD_TYPE_SINGLE_SELECT 
               constants.FIELD_TYPE_MULTI_SELECT ] %]
@@ -198,11 +198,24 @@
   <div class="uneditable_textarea">[% value FILTER wrap_comment(60)
                                             FILTER html %]</div>
 [% ELSIF field.type == constants.FIELD_TYPE_BUG_ID %]
-    [% IF bug.${field.name} %]  
-        [% bug.${field.name} FILTER bug_link(bug.${field.name}) FILTER none %]
+    [% IF value %]  
+        [% value FILTER bug_link(value, use_alias => 1) FILTER none %]
     [% END %]
 [% ELSE %]
   [% value.join(', ') FILTER html %]
 [% END %]
 [% Hook.process('end_field_column') %]
 [% '</td>' IF NOT no_tds %]
+
+[%# for reverse relationships, we show this pseudo-field after the main field %]
+[% IF bug.id && field.is_relationship %]
+    [% extra_field_item = {} %]
+    [% extra_field_item.header = field.reverse_desc _ ":" FILTER html %]
+    [% extra_field_item.data = BLOCK %]
+        [% FOREACH depbug = bug.related_bugs(field) %]
+            [% depbug.id FILTER bug_link(depbug, use_alias => 1) FILTER none %][% " " %]
+        [% END %]
+    [% END %]
+[% ELSE %]
+    [% extra_field_item = '' %]
+[% END %]
diff --git a/template/en/default/bug/show-multiple.html.tmpl b/template/en/default/bug/show-multiple.html.tmpl
index 177bea14f..0949ffc20 100644
--- a/template/en/default/bug/show-multiple.html.tmpl
+++ b/template/en/default/bug/show-multiple.html.tmpl
@@ -186,6 +186,13 @@
         [% PROCESS bug/field.html.tmpl value=bug.${field.name} editable=0 %]
         [%# Even-numbered fields get a closing <tr> %]
         [% '</tr>' IF !(field_counter % 2) %]
+        [% IF extra_field_item %]
+          [% field_counter = field_counter + 1 %]
+          [% '<tr>' IF field_counter % 2 %]
+          <th>[% extra_field_item.header FILTER none %]</th>
+          <td>[% extra_field_item.data FILTER none %]</td>
+          [% '</tr>' IF !(field_counter % 2) %]
+        [% END %]
     [% END %]
     [%# And we have to finish the row if we ended on an odd number. %]
     [% '<th></th><td></td></tr>' IF field_counter % 2 %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index eca7ef058..bdedc4172 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1450,6 +1450,12 @@
     To file this [% terms.bug %], you must first choose a component.
     If necessary, just guess.
 
+  [% ELSIF error == "relationship_loop_single" %]
+    [% title = "Relationship Loop Detected" %]
+    [% field_descs.$field_name FILTER html %]
+    for [% terms.bug %] [%+ bug_id FILTER html %]
+    has a circular dependency on [% terms.bug %] [%+ dep_id FILTER html %].
+
   [% ELSIF error == "require_new_password" %]
     [% title = "New Password Needed" %]
     You cannot change your password without choosing a new one.