Finalize ticket relations (closes issue 638)

- IssueUpdate.php: use dynamically set field validators for dynamically
  created fields; let relation_type0 and relation_issue0 exist at any time;
  check the validity of a user selection and combine the various input fields
  if possible; do the database updates for links; change the "change" format
  for labels to a more precise structure and no longer trust on a leading
  dash for removed labels
- IssueCreate.php: change the validator calls and field names
- Issue.php (getGroupedRelatedIssues): make it possible to return only a
  flat list of integers for easier processing
- 17AddIssueRelations.php: migrate the previous serialized "changes" format
  for issue comments to the new, more structured format (up and down)
- js-autocomplete.html: add support for multiple input fields
- view.html: output relation changes and wrap the related issues stanzas into
  paragraphs
- NEWS.mdtext: note the addition and the need for a specific version of Pluf

NEWS.mdtext

@@ -1,7 +1,13 @@
 # InDefero 1.2 - xxx xxx xx xx:xx 2011 UTC
 
+ATTENTION: You need Pluf [324ae60b](http://projects.ceondo.com/p/pluf/source/commit/324ae60b)
+or newer to properly run this version of Indefero!
+
 ## New Features
 
+- Indefero's issue tracker can now bi-directionally link issues with variable, configurable
+  terms, such as "is related to", "is blocked by" or "is duplicated by" (issue 638)
+
 ## Bugfixes
 
 - monotone zip archive entries now all carry the revision date as mtime (issue 645)

src/IDF/Form/IssueCreate.php

@@ -113,14 +113,14 @@ class IDF_Form_IssueCreate extends Pluf_Form
                                                                     ),
                                             ));
 
-            $this->fields['relation_type'] = new Pluf_Form_Field_Varchar(
+            $this->fields['relation_type0'] = new Pluf_Form_Field_Varchar(
                           array('required' => false,
                                 'label' => __('This issue'),
                                 'initial' => current($this->relation_types),
                                 'widget_attrs' => array('size' => 15),
                                 ));
 
-            $this->fields['relation_issue'] = new Pluf_Form_Field_Varchar(
+            $this->fields['relation_issue0'] = new Pluf_Form_Field_Varchar(
                           array('required' => false,
                                 'label' => null,
                                 'initial' => '',
@@ -253,9 +253,11 @@ class IDF_Form_IssueCreate extends Pluf_Form
         return $this->cleaned_data['status'];
     }
 
-    function clean_relation_type()
+    // this method is not called from Pluf_Form directly, but shared for
+    // among all similar fields
+    function clean_relation_type($value)
     {
-        $relation_type = trim($this->cleaned_data['relation_type']);
+        $relation_type = trim($value);
         if (empty($relation_type))
             return '';
 
@@ -272,9 +274,16 @@ class IDF_Form_IssueCreate extends Pluf_Form
         return $relation_type;
     }
 
-    function clean_relation_issue()
+    function clean_relation_type0()
     {
-        $issues = trim($this->cleaned_data['relation_issue']);
+        return $this->clean_relation_type($this->cleaned_data['relation_type0']);
+    }
+
+    // this method is not called from Pluf_Form directly, but shared for
+    // among all similar fields
+    function clean_relation_issue($value)
+    {
+        $issues = trim($value);
         if (empty($issues))
             return '';
 
@@ -296,6 +305,11 @@ class IDF_Form_IssueCreate extends Pluf_Form
         return implode(', ', $issue_ids);
     }
 
+    function clean_relation_issue0()
+    {
+        return $this->clean_relation_issue($this->cleaned_data['relation_issue0']);
+    }
+
     /**
      * Clean the attachments post failure.
      */
@@ -360,9 +374,9 @@ class IDF_Form_IssueCreate extends Pluf_Form
             $issue->setAssoc($tag);
         }
         // add relations
-        $verb = $this->cleaned_data['relation_type'];
+        $verb = $this->cleaned_data['relation_type0'];
         $other_verb = $this->relation_types[$verb];
-        $related_issues = preg_split('/\s*,\s*/', $this->cleaned_data['relation_issue'], -1, PREG_SPLIT_NO_EMPTY);
+        $related_issues = preg_split('/\s*,\s*/', $this->cleaned_data['relation_issue0'], -1, PREG_SPLIT_NO_EMPTY);
         foreach ($related_issues as $related_issue_id) {
             $related_issue = new IDF_Issue($related_issue_id);
             $rel = new IDF_IssueRelation();

src/IDF/Form/IssueUpdate.php

@@ -104,20 +104,51 @@ class IDF_Form_IssueUpdate  extends IDF_Form_IssueCreate
                                                                     ),
                                             ));
 
