tweaks to ACR compatibility tool (bug 692974)

Author: cvan

apps/addons/admin.py

@@ -1,3 +1,4 @@
+from django import forms
 from django.contrib import admin
 
 from . import models
@@ -64,9 +65,18 @@ class CompatOverrideRangeInline(admin.TabularInline):
     exclude = ('type',)
 
 
+class CompatOverrideAdminForm(forms.ModelForm):
+
+    def clean(self):
+        if '_confirm' in self.data:
+            raise forms.ValidationError('Click "Save" to confirm changes.')
+        return self.cleaned_data
+
+
 class CompatOverrideAdmin(admin.ModelAdmin):
     raw_id_fields = ('addon',)
     inlines = [CompatOverrideRangeInline]
+    form = CompatOverrideAdminForm
 
 
 admin.site.register(models.BlacklistedGuid)

apps/compat/forms.py

@@ -16,3 +16,9 @@ class CompatForm(forms.Form):
                                       ('binary', _('Binary')),
                                       ('non-binary', _('Non-binary'))),
                              widget=RadioSelect, required=False)
+
+
+class AdminCompatForm(CompatForm):
+    _choices = [('%.1f' % (x / 10.0), '%.0f%%' % (x * 10))
+                for x in xrange(9, -1, -1)]
+    ratio = forms.ChoiceField(choices=_choices, required=False)

apps/zadmin/templates/zadmin/compat.html

@@ -8,6 +8,7 @@
 <style>
 #compat-form, #compat-form ul, #compat-form li { display: inline }
 #compat-form li { display: inline-block; margin-left: 1em }
