From 6c08392c4c6cf9f73b85c868fd49826f38515966 Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Tue, 29 Oct 2013 13:26:26 +0400 Subject: [PATCH] Open reviews report is finished * Corrected calculation of review state (take into account latest patch) * Refactored reports, base report template is introduced * Review marks are converted to int Part of blueprint module-review-backlog-stats Change-Id: I995947d43f6bbb967d5212e95890f2d584e08571 --- dashboard/decorators.py | 4 +- dashboard/memory_storage.py | 2 +- dashboard/reports.py | 35 +++---- dashboard/static/css/style.css | 6 ++ dashboard/static/js/stackalytics-ui.js | 21 +++++ dashboard/templates/overview.html | 2 +- dashboard/templates/reports/base_report.html | 91 +++++++++++++++++++ .../templates/reports/blueprint_summary.html | 64 ++----------- dashboard/templates/reports/open_reviews.html | 84 +++-------------- dashboard/vault.py | 2 +- dashboard/web.py | 2 +- etc/test_default_data.json | 6 +- stackalytics/processor/record_processor.py | 17 +++- 13 files changed, 177 insertions(+), 159 deletions(-) create mode 100644 dashboard/templates/reports/base_report.html diff --git a/dashboard/decorators.py b/dashboard/decorators.py index b45bb92e1..a674407c9 100644 --- a/dashboard/decorators.py +++ b/dashboard/decorators.py @@ -140,9 +140,9 @@ def aggregate_filter(): positive = 0 mark_distribution = [] - for key in ['-2', '-1', '1', '2']: + for key in [-2, -1, 1, 2]: if key in record: - if key in ['1', '2']: + if key in [1, 2]: positive += record[key] mark_distribution.append(str(record[key])) else: diff --git a/dashboard/memory_storage.py b/dashboard/memory_storage.py index a201a2cd7..e01537452 100644 --- a/dashboard/memory_storage.py +++ b/dashboard/memory_storage.py @@ -130,7 +130,7 @@ class CachedMemoryStorage(MemoryStorage): def get_original_company_name(self, company_name): normalized = company_name.lower() if normalized not in self.company_name_mapping: - raise Exception('Unknown company name %s' % company_name) + return normalized return self.company_name_mapping[normalized] def get_companies(self): diff --git a/dashboard/reports.py b/dashboard/reports.py index 17e331f9d..6600be583 100644 --- a/dashboard/reports.py +++ b/dashboard/reports.py @@ -83,31 +83,24 @@ def open_reviews(module): memory_storage_inst = vault.get_memory_storage() time_now = int(time.time()) - review_marks = {} - reviews = {} + module_id_index = vault.get_vault()['module_id_index'] + module = module.lower() + if module in module_id_index: + modules = module_id_index[module]['modules'] + else: + modules = [module] - mark_ids = (memory_storage_inst.get_record_ids_by_modules([module]) & - memory_storage_inst.get_record_ids_by_type('mark')) - - for mark in memory_storage_inst.get_records(mark_ids): - review_id = mark['review_id'] - if review_id in review_marks: - if mark['date'] > review_marks[review_id]['date']: - review_marks[review_id] = mark - else: - review = memory_storage_inst.get_record_by_primary_key(review_id) - if not review: - continue # todo because we filter jenkins - review_marks[review_id] = mark - reviews[review_id] = review + review_ids = (memory_storage_inst.get_record_ids_by_modules(modules) & + memory_storage_inst.get_record_ids_by_type('review')) waiting_on_reviewer = [] total_open = 0 - for review_id, mark in review_marks.iteritems(): - if reviews[review_id]['open']: + + for review in memory_storage_inst.get_records(review_ids): + if review['status'] == 'NEW': total_open += 1 - if mark['value'] in ['1', '2']: - waiting_on_reviewer.append(reviews[review_id]) + if review['value'] in [1, 2]: + waiting_on_reviewer.append(review) return { 'module': module, @@ -115,7 +108,7 @@ def open_reviews(module): 'waiting_on_reviewer': len(waiting_on_reviewer), 'waiting_on_submitter': total_open - len(waiting_on_reviewer), 'latest_revision': _process_stat( - waiting_on_reviewer, 'lastUpdated', time_now), + waiting_on_reviewer, 'updated_on', time_now), 'first_revision': _process_stat(waiting_on_reviewer, 'date', time_now), } diff --git a/dashboard/static/css/style.css b/dashboard/static/css/style.css index ab341a0b2..2335acc30 100644 --- a/dashboard/static/css/style.css +++ b/dashboard/static/css/style.css @@ -159,6 +159,12 @@ div#right_list_wrapper { .message { white-space: pre-wrap; + margin-top: 0.8em; +} + +.label { + font-weight: bold; + line-height: 135%; } a[href^="https://blueprints"]:after { diff --git a/dashboard/static/js/stackalytics-ui.js b/dashboard/static/js/stackalytics-ui.js index 95a60658c..838b5dcea 100644 --- a/dashboard/static/js/stackalytics-ui.js +++ b/dashboard/static/js/stackalytics-ui.js @@ -181,6 +181,27 @@ function renderTableAndChart(url, container_id, table_id, chart_id, link_param, }); } +function render_bar_chart(chart_id, chart_data) { + $.jqplot(chart_id, chart_data, { + seriesDefaults: { + renderer: $.jqplot.BarRenderer, + rendererOptions: { + barMargin: 1 + }, + pointLabels: {show: true} + }, + axes: { + xaxis: { + renderer: $.jqplot.CategoryAxisRenderer, + label: "Age" + }, + yaxis: { + label: "Count" + } + } + }); +} + function getUrlVars() { var vars = {}; var parts = window.location.href.replace(/[?&]+([^=&]+)=([^&]*)/gi, function (m, key, value) { diff --git a/dashboard/templates/overview.html b/dashboard/templates/overview.html index 6b8d5d366..2cbcd2569 100644 --- a/dashboard/templates/overview.html +++ b/dashboard/templates/overview.html @@ -318,7 +318,7 @@ {% if show_module_profile %}

