A view into how many addons were affected by each validation error (bug 662706)

apps/zadmin/models.py

@@ -5,6 +5,8 @@ from django.conf import settings
 from django.db import models
 from django.utils.functional import memoize
 
+import redisutils
+
 import amo
 import amo.models
 from amo.urlresolvers import reverse
@@ -205,3 +207,81 @@ class EmailPreview(amo.models.ModelBase):
 
     class Meta:
         db_table = 'email_preview'
+
+
+class ValidationJobTally(object):
+    """Redis key/vals for a tally of validation job results.
+
+    The key/val pairs look like this::
+
+        # message keys that were found by this validation job:
+        validation.job_id:1234:msg_keys = set([
+            'path.to.javascript_data_urls',
+            'path.to.navigator_language'
+            ])
+
+        # translation of message keys to actual messages:
+        validation.msg_key:<msg_key>:message =
+            'javascript:/data: URIs may be incompatible with Firefox 6'
+        validation.msg_key:<msg_key>:long_message =
+            'A more detailed message....'
+
+        # type of message (error, warning, or notice)
+        validation.msg_key:<msg_key>:type = 'error'
+
+        # count of addons affected per message key, per job
+        validation.job_id:1234.msg_key:<msg_key>:addons_affected = 120
+
+    """
+
+    def __init__(self, job_id):
+        self.job_id = job_id
+        self.kv = redisutils.connections['master']
+
+    def get_messages(self):
+        for msg_key in self.kv.smembers('validation.job_id:%s' % self.job_id):
+            yield ValidationMsgTally(self.job_id, msg_key)
+
+    def save_message(self, msg):
+        msg_key = '.'.join(msg['id'])
+        self.kv.sadd('validation.job_id:%s' % self.job_id,
+                     msg_key)
+        self.kv.set('validation.msg_key:%s:message' % msg_key,
+                    msg['message'])
+        if type(msg['description']) == list:
+            des = '; '.join(msg['description'])
+        else:
+            des = msg['description']
+        self.kv.set('validation.msg_key:%s:long_message' % msg_key,
+                    des)
+        if msg.get('compatibility_type'):
+            effective_type = msg['compatibility_type']
+        else:
+            effective_type = msg['type']
+        self.kv.set('validation.msg_key:%s:type' % msg_key,
+                    effective_type)
+        self.kv.incr('validation.job_id:%s.msg_key:%s:addons_affected'
+                     % (self.job_id, msg_key))
+
+
+class ValidationMsgTally(object):
+    """Redis key/vals for a tally of validation messages.
+    """
+
+    def __init__(self, job_id, msg_key):
+        self.job_id = job_id
+        self.msg_key = msg_key
+        self.kv = redisutils.connections['master']
+
+    def __getattr__(self, key):
+        if key in ('message', 'long_message', 'type'):
+            val = self.kv.get('validation.msg_key:%s:%s'
+                              % (self.msg_key, key))
+        elif key in ('addons_affected',):
+            val = self.kv.get('validation.job_id:%s.msg_key:%s:%s'
+                              % (self.job_id, self.msg_key, key))
+        elif key in ('key',):
+            val = self.msg_key
+        else:
+            raise ValueError('Unknown field: %s' % key)
+        return val or ''

apps/zadmin/tasks.py

@@ -1,4 +1,5 @@
 from datetime import datetime
+import json
 import logging
 import os
 import sys
@@ -22,7 +23,7 @@ from amo.utils import send_mail
 from devhub.tasks import run_validator
 from users.utils import get_task_user
 from versions.models import Version
-from zadmin.models import ValidationResult, ValidationJob
+from zadmin.models import ValidationResult, ValidationJob, ValidationJobTally
 
 log = logging.getLogger('z.task')
 
@@ -80,6 +81,7 @@ def bulk_validate_file(result_id, **kw):
         res.apply_validation(validation)
         log.info('[1@None] File %s (%s) errors=%s'
                  % (res.file, file_base, res.errors))
+        tally_validation_results.delay(res.validation_job.id, validation)
     res.save()
     tally_job_results(res.validation_job.id)
 
@@ -88,6 +90,19 @@ def bulk_validate_file(result_id, **kw):
         raise etype, val, tb
 
 