-            $this->fields['relation_type'] = new Pluf_Form_Field_Varchar(
+            $idx = 0;
+            // note: clean_relation_type0 and clean_relation_issue0 already
+            //       exist in the base class
+            $this->fields['relation_type'.$idx] = new Pluf_Form_Field_Varchar(
                           array('required' => false,
                                 'label' => __('This issue'),
                                 'initial' => current($this->relation_types),
                                 'widget_attrs' => array('size' => 15),
                                 ));
 
-            $this->fields['relation_issue'] = new Pluf_Form_Field_Varchar(
+            $this->fields['relation_issue'.$idx] = new Pluf_Form_Field_Varchar(
                           array('required' => false,
                                 'label' => null,
                                 'initial' => '',
                                 'widget_attrs' => array('size' => 10),
                                 ));
 
+            ++$idx;
+            $relatedIssues = $this->issue->getGroupedRelatedIssues(array(), true);
+            foreach ($relatedIssues as $verb => $ids) {
+                $this->fields['relation_type'.$idx] = new Pluf_Form_Field_Varchar(
+                          array('required' => false,
+                                'label' => __('This issue'),
+                                'initial' => $verb,
+                                'widget_attrs' => array('size' => 15),
+                                ));
+                $m = 'clean_relation_type'.$idx;
+                $this->$m = create_function('$form', '
+                    return $form->clean_relation_type($form->cleaned_data["relation_type'.$idx.'"]);
+                ');
+
+                $this->fields['relation_issue'.$idx] = new Pluf_Form_Field_Varchar(
+                              array('required' => false,
+                                    'label' => null,
+                                    'initial' => implode(', ', $ids),
+                                    'widget_attrs' => array('size' => 10),
+                                    ));
+                $m = 'clean_relation_issue'.$idx;
+                $this->$m = create_function('$form', '
+                    return $form->clean_relation_issue($form->cleaned_data["relation_issue'.$idx.'"]);
+                ');
+
+                ++$idx;
+            }
+
             $tags = $this->issue->get_tags_list();
             for ($i=1;$i<7;$i++) {
                 $initial = '';
@@ -171,6 +202,48 @@ class IDF_Form_IssueUpdate  extends IDF_Form_IssueCreate
     public function clean()
     {
         $this->cleaned_data = parent::clean();
+
+        // normalize the user's input by removing dublettes and by combining
+        // ids from identical verbs in different input fields into one array
+        $normRelatedIssues = array();
+        for ($idx = 0; isset($this->cleaned_data['relation_type'.$idx]); ++$idx) {
+            $verb = $this->cleaned_data['relation_type'.$idx];
+            $ids = preg_split('/\s*,\s*/', $this->cleaned_data['relation_issue'.$idx],
+                              -1, PREG_SPLIT_NO_EMPTY);
+            if (count($ids) == 0)
+                continue;
+
+            if (!array_key_exists($verb, $normRelatedIssues))
+                $normRelatedIssues[$verb] = array();
+            foreach ($ids as $id) {
+                if (!in_array($id, $normRelatedIssues[$verb]))
+                    $normRelatedIssues[$verb][] = $id;
+            }
+        }
+
+        // now look at any added / removed ids
+        $added = $removed = array();
+        $relatedIssues = $this->issue->getGroupedRelatedIssues(array(), true);
+        $added = array_diff_key($normRelatedIssues, $relatedIssues);
+        $removed = array_diff_key($relatedIssues, $normRelatedIssues);
+
+        $keysToLookAt = array_keys(
+            array_intersect_key($relatedIssues, $normRelatedIssues)
+        );
+        foreach ($keysToLookAt as $key) {
+            $a = array_diff($normRelatedIssues[$key], $relatedIssues[$key]);
+            if (count($a) > 0)
+                $added[$key] = $a;
+            $r = array_diff($relatedIssues[$key], $normRelatedIssues[$key]);
+            if (count($r) > 0)
+                $removed[$key] = $r;
+        }
+
+        // cache the added / removed data, so we do not have to
+        // calculate that again
+        $this->cleaned_data['_added_issue_relations'] = $added;
+        $this->cleaned_data['_removed_issue_relations'] = $removed;
+
         // As soon as we know that at least one change was done, we
         // return the cleaned data and do not go further.
         if (strlen(trim($this->cleaned_data['content']))) {
@@ -230,6 +303,11 @@ class IDF_Form_IssueUpdate  extends IDF_Form_IssueCreate
                     return $this->cleaned_data;
                 }
             }
+
+            if (count($this->cleaned_data['_added_issue_relations']) != 0 ||
+                count($this->cleaned_data['_removed_issue_relations']) != 0) {
+                return $this->cleaned_data;
+            }
         }
         // no changes!
         throw new Pluf_Form_Invalid(__('No changes were entered.'));
@@ -271,20 +349,22 @@ class IDF_Form_IssueUpdate  extends IDF_Form_IssueCreate
             foreach ($tags as $tag) {
                 if (!Pluf_Model_InArray($tag, $oldtags)) {
                     if (!isset($changes['lb'])) $changes['lb'] = array();
+                    if (!isset($changes['lb']['add'])) $changes['lb']['add'] = array();
                     if ($tag->class != 'Other') {
-                        $changes['lb'][] = (string) $tag; //new tag
+                        $changes['lb']['add'][] = (string) $tag; //new tag
                     } else {
-                        $changes['lb'][] = (string) $tag->name;
+                        $changes['lb']['add'][] = (string) $tag->name;
                     }
                 }
             }
             foreach ($oldtags as $tag) {
                 if (!Pluf_Model_InArray($tag, $tags)) {
                     if (!isset($changes['lb'])) $changes['lb'] = array();
+                    if (!isset($changes['lb']['rem'])) $changes['lb']['rem'] = array();
                     if ($tag->class != 'Other') {
-                        $changes['lb'][] = '-'.(string) $tag; //new tag
+                        $changes['lb']['rem'][] = (string) $tag; //new tag
                     } else {
-                        $changes['lb'][] = '-'.(string) $tag->name;
+                        $changes['lb']['rem'][] = (string) $tag->name;
                     }
                 }
             }
@@ -302,6 +382,47 @@ class IDF_Form_IssueUpdate  extends IDF_Form_IssueCreate
                 or ((!is_null($owner) and !is_null($this->issue->get_owner())) and $owner->id != $this->issue->get_owner()->id)) {
                 $changes['ow'] = (is_null($owner)) ? '---' : $owner->login;
             }
+            // Issue relations - additions
+            foreach ($this->cleaned_data['_added_issue_relations'] as $verb => $ids) {
+                $other_verb = $this->relation_types[$verb];
+                foreach ($ids as $id) {
+                    $related_issue = new IDF_Issue($id);
+                    $rel = new IDF_IssueRelation();
+                    $rel->issue = $this->issue;
+                    $rel->verb = $verb;
+                    $rel->other_issue = $related_issue;
+                    $rel->submitter = $this->user;
+                    $rel->create();
+
+                    $other_rel = new IDF_IssueRelation();
+                    $other_rel->issue = $related_issue;
+                    $other_rel->verb = $other_verb;
+                    $other_rel->other_issue = $this->issue;
+                    $other_rel->submitter = $this->user;
+                    $other_rel->create();
+                }
+                if (!isset($changes['rel'])) $changes['rel'] = array();
+                if (!isset($changes['rel']['add'])) $changes['rel']['add'] = array();
+                $changes['rel']['add'][] = $verb.' '.implode(', ', $ids);
+            }
+            // Issue relations - removals
+            foreach ($this->cleaned_data['_removed_issue_relations'] as $verb => $ids) {
+                foreach ($ids as $id) {
+                    $db = &Pluf::db();
+                    $table = Pluf::factory('IDF_IssueRelation')->getSqlTable();
+                    $sql = new Pluf_SQL('verb=%s AND (
+                                        (issue=%s AND other_issue=%s) OR
+                                        (other_issue=%s AND issue=%s))',
+                                        array($verb,
+                                              $this->issue->id, $id,
+                                              $this->issue->id, $id));
+                    $db->execute('DELETE FROM '.$table.' WHERE '.$sql->gen());
+                }
+
+                if (!isset($changes['rel'])) $changes['rel'] = array();
+                if (!isset($changes['rel']['rem'])) $changes['rel']['rem'] = array();
+                $changes['rel']['rem'][] = $verb.' '.implode(', ', $ids);
+            }
             // Update the issue
             $this->issue->batchAssoc('IDF_Tag', $tagids);
             $this->issue->summary = trim($this->cleaned_data['summary']);

src/IDF/Issue.php

@@ -169,7 +169,7 @@ class IDF_Issue extends Pluf_Model
         }
     }
 
