Allow reviewers to override minimum requirements for an app (bug 862467)

apps/amo/log.py

@@ -618,6 +618,14 @@ class GROUP_USER_REMOVED(_LOG):
     admin_event = True
 
 
+class REVIEW_FEATURES_OVERRIDE(_LOG):
+    id = 122
+    format = _(u'{addon} minimum requirements manually changed by reviewer.')
+    short = _(u'Requirements Changed by Reviewer')
+    keep = True
+    review_queue = True
+
+
 LOGS = [x for x in vars().values()
         if isclass(x) and issubclass(x, _LOG) and x != _LOG]
 

media/css/devreg/reviewers.styl

@@ -862,6 +862,9 @@ button.search, .log-filter-outside button {
             }
         }
     }
+    .review-actions-features-override ul {
+        columns(2, 1em);
+    }
 }
 
 .review-actions {

mkt/reviewers/templates/reviewers/review.html

@@ -253,6 +253,16 @@
         {{ form.device_override }}
         {{ form.device_override.errors}}
       </div>
+      {% if waffle.switch('buchets') %}
+      <div class="review-actions-section review-actions-features-override data-toggle" data-value="public">
+        <strong>{{ _('Minimum Requirements Override:') }}</strong>
+        <ul>
+          {% for field in appfeatures_form.required_api_fields() %}
+            <li><label for="{{ field.auto_id }}">{{ field }} {{ field.help_text }}</label></li>
+          {% endfor %}
+        </ul>
+      </div>
+      {% endif %}
       <div class="review-actions-section">
         {{ form.notify }}
         <label for="id_notify">

mkt/reviewers/tests/test_views.py

@@ -988,6 +988,48 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
         self._check_email(msg, 'Submission Update')
         self._check_email_body(msg)
 
+    def test_pending_to_public_w_requirements_overrides(self):
+        self.create_switch(name='buchets')
+        data = {'action': 'public', 'comments': 'something',
+                'has_sms': True}
+        data.update(self._attachment_management_form(num=0))
+        assert not self.app.current_version.features.has_sms
+        self.post(data)
+        app = self.get_app()
+        assert app.current_version.features.has_sms
+        eq_(app.make_public, amo.PUBLIC_WAIT)
+        eq_(app.status, amo.STATUS_PUBLIC_WAITING)
+        self._check_log(amo.LOG.REVIEW_FEATURES_OVERRIDE)
+
+    def test_pending_to_reject_w_requirements_overrides(self):
+        # Rejecting an app doesn't let you override features requirements.
+        self.create_switch(name='buchets')
+        data = {'action': 'reject', 'comments': 'something',
+                'has_sms': True}
+        data.update(self._attachment_management_form(num=0))
+        assert not self.app.current_version.features.has_sms
+        self.post(data)
+        app = self.get_app()
+        eq_(app.make_public, amo.PUBLIC_IMMEDIATELY)
+        eq_(app.status, amo.STATUS_REJECTED)
+        assert not app.current_version.features.has_sms
+
+    def test_pending_to_reject_w_requirements_overrides_nothing_changed(self):
+        self.version.features.update(has_sms=True)
+        self.create_switch(name='buchets')
+        data = {'action': 'public', 'comments': 'something',
+                'has_sms': True}
+        data.update(self._attachment_management_form(num=0))
+        assert self.app.current_version.features.has_sms
+        self.post(data)
+        app = self.get_app()
+        assert app.current_version.features.has_sms
+        eq_(app.make_public, None)
+        eq_(app.status, amo.STATUS_PUBLIC)
+        action_id = amo.LOG.REVIEW_FEATURES_OVERRIDE.id
+        assert not AppLog.objects.filter(
+                        addon=self.app, activity_log__action=action_id).exists()
+
     @mock.patch('addons.tasks.index_addons')
     @mock.patch('mkt.webapps.models.Webapp.update_supported_locales')
     @mock.patch('mkt.webapps.models.Webapp.update_name_from_package_manifest')

mkt/reviewers/views.py

@@ -47,6 +47,7 @@ from zadmin.models import set_config, unmemoized_get_config
 from mkt.reviewers.utils import AppsReviewing, clean_sort_param
 from mkt.search.forms import ApiSearchForm
 from mkt.site.helpers import product_as_dict
+from mkt.submit.forms import AppFeaturesForm
 from mkt.webapps.models import Webapp
 
 from . import forms
@@ -185,39 +186,74 @@ def _review(request, addon, version):
                                  files=request.FILES or None, request=request,
                                  addon=addon, version=version,
                                  attachment_formset=attachment_formset)
+    postdata = request.POST if request.method == 'POST' else None
+    all_forms = [form, attachment_formset]
+
+    if waffle.switch_is_active('buchets'):
+        features_list = [unicode(f) for f in version.features.to_list()]
+        appfeatures_form = AppFeaturesForm(data=postdata,
+                                           instance=version.features)
+        all_forms.append(appfeatures_form)
+    else:
+        appfeatures_form = None
+
     queue_type = form.helper.review_type
     redirect_url = reverse('reviewers.apps.queue_%s' % queue_type)
     is_admin = acl.action_allowed(request, 'Addons', 'Edit')
 
-    forms_valid = lambda: form.is_valid() and attachment_formset.is_valid()
+    if request.method == 'POST' and all(f.is_valid() for f in all_forms):
-    if request.method == 'POST' and forms_valid():
 
         old_types = set(o.id for o in addon.device_types)
         new_types = set(form.cleaned_data.get('device_override'))
 