+#compat-form label + label { margin-left: 1em }
 .bad { color: #800 }
 em.version { color: #888 }
 td, th { border-bottom-color: #ccc }
@@ -27,7 +28,7 @@
 <h2>{{ title }}</h2>
 <form class="go" action="" id="compat-form">
   <label>App: {{ form.appver }}</label>
-  {{ form.version }}
+  <label>Show reports over {{ form.ratio }} incompatibility</label>
   {{ form.type }}
   <noscript><button type="submit">Go</button></noscript>
 </form>
@@ -96,6 +97,8 @@ <h2>{{ title }}</h2>
               {{ csrf() }}
               <input type="hidden" name="_compat_ranges-TOTAL_FORMS" value="{{ initial + 1 }}">
               <input type="hidden" name="_compat_ranges-INITIAL_FORMS" value="{{ initial }}">
+              <input type="hidden" name="_continue" value="1">
+              <input type="hidden" name="_confirm" value="1">
               {% if not addon.self_hosted %}
                 <input type="hidden" name="addon" value="{{ addon.id }}">
               {% endif %}
@@ -116,8 +119,7 @@ <h2>{{ title }}</h2>
               <input type="hidden" name="_compat_ranges-{{ initial }}-max_version" value="{{ addon.current_version }}">
               <input type="hidden" name="_compat_ranges-{{ initial }}-app" value="{{ app.id }}">
               <input type="hidden" name="_compat_ranges-{{ initial }}-min_app_version" value="{{ version }}a1">
-              <input type="hidden" name="_compat_ranges-{{ initial }}-max_app_version" value="*">
-              <input type="hidden" name="_continue" value="1">
+              <input type="hidden" name="_compat_ranges-{{ initial }}-max_app_version" value="{{ version }}*">
               <button>Override Compat</button>
             </form>
           {% endif %}

apps/zadmin/tests/test_views.py

@@ -19,7 +19,7 @@
 from amo.tests import (formset, initial, close_to_now, assert_required,
                        assert_no_validation_errors)
 from amo.urlresolvers import reverse
-from addons.models import Addon
+from addons.models import Addon, CompatOverride, CompatOverrideRange
 from applications.models import AppVersion
 from bandwagon.models import FeaturedCollection, MonthlyPick
 from compat.cron import compatibility_report
@@ -1495,6 +1495,11 @@ def generate_reports(self, addon, good, bad, app, app_version):
         for x in xrange(bad):
             CompatReport.objects.create(works_properly=False, **defaults)
 
+    def get_pq(self, **kw):
+        r = self.client.get(self.url, kw)
+        eq_(r.status_code, 200)
+        return pq(r.content)('#compat-results')
+
     def test_defaults(self):
         r = self.client.get(self.url)
         eq_(r.status_code, 200)
@@ -1532,6 +1537,8 @@ def check_row(self, tr, addon, good, bad, percentage, app, app_version):
         eq_(form.attr('action'), reverse('admin:addons_compatoverride_add'))
         self.check_field(form, '_compat_ranges-TOTAL_FORMS', '1')
         self.check_field(form, '_compat_ranges-INITIAL_FORMS', '0')
+        self.check_field(form, '_continue', '1')
+        self.check_field(form, '_confirm', '1')
         self.check_field(form, 'addon', str(addon.id))
         self.check_field(form, 'guid', addon.guid)
 
@@ -1541,7 +1548,8 @@ def check_row(self, tr, addon, good, bad, percentage, app, app_version):
         self.check_field(form, compat_field % 'app', str(app.id))
         self.check_field(form, compat_field % 'min_app_version',
                          app_version + 'a1')
-        self.check_field(form, compat_field % 'max_app_version', '*')
+        self.check_field(form, compat_field % 'max_app_version',
+                         app_version + '*')
 
     def check_field(self, form, name, val):
         eq_(form.find('input[name="%s"]' % name).val(), val)
@@ -1552,23 +1560,28 @@ def test_firefox_hosted(self):
                               app_version=self.app_version)
         self.update()
 
-        r = self.client.get(self.url)
-        eq_(r.status_code, 200)
-        table = pq(r.content)('#compat-results tbody')
-        tr = table.find('tr[data-addonid=%s]' % addon.id)
+        tr = self.get_pq().find('tr[data-addonid=%s]' % addon.id)
         self.check_row(tr, addon, good=0, bad=11, percentage='100.0',
                        app=self.app, app_version=self.app_version)
 
+        # Add an override for this current app version.
+        compat = CompatOverride.objects.create(addon=addon, guid=addon.guid)
+        CompatOverrideRange.objects.create(compat=compat,
+            app_id=amo.FIREFOX.id, min_app_version=self.app_version + 'a1',
+            max_app_version=self.app_version + '*')
+
+        # Check that there is an override for this current app version.
+        tr = self.get_pq().find('tr[data-addonid=%s]' % addon.id)
+        eq_(tr.find('.overrides a').attr('href'),
+            reverse('admin:addons_compatoverride_change', args=[compat.id]))
+
     def test_self_hosted(self):
         addon = self.populate(guid='yyy', status=amo.STATUS_LISTED)
         self.generate_reports(addon, good=2, bad=18, app=self.app,
                               app_version=self.app_version)
         self.update()
 
-        r = self.client.get(self.url)
-        eq_(r.status_code, 200)
-        table = pq(r.content)('#compat-results tbody')
-        tr = table.find('tr[data-addonid=%s]' % addon.id)
+        tr = self.get_pq().find('tr[data-addonid=%s]' % addon.id)
         eq_(tr.length, 1)
 
         eq_(tr.find('.maxver').text(), 'Self Hosted')
@@ -1589,9 +1602,7 @@ def test_non_default_version(self):
                               app_version=app_version)
         self.update()
 
-        r = self.client.get(self.url)
-        eq_(r.status_code, 200)
-        table = pq(r.content)('#compat-results tbody')
+        table = self.get_pq()
         eq_(table.find('tr[data-addonid=%s]' % addon.id).length, 0)
 
         r = self.client.get(self.url,
@@ -1601,3 +1612,20 @@ def test_non_default_version(self):
         tr = table.find('tr[data-addonid=%s]' % addon.id)
         self.check_row(tr, addon, good=0, bad=11, percentage='100.0',
                        app=self.app, app_version=app_version)
+
+    def test_ratio(self):
+        addon = self.populate(guid='aaa')
+        self.generate_reports(addon, good=11, bad=11, app=self.app,
+                              app_version=self.app_version)
+        self.update()
+
+        # Should not show up for > 80%.
+        eq_(self.get_pq().find('tr[data-addonid=%s]' % addon.id).length, 0)
+
+        # Should not show up for > 50%.
+        tr = self.get_pq(ratio=.5).find('tr[data-addonid=%s]' % addon.id)
+        eq_(tr.length, 0)
+
+        # Should show up for > 40%.
+        tr = self.get_pq(ratio=.4).find('tr[data-addonid=%s]' % addon.id)
+        eq_(tr.length, 1)

apps/zadmin/views.py

@@ -42,7 +42,7 @@
 from addons.utils import ReverseNameLookup
 from bandwagon.models import Collection
 from compat.models import AppCompat
-from compat.forms import CompatForm
+from compat.forms import AdminCompatForm
 from devhub.models import ActivityLog
 from files.models import Approval, File
 from versions.compare import version_int as vint
@@ -402,23 +402,28 @@ def jetpack_resend(request, file_id):
 def compat(request):
     APP = amo.FIREFOX
     VER = site_settings.COMPAT[0]['main']  # Default: latest Firefox version.
+    ratio = .8
     binary = None
 
     # Expected usage:
-    #     For Firefox 8.0 reports: ?appver=1-8.0
-    #     For binary-only add-ons: ?appver=1-8.0&type=binary
-    initial = {'appver': '%s-%s' % (APP.id, VER), 'type': 'all'}
+    #     For Firefox 8.0 reports:      ?appver=1-8.0
+    #     For over 70% incompatibility: ?appver=1-8.0&ratio=0.7
+    #     For binary-only add-ons:      ?appver=1-8.0&type=binary
+    initial = {'appver': '%s-%s' % (APP.id, VER), 'ratio': ratio,
+               'type': 'all'}
     initial.update(request.GET.items())
 
-    form = CompatForm(initial)
+    form = AdminCompatForm(initial)
     if request.GET and form.is_valid():
         APP, VER = form.cleaned_data['appver'].split('-')
         APP = amo.APP_IDS[int(APP)]
+        if form.cleaned_data['ratio']:
+            ratio = float(form.cleaned_data['ratio'])
         if form.cleaned_data['type'] == 'binary':
             binary = True
 
-    app, ver = str(APP.id), str(vint(VER))
-    usage_addons, usage_total = compat_stats(request, app, ver, binary)
+    app, ver = str(APP.id), VER
+    usage_addons, usage_total = compat_stats(request, app, ver, ratio, binary)
 
     return jingo.render(request, 'zadmin/compat.html', {
         'app': APP,
@@ -429,15 +434,16 @@ def compat(request):
     })
 
 
-def compat_stats(request, app, ver, binary):
+def compat_stats(request, app, ver, ratio, binary):
     # Get the list of add-ons for usage stats.
     redis = redisutils.connections['master']
     # Show add-ons marked as incompatible with this current version having
     # greater than 10 incompatible reports and whose average exceeds 80%.
-    prefix = 'works.%s.%s' % (app, ver)
+    ver_int = str(vint(ver))
+    prefix = 'works.%s.%s' % (app, ver_int)
     qs = (AppCompat.search()
           .filter(**{'%s.failure__gt' % prefix: 10,
-                     '%s.failure_ratio__gt' % prefix: .8,
+                     '%s.failure_ratio__gt' % prefix: ratio,
                      'support.%s.max__gte' % app: 0})
           .order_by('-%s.failure_ratio' % prefix,
                     '-%s.total' % prefix)
@@ -448,7 +454,7 @@ def compat_stats(request, app, ver, binary):
     for obj in addons.object_list:
         obj['usage'] = obj['usage'][app]
         obj['max_version'] = obj['max_version'][app]
-        obj['works'] = obj['works'][app].get(ver, {})
+        obj['works'] = obj['works'][app].get(ver_int, {})
         # Get all overrides for this add-on.
         obj['overrides'] = CompatOverride.objects.filter(addon__id=obj['id'])
         # Determine if there is an override for this current app version.

settings.py

@@ -1209,6 +1209,8 @@ def read_only_mode(env):
 # previous: the major version before :main.
 COMPAT = (
     # Firefox.
+    dict(app=1, main='11.0', versions=('11.0', '11.0a2', '11.0a1'),
+         previous='10.0'),
     dict(app=1, main='10.0', versions=('10.0', '10.0a2', '10.0a1'),
          previous='9.0'),
     dict(app=1, main='9.0', versions=('9.0', '9.0a2', '9.0a1'),