+@task
+def tally_validation_results(job_id, validation_str, **kw):
+    """Saves a tally of how many addons received each validation message.
+    """
+    validation = json.loads(validation_str)
+    log.info('[@%s] tally_validation_results (job %s, %s messages)'
+             % (tally_validation_results.rate_limit, job_id,
+                len(validation['messages'])))
+    v = ValidationJobTally(job_id)
+    for msg in validation['messages']:
+        v.save_message(msg)
+
+
 @task
 @write
 def add_validation_jobs(pks, job_pk, **kw):

apps/zadmin/templates/zadmin/validation.html

@@ -94,7 +94,13 @@
           {% endif %}
       </td>
       <td class="exceptions">{{ job.stats['errors'] }}</td>
-      <td></td>
+      <td>
+        {% if job.completed %}
+          [<a href="{{ url('zadmin.validation_tally_csv', job.pk) }}">
+              Download Errors/Warnings
+          </a>]
+        {% endif %}
+      </td>
     </tr>
     {% endfor %}
   </table>

apps/zadmin/tests/test_views.py

@@ -32,7 +32,7 @@ from zadmin.views import completed_versions_dirty, find_files
 from zadmin import tasks
 
 
-no_op_validation = dict(errors=0, warnings=0, notices=0,
+no_op_validation = dict(errors=0, warnings=0, notices=0, messages=[],
                         compatibility_summary=dict(errors=0, warnings=0,
                                                    notices=0))
 
@@ -146,6 +146,16 @@ class BulkValidationTest(test_utils.TestCase):
         kw.update(kwargs)
         return ValidationResult.objects.create(**kw)
 
+    def start_validation(self, new_max='3.7a3'):
+        self.new_max = self.appversion(new_max)
+        r = self.client.post(reverse('zadmin.start_validation'),
+                             {'application': amo.FIREFOX.id,
+                              'curr_max_version': self.curr_max.id,
+                              'target_version': self.new_max.id,
+                              'finish_email': 'fliggy@mozilla.com'},
+                             follow=True)
+        eq_(r.status_code, 200)
+
 
 class TestBulkValidation(BulkValidationTest):
 
@@ -594,16 +604,6 @@ class TestBulkNotify(BulkValidationTest):
 
 class TestBulkValidationTask(BulkValidationTest):
 
-    def start_validation(self, new_max='3.7a3'):
-        self.new_max = self.appversion(new_max)
-        r = self.client.post(reverse('zadmin.start_validation'),
-                             {'application': amo.FIREFOX.id,
-                              'curr_max_version': self.curr_max.id,
-                              'target_version': self.new_max.id,
-                              'finish_email': 'fliggy@mozilla.com'},
-                             follow=True)
-        eq_(r.status_code, 200)
-
     @attr('validator')
     def test_validate(self):
         self.start_validation()
@@ -689,6 +689,7 @@ class TestBulkValidationTask(BulkValidationTest):
                 "tier": 3,
                 "message": "Global called in dangerous manner",
                 "uid": "de93a48831454e0b9d965642f6d6bf8f",
+                "id": [],
                 "compatibility_type": None,
                 "for_appversions": None,
                 "type": "warning",
@@ -699,6 +700,7 @@ class TestBulkValidationTask(BulkValidationTest):
                 "tier": 5,
                 "message": "navigator.language may not behave as expected",
                 "uid": "f44c1930887c4d9e8bd2403d4fe0253a",
+                "id": [],
                 "compatibility_type": "error",
                 "for_appversions": {
                     "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}": ["4.2a1pre",
@@ -916,6 +918,73 @@ class TestBulkValidationTask(BulkValidationTest):
         eq_(len(ids), 0)
 
 
+class TestTallyValidationErrors(BulkValidationTest):
+
+    def setUp(self):
+        super(TestTallyValidationErrors, self).setUp()
+        self.data = {
+            "errors": 1,
+            "warnings": 1,
+            "notices": 0,
+            "messages": [
+            {
+                "message": "message one",
+                "description": ["message one long"],
+                "id": ["path", "to", "test_one"],
+                "uid": "de93a48831454e0b9d965642f6d6bf8f",
+                "type": "error",
+            },
+            {
+                "message": "message two",
+                "description": "message two long",
+                "id": ["path", "to", "test_two"],
+                "uid": "f44c1930887c4d9e8bd2403d4fe0253a",
+                "compatibility_type": "error",
+                "type": "warning"
+            }],
+            "metadata": {},
+            "compatibility_summary": {
+                "errors": 1,
+                "warnings": 1,
+                "notices": 0
+            }
+        }
+
+    def csv(self, job_id):
+        r = self.client.get(reverse('zadmin.validation_tally_csv',
+                            args=[job_id]))
+        eq_(r.status_code, 200)
+        rdr = csv.reader(StringIO(r.content))
+        header = rdr.next()
+        rows = sorted((r for r in rdr), key=lambda r: r[0])
+        return header, rows
+
+    @mock.patch('zadmin.tasks.run_validator')
+    def test_csv(self, run_validator):
+        run_validator.return_value = json.dumps(self.data)
+        self.start_validation()
+        res = ValidationResult.objects.get()
+        eq_(res.task_error, None)
+        header, rows = self.csv(res.validation_job.pk)
+        eq_(header, ['message_id', 'message', 'long_message',
+                     'type', 'addons_affected'])
+        eq_(rows.pop(0), ['path.to.test_one',
+                          'message one', 'message one long', 'error', '1'])
+        eq_(rows.pop(0), ['path.to.test_two',
+                          'message two', 'message two long', 'error', '1'])
+
+    def test_count_per_addon(self):
+        job = self.create_job()
+        data_str = json.dumps(self.data)
+        for i in range(3):
+            tasks.tally_validation_results(job.pk, data_str)
+        header, rows = self.csv(job.pk)
+        eq_(rows.pop(0), ['path.to.test_one',
+                          'message one', 'message one long', 'error', '3'])
+        eq_(rows.pop(0), ['path.to.test_two',
+                          'message two', 'message two long', 'error', '3'])
+
+
 def test_settings():
     # Are you there, settings page?
     response = test.Client().get(reverse('zadmin.settings'), follow=True)

apps/zadmin/urls.py

@@ -28,6 +28,8 @@ urlpatterns = patterns('',
         name='zadmin.notify.failure'),
     url(r'^validation/notify/syntax.json$', views.notify_syntax,
         name='zadmin.notify.syntax'),
+    url(r'^validation/(?P<job_id>\d+)/tally\.csv$',
+        views.validation_tally_csv, name='zadmin.validation_tally_csv'),
     url(r'^validation$', views.validation, name='zadmin.validation'),
     url(r'^email_preview/(?P<topic>.*)\.csv$',
         views.email_preview_csv, name='zadmin.email_preview_csv'),

apps/zadmin/views.py

@@ -9,6 +9,7 @@ from django import http
 from django.conf import settings as site_settings
 from django.contrib import admin
 from django.shortcuts import redirect, get_object_or_404
+from django.utils.encoding import smart_str
 from django.views import debug
 
 import commonware.log
@@ -39,7 +40,7 @@ from versions.models import Version
 
 from . import tasks
 from .forms import BulkValidationForm, NotifyForm, FeaturedCollectionFormSet
-from .models import ValidationJob, EmailPreviewTopic
+from .models import ValidationJob, EmailPreviewTopic, ValidationJobTally
 
 log = commonware.log.getLogger('z.zadmin')
 
@@ -293,6 +294,26 @@ def email_preview_csv(request, topic):
     return resp
 
 
+@admin.site.admin_view
+def validation_tally_csv(request, job_id):
+    resp = http.HttpResponse()
+    resp['Content-Type'] = 'text/csv; charset=utf-8'
+    resp['Content-Disposition'] = ('attachment; '
+                                   'filename=validation_tally_%s.csv'
+                                   % job_id)
+    writer = csv.writer(resp)
+    fields = ['message_id', 'message', 'long_message',
+              'type', 'addons_affected']
+    writer.writerow(fields)
+    job = ValidationJobTally(job_id)
+    for msg in job.get_messages():
+        row = [msg.key, msg.message, msg.long_message, msg.type,
+               msg.addons_affected]
+        writer.writerow([smart_str(r, encoding='utf8', strings_only=True)
+                         for r in row])
+    return resp
+
+
 @admin.site.admin_view
 def jetpack(request):
     upgrader = files.utils.JetpackUpgrader()