-        if (form.cleaned_data.get('action') == 'public' and
+        if waffle.switch_is_active('buchets'):
-            old_types != new_types):
+            old_features = set(features_list)
+            new_features = set(unicode(f) for f
+                               in appfeatures_form.instance.to_list())
 
-            # The reviewer overrode the device types. We need to not publish
+        if form.cleaned_data.get('action') == 'public':
-            # this app immediately.
+            if old_types != new_types:
-            if addon.make_public == amo.PUBLIC_IMMEDIATELY:
+                # The reviewer overrode the device types. We need to not publish
-                addon.update(make_public=amo.PUBLIC_WAIT)
+                # this app immediately.
+                if addon.make_public == amo.PUBLIC_IMMEDIATELY:
+                    addon.update(make_public=amo.PUBLIC_WAIT)
 
-            # And update the device types to what the reviewer set.
+                # And update the device types to what the reviewer set.
-            AddonDeviceType.objects.filter(addon=addon).delete()
+                AddonDeviceType.objects.filter(addon=addon).delete()
-            for device in form.cleaned_data.get('device_override'):
+                for device in form.cleaned_data.get('device_override'):
-                addon.addondevicetype_set.create(device_type=device)
+                    addon.addondevicetype_set.create(device_type=device)
 
-            # Log that the reviewer changed the device types.
+                # Log that the reviewer changed the device types.
-            added_devices = new_types - old_types
+                added_devices = new_types - old_types
-            removed_devices = old_types - new_types
+                removed_devices = old_types - new_types
-            msg = _(u'Device(s) changed by reviewer: {0}').format(', '.join(
+                msg = _(u'Device(s) changed by reviewer: {0}').format(', '.join(
-                [_(u'Added {0}').format(unicode(amo.DEVICE_TYPES[d].name))
+                    [_(u'Added {0}').format(unicode(amo.DEVICE_TYPES[d].name))
-                 for d in added_devices] +
+                     for d in added_devices] +
-                [_(u'Removed {0}').format(unicode(amo.DEVICE_TYPES[d].name))
+                    [_(u'Removed {0}').format(unicode(amo.DEVICE_TYPES[d].name))
-                 for d in removed_devices]))
+                     for d in removed_devices]))
-            amo.log(amo.LOG.REVIEW_DEVICE_OVERRIDE, addon,
+                amo.log(amo.LOG.REVIEW_DEVICE_OVERRIDE, addon,
-                    addon.current_version, details={'comments': msg})
+                        addon.current_version, details={'comments': msg})
+
+            if (waffle.switch_is_active('buchets') and
+                 old_features != new_features):
+                # The reviewer overrode the requirements. We need to not publish
+                # this app immediately.
+                if addon.make_public == amo.PUBLIC_IMMEDIATELY:
+                    addon.update(make_public=amo.PUBLIC_WAIT)
+
+                appfeatures_form.save()
+
+                # Log that the reviewer changed the minimum requirements.
+                added_features = new_features - old_features
+                removed_features = old_features - new_features
+
+                fmt = ', '.join(
+                      [_(u'Added {0}').format(f) for f in added_features] +
+                      [_(u'Removed {0}').format(f) for f in removed_features])
+                # L10n: {0} is the list of requirements changes.
+                msg = _(u'Requirements changed by reviewer: {0}').format(fmt)
+                amo.log(amo.LOG.REVIEW_FEATURES_OVERRIDE, addon,
+                        addon.current_version, details={'comments': msg})
 
         form.helper.process()
 
@@ -271,10 +307,11 @@ def _review(request, addon, version):
                   allow_unchecking_files=allow_unchecking_files,
                   actions=actions, actions_minimal=actions_minimal,
                   tab=queue_type, product_attrs=product_attrs,
-                  attachment_formset=attachment_formset)
+                  attachment_formset=attachment_formset,
+                  appfeatures_form=appfeatures_form)
 
     if waffle.switch_is_active('buchets'):
-        ctx['feature_list'] = [unicode(f) for f in version.features.to_list()]
+        ctx['feature_list'] = features_list
 
     return jingo.render(request, 'reviewers/review.html', ctx)
 

mkt/webapps/fixtures/webapps/337141-steamcube.json

@@ -227,5 +227,46 @@
             "listed": true,
             "user": 31337
         }
+    },
+    {
+        "pk": 175,
+        "model": "webapps.appfeatures",
+        "fields": {
+            "has_video_h264": false,
+            "has_mp3": false,
+            "has_fullscreen": false,
+            "has_packaged_apps": false,
+            "has_bluetooth": false,
+            "has_gamepad": false,
+            "has_geolocation": false,
+            "has_indexeddb": false,
+            "has_push": false,
+            "has_idle": false,
+            "has_network_info": false,
+            "version": 1268829,
+            "has_orientation": false,
+            "has_apps": false,
+            "has_light_events": false,
+            "has_device_storage": false,
+            "has_touch": false,
+            "has_archive": false,
+            "has_proximity": false,
+            "has_contacts": false,
+            "has_quota": false,
+            "has_sms": false,
+            "has_time_clock": false,
+            "has_battery": false,
+            "has_fm": false,
+            "has_webaudio": false,
+            "has_pay": false,
+            "created": "2013-06-06T11:46:12",
+            "has_activity": false,
+            "has_qhd": false,
+            "modified": "2013-06-06T11:46:12",
+            "has_video_webm": false,
+            "has_network_stats": false,
+            "has_vibrate": false,
+            "has_audio": false
+        }
     }
 ]