Fixed handler of engineer contribution stats

Split stats retrieving on /stats/engineer (used by charts and tables)
and /stats/engineers_extended (used in contribution report)
Added tests for these handlers.

Change-Id: Ia839ffeac52984c2d63690ef5bc473450c0d46ee

dashboard/decorators.py

@@ -145,17 +145,14 @@ def record_filter(ignore=None, use_default=True):
     return decorator
 
 
-def aggregate_filter():
-    def decorator(f):
-        @functools.wraps(f)
-        def aggregate_filter_decorated_function(*args, **kwargs):
-
 def incremental_filter(result, record, param_id):
     result[record[param_id]]['metric'] += 1
 
+
 def loc_filter(result, record, param_id):
     result[record[param_id]]['metric'] += record['loc']
 
+
 def mark_filter(result, record, param_id):
     result_by_param = result[record[param_id]]
     if record['type'] == 'APRV':
@@ -169,6 +166,7 @@ def aggregate_filter():
         result_by_param['disagreements'] = (
             result_by_param.get('disagreements', 0) + 1)
 
+
 def mark_finalize(record):
     new_record = record.copy()
 
@@ -188,17 +186,22 @@ def aggregate_filter():
         positive_ratio = '%.1f%%' % (
             (positive * 100.0) / record['metric'])
         new_record['disagreement_ratio'] = '%.1f%%' % (
-                        (record.get('disagreements', 0) * 100.0) /
+            (record.get('disagreements', 0) * 100.0) / record['metric'])
-                        record['metric'])
     else:
         positive_ratio = helpers.INFINITY_HTML
         new_record['disagreement_ratio'] = helpers.INFINITY_HTML
-                new_record['mark_ratio'] = ('|'.join(mark_distribution) +
+    new_record['mark_ratio'] = (
-                                            ' (' + positive_ratio + ')')
+        '|'.join(mark_distribution) + ' (' + positive_ratio + ')')
     new_record['positive_ratio'] = positive_ratio
 
     return new_record
 
+
+def aggregate_filter():
+    def decorator(f):
+        @functools.wraps(f)
+        def aggregate_filter_decorated_function(*args, **kwargs):
+
             metric_param = (flask.request.args.get('metric') or
                             parameters.get_default('metric'))
             metric = metric_param.lower()

dashboard/templates/reports/contribution.html