Module {{ module }}

-
Open reviews report
+
Open reviews report↗
{% endif %} diff --git a/dashboard/templates/reports/base_report.html b/dashboard/templates/reports/base_report.html new file mode 100644 index 000000000..8ccacfd4f --- /dev/null +++ b/dashboard/templates/reports/base_report.html @@ -0,0 +1,91 @@ + + + + {% block title %}{% endblock %} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +{% block head %}{% endblock %} + + + + +{% macro show_activity_log(activity) -%} + +

Activity Log

+ +{% if not activity %} +
No activity.
+{% endif %} +{% for item in activity %} +
+
+
+
+
{{ item.date_str}}
+
{{ item.author_name }} ({{ item.company_name }})
+ {% if item.record_type == "commit" %} +
Commit “{{ item.subject }}”
+
{{ item.message | safe }}
+
+{{ item.lines_added }} + - {{ item.lines_deleted }} +
+ {% if item.correction_comment %} +
Commit corrected: + {{ item.correction_comment }}
+ {% endif %} + {% elif item.record_type == "mark" %} +
Review “{{item.subject}}”
+
Patch submitted by {{ parent_author_link }}
+
Change Id: {{item.review_id}}
+
+ {{item.description}}: {{item.value}}
+ {% elif item.record_type == "email" %} +
Email “{{item.subject}}”
+ {% if item.body %} +
{{ item.body|safe }}
+ {% endif %} + {% endif %} +
+
+ +{% endfor %} + +{%- endmacro %} + +
+ + | community heartbeat +
+ +{% block body %}{% endblock %} + + + diff --git a/dashboard/templates/reports/blueprint_summary.html b/dashboard/templates/reports/blueprint_summary.html index 9df4c8bc7..7b558761a 100644 --- a/dashboard/templates/reports/blueprint_summary.html +++ b/dashboard/templates/reports/blueprint_summary.html @@ -1,21 +1,10 @@ - - - - {{ blueprint.title }} - - - - +{% extends "reports/base_report.html" %} +{% block title %} +Blueprint {{ blueprint.title }} detailed report +{% endblock %} + +{% block body %}

Blueprint “{{ blueprint.name }}”

Title: {{ blueprint.title }}
@@ -42,43 +31,6 @@
{{ blueprint.whiteboard }}
{% endif %} -

Activity Log