-    function getGroupedRelatedIssues($opts = array())
+    function getGroupedRelatedIssues($opts = array(), $idsOnly = false)
     {
         $rels = $this->get_related_issues_list(array_merge($opts, array(
                'view' => 'with_other_issue',
@@ -181,7 +181,7 @@ class IDF_Issue extends Pluf_Model
             if (!array_key_exists($verb, $res)) {
                 $res[$verb] = array();
             }
-            $res[$verb][] = $rel;
+            $res[$verb][] = $idsOnly ? $rel->other_issue : $rel;
         }
 
         return $res;

src/IDF/IssueComment.php

@@ -155,10 +155,18 @@ class IDF_IssueComment extends Pluf_Model
                     $out .= __('Owner:'); break;
                 case 'lb':
                     $out .= __('Labels:'); break;
+                case 'rel':
+                    $out .= __('Relations:'); break;
                 }
                 $out .= '</strong>&nbsp;';
-                if ($w == 'lb') {
+                if ($w == 'lb' || $w == 'rel') {
-                    $out .= Pluf_esc(implode(', ', $v));
+                    foreach ($v as $t => $ls) {
+                        foreach ($ls as $l) {
+                            if ($t == 'rem') $out .= '<s>';
+                            $out .= Pluf_esc($l);
+                            if ($t == 'rem') $out .= '</s> ';
+                        }
+                    }
                 } else {
                     $out .= Pluf_esc($v);
                 }

src/IDF/Migrations/17AddIssueRelations.php

@@ -32,6 +32,27 @@ function IDF_Migrations_17AddIssueRelations_up($params=null)
     $schema = new Pluf_DB_Schema($db);
     $schema->model = new IDF_IssueRelation();
     $schema->createTables();
+
+    // change the serialization format for added / removed labels in IDF_IssueComment
+    $comments = Pluf::factory('IDF_IssueComment')->getList();
+    foreach ($comments as $comment) {
+        if (!isset($comment->changes['lb'])) continue;
+        $changes = $comment->changes;
+        $adds = $removals = array();
+        foreach ($comment->changes['lb'] as $lb) {
+            if (substr($lb, 0, 1) == '-')
+                $removals[] = substr($lb, 1);
+            else
+                $adds[] = $lb;
+        }
+        $changes['lb'] = array();
+        if (count($adds) > 0)
+            $changes['lb']['add'] = $adds;
+        if (count($removals) > 0)
+            $changes['lb']['rem'] = $removals;
+        $comment->changes = $changes;
+        $comment->update();
+    }
 }
 
 function IDF_Migrations_17AddIssueRelations_down($params=null)
@@ -40,5 +61,30 @@ function IDF_Migrations_17AddIssueRelations_down($params=null)
     $schema = new Pluf_DB_Schema($db);
     $schema->model = new IDF_IssueRelation();
     $schema->dropTables();
+
+    // change the serialization format for added / removed labels in IDF_IssueComment
+    $comments = Pluf::factory('IDF_IssueComment')->getList();
+    foreach ($comments as $comment) {
+        $changes = $comment->changes;
+        if (empty($changes))
+            continue;
+        if (isset($changes['lb'])) {
+            $labels = array();
+            foreach ($changes['lb'] as $type => $lbs) {
+                if (!is_array($lbs)) {
+                    $labels[] = $lbs;
+                    continue;
+                }
+                foreach ($lbs as $lb) {
+                    $labels[] = ($type == 'rem' ? '-' : '') . $lb;
+                }
+            }
+            $changes['lb'] = $labels;
+        }
+        // while we're at it, remove any 'rel' changes
+        unset($changes['rel']);
+        $comment->changes = $changes;
+        $comment->update();
+    }
 }
 

src/IDF/Views/Issue.php

@@ -404,7 +404,7 @@ class IDF_Views_Issue
         $issue = Pluf_Shortcuts_GetObjectOr404('IDF_Issue', $match[2]);
         $prj->inOr404($issue);
         $comments = $issue->get_comments_list(array('order' => 'id ASC'));
-        $related_issues = $issue->getGroupedRelatedIssues(array('order' => 'creation_dtime DESC'));
+        $related_issues = $issue->getGroupedRelatedIssues(array('order' => 'other_issue ASC'));
 
         $url = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::view',
                                         array($prj->shortname, $issue->id));