@@ -7,12 +7,12 @@ Contribution into {{ module }} for the last {{ days }} days
 {% block scripts %}
     <script type="text/javascript">
         $(document).ready(function () {
-            var table_column_names = ["index", "link", "metric", "-2", "-1", "1", "2", "A", "positive_ratio", "disagreements", "disagreement_ratio",
+            var table_column_names = ["index", "link", "mark", "-2", "-1", "1", "2", "A", "positive_ratio", "disagreements", "disagreement_ratio",
                                       "review_ratio", "commit", "email"];
             var table_id = "review_stats_table";
 
             $.ajax({
-                url: make_uri("/api/1.0/stats/engineers?metric=marks&project_type=all&module={{ module }}&release=all&start_date={{ start_date }}"),
+                url: make_uri("/api/1.0/stats/engineers_extended?project_type=all&module={{ module }}&release=all&start_date={{ start_date }}"),
                 dataType: "json",
                 success: function (data) {
                     var tableData = data["stats"];
@@ -21,7 +21,7 @@ Contribution into {{ module }} for the last {{ days }} days
                     var sort_by_column = 0;
                     for (var i = 0; i < table_column_names.length; i++) {
                         tableColumns.push({"mData": table_column_names[i]});
-                        if (table_column_names[i] == "metric") {
+                        if (table_column_names[i] == "mark") {
                             sort_by_column = i;
                         }
                     }
@@ -49,16 +49,16 @@ Contribution into {{ module }} for the last {{ days }} days
 
                         if (tableData[i].core == "master") {
                             tableData[i].link += "&nbsp;&#x273B;";
-                            summary.core_marks += tableData[i].metric;
+                            summary.core_marks += tableData[i].mark;
                             summary.core_reviewers ++;
                         } else if (tableData[i].core) {
                             tableData[i].link += "&nbsp;&#x272C; <small><i>" + tableData[i].core + "</i></small>";
                         }
-                        if (tableData[i].metric > 0) {
+                        if (tableData[i].mark > 0) {
                             summary.reviewers ++;
                         }
                         tableData[i].review_ratio = tableData[i].review + " / " + tableData[i].patch_count;
-                        summary.marks += tableData[i].metric;
+                        summary.marks += tableData[i].mark;
                         summary.commits += tableData[i].commit;
                         summary.reviews += tableData[i].review;
                         summary.patch_count += tableData[i].patch_count;

dashboard/web.py

@@ -85,10 +85,7 @@ def _get_aggregated_stats(records, metric_filter, keys, param_id,
         metric_filter(result, record, param_id)
         result[record[param_id]]['name'] = record[param_title]
 
-    if not finalize_handler:
     response = [r for r in result.values() if r['metric']]
-    else:
-        response = result.values()
     response.sort(key=lambda x: x['metric'], reverse=True)
     response = [item for item in map(finalize_handler, response) if item]
     utils.add_index(response, item_filter=lambda x: x['id'] != '*independent')
@@ -117,14 +114,23 @@ def get_modules(records, metric_filter, finalize_handler):
                                  'module')
 
 
+def is_engineer_core_in_modules(user, modules):
+    is_core = False
+    for (module, branch) in user['core']:
+        if module in modules:
+            is_core = branch
+            if branch == 'master':  # we need master, but stables are ok
+                break
+    return is_core
+
+
 @app.route('/api/1.0/stats/engineers')
 @decorators.jsonify('stats')
 @decorators.exception_handler()
-@decorators.record_filter(ignore='metric')
+@decorators.record_filter()
 @decorators.aggregate_filter()
 def get_engineers(records, metric_filter, finalize_handler):
 
-    exclude = flask.request.args.get('exclude')
     modules_names = parameters.get_parameter({}, 'module', 'modules')
     modules = vault.resolve_modules(modules_names)
 
@@ -132,52 +138,63 @@ def get_engineers(records, metric_filter, finalize_handler):
         if finalize_handler:
             record = finalize_handler(record)
         user = vault.get_user_from_runtime_storage(record['id'])
-        record['company'] = user['companies'][-1]['company_name']
+        record['core'] = is_engineer_core_in_modules(user, modules)
-        record['review'] = record.get('review', 0)
-        record['commit'] = record.get('commit', 0)
-        record['email'] = record.get('email', 0)
-        record['patch_count'] = record.get('patch_count', 0)
-
-        if not (record['metric'] or record['review'] or record['commit'] or
-                record['email'] or record['patch_count']):
-            return
-
-        is_core = False
-        for (module, branch) in user['core']:
-            if module in modules:
-                is_core = branch
-                if branch == 'master':  # we need master, but stables are ok
-                    break
-
-        if exclude:
-            if ((exclude == 'non-core' and is_core) or
-                    (exclude == 'core' and not is_core)):
-                return record
-        else:
-            record['core'] = is_core
         return record
 
-    def record_processing(result, record, param_id):
+    return _get_aggregated_stats(records, metric_filter,
-        record_type = record['record_type']
-
-        result_row = result[record[param_id]]
-        if record_type == 'mark':
-            metric_filter(result, record, param_id)
-        elif record_type == 'commit':
-            result_row['commit'] = result_row.get('commit', 0) + 1
-        elif record_type == 'email':
-            result_row['email'] = result_row.get('email', 0) + 1
-        elif record_type == 'review':
-            result_row['review'] = result_row.get('review', 0) + 1
-            result_row['patch_count'] = (result_row.get('patch_count', 0) +
-                                         record['patch_count'])
-
-    return _get_aggregated_stats(records, record_processing,
                                  vault.get_memory_storage().get_user_ids(),
                                  'user_id', 'author_name',
                                  finalize_handler=postprocessing)
 
 
+@app.route('/api/1.0/stats/engineers_extended')
+@decorators.jsonify('stats')
+@decorators.exception_handler()
+@decorators.record_filter(ignore='metric')
+def get_engineers_extended(records):
+    modules_names = parameters.get_parameter({}, 'module', 'modules')
+    modules = vault.resolve_modules(modules_names)
+
+    def postprocessing(record):
+        record = decorators.mark_finalize(record)
+
+        if not (record['mark'] or record['review'] or record['commit'] or
+                record['email'] or record['patch_count']):
+            return
+
+        user = vault.get_user_from_runtime_storage(record['id'])
+        record['company'] = user['companies'][-1]['company_name']
+        record['core'] = is_engineer_core_in_modules(user, modules)
+        return record
+
+    def record_processing(result, record, param_id):
+        result_row = result[record[param_id]]
+        record_type = record['record_type']
+        result_row[record_type] = result_row.get(record_type, 0) + 1
+        if record_type == 'mark':
+            decorators.mark_filter(result, record, param_id)
+            result_row['metric'] = result_row['mark']
+        if record_type == 'review':
+            result_row['patch_count'] = (result_row.get('patch_count', 0) +
+                                         record['patch_count'])
+
+    result = dict((user_id, {'id': user_id, 'mark': 0, 'review': 0,
+                             'commit': 0, 'email': 0, 'patch_count': 0,
+                             'metric': 0})
+                  for user_id in vault.get_memory_storage().get_user_ids())
+
+    for record in records:
+        record_processing(result, record, 'user_id')
+        result[record['user_id']]['name'] = record['author_name']
+
+    response = result.values()
+    response.sort(key=lambda x: x['mark'], reverse=True)
+    response = [item for item in map(postprocessing, response) if item]
+    utils.add_index(response)
+
+    return response
+
+
 @app.route('/api/1.0/stats/distinct_engineers')
 @decorators.jsonify('stats')
 @decorators.exception_handler()

tests/api/test_stats.py

@@ -41,3 +41,82 @@ class TestAPIStats(test_api.TestAPI):
             self.assertEqual('glance', stats[0]['id'])
             self.assertEqual(60, stats[1]['metric'])
             self.assertEqual('nova', stats[1]['id'])
+
+    def test_get_engineers(self):
+        with test_api.make_runtime_storage(
+                {'repos': [{'module': 'nova', 'project_type': 'openstack',
+                            'organization': 'openstack',
+                            'uri': 'git://github.com/openstack/nova.git'},
+                           {'module': 'glance', 'project_type': 'openstack',
+                            'organization': 'openstack',
+                            'uri': 'git://github.com/openstack/glance.git'}],
+                 'user:john_doe': {
+                     'seq': 1, 'user_id': 'john_doe', 'user_name': 'John Doe',
+                     'companies': [{'company_name': 'NEC', 'end_date': 0}],
+                     'emails': ['john_doe@gmail.com'], 'core': []},
+                 'user:bill': {
+                     'seq': 1, 'user_id': 'bill', 'user_name': 'Bill Smith',
+                     'companies': [{'company_name': 'IBM', 'end_date': 0}],
+                     'emails': ['bill_smith@gmail.com'], 'core': []}},
+                test_api.make_records(record_type=['commit'],
+                                      loc=[10, 20, 30],
+                                      module=['nova'],
+                                      user_id=['john_doe']),
+                test_api.make_records(record_type=['commit'],
+                                      loc=[100, 200, 300],
+                                      module=['glance'],
+                                      user_id=['john_doe']),
+                test_api.make_records(record_type=['review'],
+                                      primary_key=['0123456789'],
+                                      module=['glance']),
+                test_api.make_records(record_type=['mark'],
+                                      review_id=['0123456789'],
+                                      module=['glance'],
+                                      user_id=['john_doe', 'bill'])):
+            response = self.app.get('/api/1.0/stats/engineers?metric=loc')
+            stats = json.loads(response.data)['stats']
+            self.assertEqual(1, len(stats))
+            self.assertEqual(660, stats[0]['metric'])
+
+    def test_get_engineers_extended(self):
+        with test_api.make_runtime_storage(
+                {'repos': [{'module': 'nova', 'project_type': 'openstack',
+                            'organization': 'openstack',
+                            'uri': 'git://github.com/openstack/nova.git'},
+                           {'module': 'glance', 'project_type': 'openstack',
+                            'organization': 'openstack',
+                            'uri': 'git://github.com/openstack/glance.git'}],
+                 'user:john_doe': {
+                     'seq': 1, 'user_id': 'john_doe', 'user_name': 'John Doe',
+                     'companies': [{'company_name': 'NEC', 'end_date': 0}],
+                     'emails': ['john_doe@gmail.com'], 'core': []},
+                 'user:smith': {
+                     'seq': 1, 'user_id': 'smith', 'user_name': 'Bill Smith',
+                     'companies': [{'company_name': 'IBM', 'end_date': 0}],
+                     'emails': ['bill_smith@gmail.com'], 'core': []}},
+                test_api.make_records(record_type=['commit'],
+                                      loc=[10, 20, 30],
+                                      module=['nova'],
+                                      user_id=['john_doe']),
+                test_api.make_records(record_type=['review'],
+                                      primary_key=['0123456789', '9876543210'],
+                                      module=['glance']),
+                test_api.make_records(record_type=['mark'],
+                                      review_id=['0123456789', '9876543210'],
+                                      module=['glance'],
+                                      value=[1],
+                                      type=['CRVW'],
+                                      author_name=['John Doe'],
+                                      user_id=['john_doe']),
+                test_api.make_records(record_type=['mark'],
+                                      review_id=['0123456789'],
+                                      module=['glance'],
+                                      author_name=['Bill Smith'],
+                                      user_id=['smith'])):
+            response = self.app.get('/api/1.0/stats/engineers_extended')
+            stats = json.loads(response.data)['stats']
+            self.assertEqual(2, len(stats))
+            self.assertEqual(2, stats[0]['mark'])
+            self.assertEqual('john_doe', stats[0]['id'])
+            self.assertEqual(3, stats[0]['commit'])
+            self.assertEqual(2, stats[0]['1'])