-{% if not activity %} -
No activities related to this blueprint.
-{% endif %} -{% for item in activity %} -
-
-
-
-
{{ item.date_str}}
-
{{ item.author_name }} ({{ item.company_name }})
- {% if item.record_type == "commit" %} -
Commit “{{ item.subject }}”
-
{{ item.message | safe }}
-
+{{ item.lines_added }} - - {{ item.lines_deleted }} -
- {% if item.correction_comment %} -
Commit corrected: - {{ item.correction_comment }}
- {% endif %} - {% elif item.record_type == "mark" %} -
Review “{{item.subject}}”
-
Patch submitted by {{ parent_author_link }}
-
Change Id: {{item.review_id}}
-
- {{item.description}}: {{item.value}}
- {% elif item.record_type == "email" %} -
Email “{{item.subject}}”
- {% if item.body %} -
{{ item.body|safe }}
- {% endif %} - {% endif %} -
-
- -{% endfor %} - - \ No newline at end of file +{{ show_activity_log(activity) }} +{% endblock %} diff --git a/dashboard/templates/reports/open_reviews.html b/dashboard/templates/reports/open_reviews.html index 1cb8c6eca..779f44fdf 100644 --- a/dashboard/templates/reports/open_reviews.html +++ b/dashboard/templates/reports/open_reviews.html @@ -1,87 +1,26 @@ - - - - Open reviews report for {{ module }} - +{% extends "reports/base_report.html" %} - - - - - - - - - - - - - - - - - - - - - - - - - - - +{% block title %} +Open reviews report for {{ module }} +{% endblock %} +{% block head %} +{% endblock %} - - - - +{% block body %}

Open reviews for {{ module }}

-

Summary:

+

Summary

{% if total_open %} @@ -96,7 +35,7 @@
    {% for item in latest_revision.reviews[:5] %} -
  1. {{ item.lastUpdated_age }} {{ item.url }} {{ item.subject }} by {{ item.author_name }} ({{ item.company_name }})
    2. +
  2. {{ item.updated_on_age }} {{ item.url }} {{ item.subject }} by {{ item.author_name }} ({{ item.company_name }})
    3. {% endfor %}
@@ -115,4 +54,5 @@ {% endfor %} -{% endif %} \ No newline at end of file +{% endif %} +{% endblock %} diff --git a/dashboard/vault.py b/dashboard/vault.py index d19053402..afa07b03f 100644 --- a/dashboard/vault.py +++ b/dashboard/vault.py @@ -127,7 +127,7 @@ def init_module_groups(vault): module_group_name = module_group['module_group_name'] module_group_id = module_group_name.lower() - module_id_index[module_group_name] = { + module_id_index[module_group_id] = { 'group': True, 'id': module_group_id, 'text': module_group_name, diff --git a/dashboard/web.py b/dashboard/web.py index 3c5e7acdb..8d8015480 100644 --- a/dashboard/web.py +++ b/dashboard/web.py @@ -170,7 +170,7 @@ def get_contribution_json(records): commit_count += 1 loc += record['loc'] elif record['record_type'] == 'mark': - marks[int(record['value'])] += 1 + marks[record['value']] += 1 elif record['record_type'] == 'email': email_count += 1 elif record['record_type'] == 'bpd': diff --git a/etc/test_default_data.json b/etc/test_default_data.json index 9c5a15089..e6156f615 100644 --- a/etc/test_default_data.json +++ b/etc/test_default_data.json @@ -16,7 +16,7 @@ ] }, { - "launchpad_id": "openstack", + "launchpad_id": "jenkins", "companies": [ { "company_name": "*robots", @@ -48,8 +48,8 @@ "domains": ["intel.com"] }, { - "domains": ["openstack.org"], - "company_name": "OpenStack Foundation" + "domains": ["robots"], + "company_name": "*robots" } ], diff --git a/stackalytics/processor/record_processor.py b/stackalytics/processor/record_processor.py index 6cafbebae..738448889 100644 --- a/stackalytics/processor/record_processor.py +++ b/stackalytics/processor/record_processor.py @@ -206,6 +206,20 @@ class RecordProcessor(object): review['author_email'] = owner['email'].lower() review['date'] = record['createdOn'] + patch_sets = record.get('patchSets', []) + review['updated_on'] = review['date'] + if patch_sets: + patch = patch_sets[-1] + if 'approvals' in patch: + review['value'] = min([int(p['value']) + for p in patch['approvals']]) + review['updated_on'] = patch['approvals'][0]['grantedOn'] + else: + review['updated_on'] = patch['createdOn'] + + if 'value' not in review: + review['value'] = 0 + self._update_record_and_user(review) yield review @@ -220,13 +234,14 @@ class RecordProcessor(object): for approval in patch['approvals']: # copy everything and flatten user data mark = dict([(k, v) for k, v in approval.iteritems() - if k not in ['by', 'grantedOn']]) + if k not in ['by', 'grantedOn', 'value']]) reviewer = approval['by'] if 'email' not in reviewer or 'username' not in reviewer: continue # ignore mark['record_type'] = 'mark' + mark['value'] = int(approval['value']) mark['date'] = approval['grantedOn'] mark['primary_key'] = (record['id'] + str(mark['date']) +