src/IDF/templates/idf/issues/create.html

@@ -63,12 +63,12 @@
 </td>
 </tr>
 <tr>
-<th>{$form.f.relation_type.labelTag}:</th>
+<th>{$form.f.relation_type0.labelTag}:</th>
 <td>
-{if $form.f.relation_type.errors}{$form.f.relation_type.fieldErrors}{/if}
+{if $form.f.relation_type0.errors}{$form.f.relation_type0.fieldErrors}{/if}
-{if $form.f.relation_issue.errors}{$form.f.relation_issue.fieldErrors}{/if}
+{if $form.f.relation_issue0.errors}{$form.f.relation_issue0.fieldErrors}{/if}
-{$form.f.relation_type|unsafe}
+{$form.f.relation_type0|unsafe}
-{$form.f.relation_issue|unsafe}
+{$form.f.relation_issue0|unsafe}
 </td>
 </tr>
 <tr>

src/IDF/templates/idf/issues/js-autocomplete.html

@@ -51,7 +51,11 @@
             return row.to;
         }
     });
-  $("#id_relation_type").autocomplete(auto_relation_types, {
+  for (var idx = 0; ; ++idx) {
+      if ($("#id_relation_type" + idx).length == 0)
+          break;
+
+      $("#id_relation_type" + idx).autocomplete(auto_relation_types, {
             minChars: 0,
             width: 310,
             matchContains: true,
@@ -64,7 +68,7 @@
                 return row.to;
             }
         });
-  $("#id_relation_issue").autocomplete("{/literal}{url 'IDF_Views_Issue::autoCompleteIssueList', array($project.shortname, $issue.id)}{literal}", {
+      $("#id_relation_issue" + idx).autocomplete("{/literal}{url 'IDF_Views_Issue::autoCompleteIssueList', array($project.shortname, $issue.id)}{literal}", {
             minChars: 0,
             width: 310,
             matchContains: true,
@@ -79,6 +83,7 @@
                 return row[1];
             }
         });
+    }
   });
 {/literal} //-->
 </script>

