From be9efd7e2ad49e1a494e5210e136622b14fadc79 Mon Sep 17 00:00:00 2001
From: Ilya Shakhat <ishakhat@mirantis.com>
Date: Fri, 14 Feb 2014 20:12:32 +0400
Subject: [PATCH] Fixed pattern for parsing co-authors

The exception was raised if tag co-authors existed in message,
but had wrong format. Added test for case when co-author email
is invalid.

Also changed activity log to print co-authors instead of original
author

Change-Id: I3fdf9295415720414f2522f22053181b51228b32
---
 dashboard/templates/_macros/activity_log.html | 13 ++++-----
 stackalytics/processor/vcs.py                 | 27 +++++++++++--------
 tests/unit/test_vcs.py                        |  4 +++
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/dashboard/templates/_macros/activity_log.html b/dashboard/templates/_macros/activity_log.html
index f541adfc9..b9dac5ee7 100644
--- a/dashboard/templates/_macros/activity_log.html
+++ b/dashboard/templates/_macros/activity_log.html
@@ -69,17 +69,14 @@ show_record_type=True, show_user_gravatar=True, gravatar_size=32, show_all=True)
     <div style="margin-left: {{ gravatar_size * 1.4 }}px;">
 
     {% raw %}
-        <div class="header">{%html author_link %} ({%html company_link %})</div>
-        <div class="header">${date_str} in {%html module_link%}</div>
-
         {%if coauthor %}
-        <div class="header">Co-Authors:
-            {%each(index,value) coauthor %}
-                {%if index>0 %},{%/if%}
-                {%html value.author_link %} ({%html value.company_link %})
-            {%/each%}
+        <div class="header">
+            {%each(index,value) coauthor %}{%if index>0 %}, {%/if%}{%html value.author_link %} ({%html value.company_link %}){%/each%}
         </div>
+        {%else%}
+            <div class="header">{%html author_link %} ({%html company_link %})</div>
         {%/if%}
+        <div class="header">${date_str} in {%html module_link%}</div>
 
         {%if record_type == "commit" %}
             <div class="header">Commit &ldquo;${subject}&rdquo;</div>
diff --git a/stackalytics/processor/vcs.py b/stackalytics/processor/vcs.py
index c4ace596a..8c5328335 100644
--- a/stackalytics/processor/vcs.py
+++ b/stackalytics/processor/vcs.py
@@ -68,18 +68,19 @@ GIT_LOG_PATTERN = re.compile(''.join([(r[0] + ':(.*?)\n')
                              'diff_stat:' + DIFF_STAT_PATTERN,
                              re.DOTALL)
 
+CO_AUTHOR_PATTERN_RAW = '(?P<author_name>.+?)\s*<(?P<author_email>.+)>'
+CO_AUTHOR_PATTERN = re.compile(CO_AUTHOR_PATTERN_RAW, re.IGNORECASE)
+
 MESSAGE_PATTERNS = {
     'bug_id': re.compile(r'bug[\s#:]*(?P<id>\d+)', re.IGNORECASE),
     'blueprint_id': re.compile(r'\b(?:blueprint|bp)\b[ \t]*[#:]?[ \t]*'
                                r'(?P<id>[a-z0-9-]+)', re.IGNORECASE),
     'change_id': re.compile('Change-Id: (?P<id>I[0-9a-f]{40})', re.IGNORECASE),
     'coauthor': re.compile(r'(?:Co-Authored|Also)-By:'
-                           r'\s*(?P<id>.*)\s', re.IGNORECASE)
+                           r'\s*(?P<id>%s)\s' % CO_AUTHOR_PATTERN_RAW,
+                           re.IGNORECASE)
 }
 
-CO_AUTHOR_PATTERN = re.compile(
-    r'(?P<author_name>.+?)\s*<(?P<author_email>.+)>', re.IGNORECASE)
-
 
 class Git(Vcs):
 
@@ -228,14 +229,18 @@ class Git(Vcs):
                                           for bp_name
                                           in commit['blueprint_id']]
 
-            coauthors = []
-            for coauthor in commit.get('coauthor') or []:
-                m = re.match(CO_AUTHOR_PATTERN, coauthor)
-                if utils.check_email_validity(m.group("author_email")):
-                    coauthors.append(m.groupdict())
+            if 'coauthor' in commit:
+                verified_coauthors = []
+                for coauthor in commit['coauthor']:
+                    m = re.match(CO_AUTHOR_PATTERN, coauthor)
+                    if m and utils.check_email_validity(
+                            m.group("author_email")):
+                        verified_coauthors.append(m.groupdict())
 
-            if coauthors:
-                commit['coauthor'] = coauthors
+                if verified_coauthors:
+                    commit['coauthor'] = verified_coauthors
+                else:
+                    del commit['coauthor']  # no valid authors
 
             yield commit
 
diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py
index 73555fdcb..5aa31e706 100644
--- a/tests/unit/test_vcs.py
+++ b/tests/unit/test_vcs.py
@@ -84,6 +84,7 @@ author_name:John Doe
 author_email:john.doe@dreamhost.com
 subject:add readme for 2.2.2
 message: implements blueprint fix-me.
+Co-Authored-By: Anonymous <wrong@email>
 Change-Id: Id32a4a72ec1d13992b306c4a38e73605758e26c7
 
 diff_stat:
@@ -95,6 +96,7 @@ author_name:Doug Hoffner
 author_email:mark.mcclain@dreamhost.com
 subject:add readme for 2.2.2
 message:Change-Id: Id32a4a72ec1d13992b306c4a38e73605758e26c7
+Co-Authored-By: some friend of mine
 
 diff_stat:
 
@@ -109,6 +111,7 @@ subject:adds support off co-authors
 message:Change-Id: Id811c762ec1d13992b306c4a38e7360575e61451
 Co-Authored-By: Tupac Shakur <tupac.shakur@openstack.com>
 Also-By: Bob Dylan <bob.dylan@openstack.com>
+Also-By: Anonymous <wrong@email>
 
 diff_stat:
 
@@ -140,6 +143,7 @@ diff_stat:
         self.assertEqual(0, commits[3]['lines_deleted'])
         self.assertEqual(set(['dummy:fix-me']),
                          set(commits[3]['blueprint_id']))
+        self.assertFalse('coauthor' in commits[3])
 
         self.assertEqual(0, commits[4]['files_changed'])
         self.assertEqual(0, commits[4]['lines_added'])