src/IDF/templates/idf/issues/view.html

@@ -40,7 +40,16 @@
 {if $i> 0 and $c.changedIssue()}
 <div class="issue-changes">
 {foreach $c.changes as $w => $v}
-<strong>{if $w == 'su'}{trans 'Summary:'}{/if}{if $w == 'st'}{trans 'Status:'}{/if}{if $w == 'ow'}{trans 'Owner:'}{/if}{if $w == 'lb'}{trans 'Labels:'}{/if}</strong> {if $w == 'lb'}{assign $l = implode(', ', $v)}{$l}{else}{$v}{/if}<br />
+<strong>{if $w == 'su'}{trans 'Summary:'}{/if}{if $w == 'st'}{trans 'Status:'}{/if}{if $w == 'ow'}{trans 'Owner:'}{/if}{if $w == 'lb'}{trans 'Labels:'}{/if}{if $w == 'rel'}{trans 'Relations:'}{/if}</strong>
+{if $w == 'lb' or $w == 'rel'}
+  {foreach $v as $t => $ls}
+    {foreach $ls as $l}
+      {if $t == 'rem'}<s>{/if}{$l}{if $t == 'rem'}</s> {/if}
+    {/foreach}
+  {/foreach}
+{else}
+  {$v}
+{/if}<br />
 {/foreach}
 </div>
 {/if}
@@ -120,12 +129,20 @@
 </td>
 </tr>
 <tr>
-<th>{$form.f.relation_type.labelTag}:</th>
+<th>{$form.f.relation_type0.labelTag}:</th>
 <td>
-{if $form.f.relation_type.errors}{$form.f.relation_type.fieldErrors}{/if}
+{assign $prevField}
-{if $form.f.relation_issue.errors}{$form.f.relation_issue.fieldErrors}{/if}
+{foreach $form as $field}
-{$form.f.relation_type|unsafe}
+   {if strpos($field.name, 'relation_type') === 0}
-{$form.f.relation_issue|unsafe}
+        {$field|unsafe}
+        {assign $prevField = $field}
+   {/if}
+   {if strpos($field.name, 'relation_issue') === 0}
+        {$field|unsafe}<br />
+        {if $prevField.errors}{$prevField.fieldErrors}{/if}
+        {if $field.errors}{$field.fieldErrors}{/if}
+   {/if}
+{/foreach}
 </td>
 </tr>
 <tr>
@@ -172,15 +189,17 @@
 </p>{/if}
 {if count($related_issues) > 0}
 {foreach $related_issues as $verb => $rel_issues}
+<p>
 <strong>{blocktrans}This issue {$verb}{/blocktrans}</strong><br />
   {foreach $rel_issues as $rel_issue}
      <span class="label">
         <a href="{url 'IDF_Views_Issue::view', array($project.shortname, $rel_issue.other_issue)}"
-           title="{$rel_issue.other_summary}">
+           class="label" title="{$rel_issue.other_summary}">
            <strong>{$rel_issue.other_issue}</strong> - {$rel_issue.other_summary|shorten:30}
         </a>
      </span><br />
   {/foreach}
+</p>
 {/foreach}
 {/if}
 </div>